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 array allocations from StaticFile#path
#8083
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any possible side-effect due to memoization? Not sure if our tests would catch those.
In theory, a resource's (irrespective of page, document, static-file, layout, etc) If at all something is mutating the |
Should I freeze the attribute value and see if something breaks / fails..? |
As you see fit, just want to make sure we don't ship any regression, or that we don't miss any test here 😅 |
Note: I'll unfreeze the attribute value after some testing.. |
This reverts commit d042e7a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Ashwin.
@jekyllbot: merge +fix |
Summary
Jekyll::StaticFile#path
involves duplicating arrays viaArray#Compact
. So multiple calls to the unmemoized method would result in unnecessary Array allocations.