-
-
Notifications
You must be signed in to change notification settings - Fork 781
refactor(transformer): use StringBuilder instead of String
#11260
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
refactor(transformer): use StringBuilder instead of String
#11260
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Instrumentation Performance ReportMerging #11260 will improve performances by 29.52%Comparing Summary
Benchmarks breakdown
|
c93fda2 to
6af0650
Compare
|
Oh bollocks! It's a -1% perf hit. Probably because main usage is generating UIDs, where we only use |
6af0650 to
7f10c31
Compare
447d22e to
dc1afcb
Compare
|
The perf drop is now gone thanks to #11261 (or maybe it was just noise in benchmarks, hard to know). I did find from Godbolt that |
Merge activity
|
Replace usage of `String` with `StringBuilder` (introduced in #11257) in transformer.
Add `Atom::from_strs_array_in` method, to complement `String::from_strs_array_in`. In many places we were using `String::from_strs_array_in` only as a means to get an `Atom`. Cut out the middleman! This prepares the way for #11260, and getting rid of `String` entirely.
dc1afcb to
02d3bb7
Compare
7f10c31 to
202ffd2
Compare

Replace usage of
StringwithStringBuilder(introduced in #11257) in transformer.