Skip to content

Mustachio: cache property map for each type #2549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 2, 2021

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Mar 1, 2021

Currently the property maps are each a static function, rather than a getter, because they need to be type parameterized. In this case, however, they are built up every time a property needs to be accessed during mustache rendering. Here's one example, the property map for Package:

=> {
          ..._Renderer_LibraryContainer.propertyMap<CT_>(),
          ..._Renderer_Nameable.propertyMap<CT_>(),
          ..._Renderer_Locatable.propertyMap<CT_>(),
          ..._Renderer_Canonicalization.propertyMap<CT_>(),
          ..._Renderer_Warnable.propertyMap<CT_>(),
          'allLibraries': Property(...),
          ...

Package extends LibraryContainer, so it's property map is spread in first. Package then mixes in Nameable, Locatable, Canonicalization, and Warnable, so those are each spread in next. Finally, Package declares its own methods, like allLibraries, which are added last. Each property like allLibraries contains a few function literals. Overall, this Map is expensive to compute repeatedly.

By caching it on the context type, CT_, we generally build it very few times per renderer type. Generics don't play a huge role, so each renderer ultimately supports a small handful of types, typically one.

@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Mar 1, 2021
@coveralls
Copy link

coveralls commented Mar 1, 2021

Coverage Status

Coverage remained the same at 91.741% when pulling fbabdc7 on srawlins:mustachio-s into f8d62e0 on dart-lang:master.

@srawlins srawlins requested a review from jcollins-g March 1, 2021 15:57
Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, optional minor change only

@srawlins srawlins merged commit f0adbfa into dart-lang:master Mar 2, 2021
@srawlins srawlins deleted the mustachio-s branch March 2, 2021 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants