-
Notifications
You must be signed in to change notification settings - Fork 137
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
Cache normalized string #2777
Cache normalized string #2777
Conversation
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:feature/stable-cadence commit 9a146ab Collapsed results for better readability
|
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.
Nice! Thanks
There's a bit of a problem with the lazy approach: Cadence currently stores Strings inline or as a separate slab, if too large. This is possible because strings are immutable. However, in this PR we're lazily normalizing the string, effectively "mutating" it (replacing it, with a potentially smaller or larger form). In the Discussing this in https://discord.com/channels/613813861610684416/1108479699732152503/1151271319548211210 Ideas:
I opened an implementation of the later as #2781 |
Closing in favor of #2781 |
Work towards #2752
Description
String functions, like checking for equality, already operate on the normalized form of the string.
However, the results are currently discarded.
As strings are immutable, we can cache the normalized form and re-use it.
As the hash value is changed, this will need a migration. Given that we can perform breaking changes in the Stable Cadence release, we should use this opportunity.
master
branchFiles changed
in the Github PR explorer