-
-
Notifications
You must be signed in to change notification settings - Fork 511
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
perf(js_analyze): use Box<str>/Box<[_]> to reduce memory usage #4211
Conversation
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
CodSpeed Performance ReportMerging #4211 will improve performances by 9.25%Comparing Summary
Benchmarks breakdown
|
a08b0bf
to
aa7b4a3
Compare
@Conaclos did you see concrete differences by any chance when running the CLI? |
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.
Since you changed many lint rules, you might as well want to update the contribution guide of the analyzer, and add a chapter that talks about it.
@@ -4132,7 +4132,6 @@ | |||
"properties": { | |||
"attributes": { | |||
"description": "Additional attributes that will be sorted.", | |||
"default": ["class", "className"], |
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.
Is it expected?
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.
Yes. This avoids some allocations and doesn't change the behavior of the rule.
Unfortunately it is not significant in the test I did. However, this reduces a bit the size of our configuration. This is always good to take.
I added some notes. |
Summary
This PR replace
Vec<_>
andString
with their boxed versionsBox<[_]>
andBox<str>
in Rules'Options
andSignals
. This allows reducing memory usage and could improve perf in some scenario.To do so I implemented
Deserializable
forBox<[_]>
andBox<str>
.Also, I found some use of
text
instead oftrimmed_text
.I took the opportunity of replacing
text
withtrimmed_text
.Test Plan
CI must pass.