Build the render context without per-key ChainMap lookups#2185
Open
frenck wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A template's globals are stored as a
collections.ChainMapthat layers the template globals over the environment globals (seeEnvironment.make_globals).On every render
runtime.new_contextflattens that into the contextparentdict withdict(globals or (), **vars). Sinceglobalsis aChainMap, that callsChainMap.__getitem__once for every key. On top of thatContext.__init__walks the same mapping again withChainMap.__iter__to buildglobals_keysFlattening to a plain dict is fine and worth keeping, it makes the later
resolve_or_missinglookups fast. This PR only changes how the flatten happens: it merges the ChainMaps.mapswithdict.update(andset.unionforglobals_keys) instead of going through the ChainMap. The result is the same dict, it just takes fewer calls to build the result.It's per-render work that grows with the number of globals, and every template pays it. Even
does 6
ChainMap.__getitem__and 2ChainMap.__iter__calls for the 6 default globals. After this change it does none.Whether that's noticeable depends a lot on how Jinja is used. As the FAQ says, for plenty of apps the engine is dwarfed by database or API calls and you won't see any difference. Where it does help is the other case: lots of small renders, a decent number of globals, and little I/O per render. Like Home Assistant 😄 (2M+ installs) where templates render off an in-memory state machine in the hot path, so doing fewer calls per render adds up across a lot of renders.
Behavior is unchanged either way, it's the same output with less work.
One honest note on the tests: The contributing guide asks for tests that fail without the change, and these don't, because there's no behavior change to catch. I added them anyway as a safety net.
../Frenck