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 computing item property #8485

Merged

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Nov 29, 2020

  • This is an ⚡ optimization change.

Summary

The :to_liquid method for Convertible objects (mainly Jekyll::Page and subclasses) is not memoized. Therefore, calling Jekyll::Page#to_liquid multiple times results in unnecessary Hash allocations.

This change optimizes iteration through site.pages when item_property of each page is computed.

Possible use-cases

Filtering site.pages with either of sort, where, find filters multiple times.

@ashmaroli
Copy link
Member Author

Third-party Profiling Summary

--- master https://github.com/jekyll/jekyll/runs/1469611301
+++ PR     https://github.com/jekyll/jekyll/runs/1470442250

- done in 49.315 seconds.
- done in 50.467 seconds.
- done in 50.72 seconds.
+ done in 29.244 seconds.
+ done in 30.09 seconds.
+ done in 30.28 seconds.

- Total allocated: 2.32 GB (4820149 objects)
- Total retained:  46.63 MB (127078 objects)
+ Total allocated: 433.99 MB (1926164 objects)
+ Total retained:  45.91 MB (126642 objects)

@ashmaroli ashmaroli added this to the 4.2 milestone Nov 30, 2020
@ashmaroli
Copy link
Member Author

@jekyllbot: merge +fix

@jekyllbot jekyllbot merged commit 573b431 into jekyll:master Dec 2, 2020
@ashmaroli ashmaroli deleted the reduce-item-property-allocations branch December 2, 2020 05:57
jekyllbot added a commit that referenced this pull request Dec 2, 2020
@ashmaroli ashmaroli removed the bug label Dec 2, 2020
github-actions bot pushed a commit that referenced this pull request Dec 2, 2020
Ashwin Maroli: Reduce allocations from computing item property (#8485)

Merge pull request 8485
github-actions bot pushed a commit to tigefa4u/jekyll that referenced this pull request Dec 2, 2020
jekyllbot: Update history to reflect merge of jekyll#8485 [ci skip]
@jekyll jekyll locked and limited conversation to collaborators Dec 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants