Add support for caching in build step#99
Open
joelhawksley wants to merge 6 commits into
Open
Conversation
Owner
|
Thank you @joelhawksley! 🙏🏼 I want to be transparent here: I'll be at RubyKaigi next week and just added a mental note for this PR. I will get back to you after the conference, especially because I have some ideas I want to discuss with people to see if we actually need something like this. I'll keep you posted 🙌🏼 |
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.
Context
As documented in #96, we have observed a significant increase in ERB compilation overhead, which we see at boot time when using https://github.com/jhawthorn/actionview_precompiler and when ViewComponents compile their templates. On a test production deployment, we saw boot-time template precompilation jump from ~15s to nearly a minute, an unacceptable overhead to pay every time we boot a container.
Proposed solution
This PR adds a
reactionview:precompilerake task that compiles all files covered by the Herb configuration inclusion/exclusion rules. Ifconfig.cache = true, the compiled cache is used, skipping the work of compiling ERB to Ruby. In a test production deployment we saw equivalent boot-time precompilation times in the 15s range.If this change is generally deemed acceptable, I'd be happy to add docs to this PR.
Alternatives considered
Add cache to ActionView::Precompiler
I wrote jhawthorn/actionview_precompiler#46 (cc @jhawthorn) and deployed it to a lab environment with good results. However, the optimization is really only necessary for Herb, so adding the complexity there felt like a bit much.
Add cache to Herb
I wrote https://github.com/marcoroth/herb/compare/main...joelhawksley:herb:cache?expand=1 to add a cache to Herb instead. Unfortunately, ReActionView configures Herb with specific options which resulted in the cache not being hit.
Further opportunities
In a preliminary benchmark, I tried compiling to Ruby instruction sequences, resulting in performance faster than Erubi. I did not include this change here in order to keep things simple.
References
Closes #96