Code

Opened 6 years ago

Closed 8 months ago

#7116 closed Cleanup/optimization (fixed)

Optimize RequestContext construction

Reported by: Bastian Kleineidam <calvin@…> Owned by: nobody
Component: Template system Version: master
Severity: Normal Keywords:
Cc: lrekucki@…, FunkyBob Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The current RequestContext constructor pushes a new context variable
onto the stack for each template processor result. This is not
necessary. This patch just updates the top dict entry of the
current context instead of pushing a new one onto the context dict stack.

This might break some apps that pop() template processor contexts after
they have been added, but that would be using an undocumented feature, and its
a bad idea: why add the contexts in the first place if they get removed afterwards.

Attachments (1)

commit-5ec2f17 (440 bytes) - added by Bastian Kleineidam <calvin@…> 6 years ago.

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Bastian Kleineidam <calvin@…>

comment:1 Changed 6 years ago by programmerq

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:3 Changed 3 years ago by Alex

  • Easy pickings unset
  • Triage Stage changed from Design decision needed to Accepted
  • UI/UX unset

comment:4 Changed 18 months ago by lrekucki

  • Needs tests set
  • Patch needs improvement set

Modifying dict[0] isn't a good idea as those most likely would be the builtins that are initialized in BaseContext._reset_dicts, which means they get overshadowed by dict_ passed to the constructor.

Currently context processors overshadow the dict_ (I seen people debug their code for hours before realizing that, a real PITA), so any change to this behavior needs to be documented. A test case to show that no extra dicts are created while the right order is preserved would be good too :).

comment:5 Changed 18 months ago by lrekucki

  • Cc lrekucki@… added

comment:6 Changed 8 months ago by FunkyBob

  • Cc FunkyBob added

Given that most of the rest of the dict-ish methods on BaseContext use self.dicts[-1] ... it would make sense for this to, also.

I've tested it locally, and it passes all current tests.

comment:7 Changed 8 months ago by FunkyBob

I've created a PR for this - https://github.com/django/django/pull/1527

I put all the context processor data into its own stack dict, on the off chance someone relies on the dict they pass to RequestContext not being altered.

comment:8 Changed 8 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 8d473b2c54035cbcd3aacef0cb83a9769cd05ad3:

Fixed #7116 -- Optimize RequestContext construction

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.