Skip to content
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

Reduce allocations from parsing include params #8851

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ashmaroli
Copy link
Member

Problem

Tags::IncludeTag#parse_params is called from within the :render method. to generate the Hash object for context["include"] from string markup by using a regex. Consequently, if an include is rendered numerous times, a lot of intermediate objects get allocated.

Prior art

The allocations were partially reduced for v4.2.0 in #8192.

Current Proposal

  • Move parsing markup via regex into the :initialize method. The final Hash is however generated only via :render calls.
  • Add new IncludeTag::ParameterTokenList class to avoid generating new Hashes if the params are context-agnostic (i.e. no variable).

Additional Notes

Also includes a benchmark script that compares current proposal with v4.2 optimization and pre-v4.2 implementation.

@ashmaroli ashmaroli added memory-optimization ⚡ Reduced memory usage for the work done micro-optimization Matters only if it is a very large site ¯\_(ツ)_/¯ labels Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
memory-optimization ⚡ Reduced memory usage for the work done micro-optimization Matters only if it is a very large site ¯\_(ツ)_/¯
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant