-
-
Notifications
You must be signed in to change notification settings - Fork 722
perf(transformer/styled-components): remove quasis and expressions in batch #12256
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(transformer/styled-components): remove quasis and expressions in batch #12256
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. |
7459b4d to
b20f2ab
Compare
CodSpeed Instrumentation Performance ReportMerging #12256 will not alter performanceComparing Summary
|
8152fe7 to
fb76e3c
Compare
b20f2ab to
f76ca34
Compare
f76ca34 to
85bdee6
Compare
overlookmotel
left a comment
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.
As you say, this case is uncommon, but yes using retain is ideal to avoid poor perf in odd cases where there are tons of quasis and many get deleted. 👍
If we do #12213, then probably we'll be able to make the main loop use retain, and then we won't need the extra loops to remove nodes at the end.
85bdee6 to
505b8db
Compare
Merge activity
|
… batch (#12256) Remove `quasis` and `expressions` in batch instead of removing them one by one, so that it would be more efficient. Considering that the expression's removal is unlikely to be common, I add a `has_deletion_marker` flag to avoid unnecessary `retain` calls, so that we can keep the performance as before in this situation.
505b8db to
a8d1e22
Compare
Follow-on after #12256. Update comments to reflect the new removal logic. Also tidy up some other comments.
Follow-on after #12256. Nit! Rename var to a name which reflects what the var *means*, rather than the implementation detail.
Follow-on after #12256. Optimize the `retain` loop for removing quasis and expressions by doing bounds check only once at start, rather than on each turn of the loop.
Follow-on after #12256. Optimize the `retain` loop for removing quasis and expressions by doing bounds check only once at start, rather than on each turn of the loop.

Remove
quasisandexpressionsin batch instead of removing them one by one, so that it would be more efficient. Considering that the expression's removal is unlikely to be common, I add ahas_deletion_markerflag to avoid unnecessaryretaincalls, so that we can keep the performance as before in this situation.