Opened 17 years ago
Closed 11 years ago
#7116 closed Cleanup/optimization (fixed)
Optimize RequestContext construction
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Template system | Version: | dev |
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)
Change History (9)
by , 17 years ago
Attachment: | commit-5ec2f17 added |
---|
comment:1 by , 17 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:2 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Cleanup/optimization |
comment:3 by , 13 years ago
Easy pickings: | unset |
---|---|
Triage Stage: | Design decision needed → Accepted |
UI/UX: | unset |
comment:4 by , 12 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:5 by , 12 years ago
Cc: | added |
---|
comment:6 by , 11 years ago
Cc: | 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 by , 11 years ago
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 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Modifying
dict[0]
isn't a good idea as those most likely would be the builtins that are initialized inBaseContext._reset_dicts
, which means they get overshadowed bydict_
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 :).