Performance: Whiskers Regexes Cache+Optimize#16736
Merged
Merged
Conversation
Reuse static optimized regex objects in Whiskers template validation and rendering. On the target via-IR benchmark this regex-only variant measured 1.0335s warm average versus the 1.0640s baseline.
Contributor
|
First of all, thanks! Looks very neat, I hope we can merge this within a week or so. Scheduled the run on our benchmarking setup, https://github.com/argotorg/solc-bench/ I'll post the |
Contributor
Author
|
Running my own wider benchmark made the results better. Looking forward to yours.
|
Contributor
|
OOoo that's a nice set! Might steal that :D Do you wanna make a PR to solc-bench adding these? Otherwise, I'll add them :) But you know, while I like stealing, I'd rather you gave them away 😆 |
Contributor
Author
|
It's my list from purplebench, available here: https://github.com/DanielVF/purplebench/tree/master/suite/contracts If you import them, you could probably rename the actual contract name in the code to be clearer. I tried to select a set of important contracts with high TLV / transactions across a couple different domains. I then ripped out the pragmas, and upgraded the code to run on the latest compiler versions. |
Contributor
|
We should review and merge this asap :) |
Contributor
OK, stealing it then 😆 😆 😆 |
Member
|
Well, actually - this could've (should've?) gotten a changelog entry as user-visible performance improvement. |
Contributor
Is there precedent for this? I wouldn't be against it to be honest, could even write a blog post after we merge more in. |
Contributor
|
BTW, I had a similar, but shittier one at one point in my not very long Solidity life 😆 |
Member
Yep there is! Check the changelog for recent entries. There are a couple in there. |
Contributor
|
We can add it as a follow-up, perhaps as part of merging #16741 ? |
Contributor
Author
|
Yes, I'll add to 16741. |
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
5 participants
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.
Description
3% compile time improvement for
--via-ircomplilation+optimization and large solidity files, over 100 runs for this change and a baseline.Adding the 'std::regex_constants::optimize' flag increases the compile time for the regex, but reduces the run time each time it is used. Adding const to the regex creation, means that we only pay this penalty once, but benefit from it each run.
(The const change alone makes a small speedup, while optimize without the const makes a slowdown, since the extra computation for optimization isn't worth it unless the regex object is used multiple times.)
Checklist
Al Disclosure