Skip to content
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: Reduce the use of temporary objects #16959

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Nov 12, 2024

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

#16958 (review)

main

PS F:\babel\benchmark> node --expose-gc .\babel-generator\real-case\jquery.mjs
current 1 jquery 3.6: 151 ops/sec ±2.81% (6.614ms)
current 4 jquery 3.6: 35.75 ops/sec ±0.5% (28ms)
current 16 jquery 3.6: 8.93 ops/sec ±0.7% (112ms)
current 64 jquery 3.6: 2.09 ops/sec ±6.15% (478ms)
baseline 1 jquery 3.6: 162 ops/sec ±0.75% (6.17ms)
baseline 4 jquery 3.6: 37.09 ops/sec ±0.71% (27ms)
baseline 16 jquery 3.6: 9.29 ops/sec ±0.78% (108ms)
baseline 64 jquery 3.6: 2.26 ops/sec ±0.93% (443ms)

PR

PS F:\babel\benchmark> node --expose-gc .\babel-generator\real-case\jquery.mjs
current 1 jquery 3.6: 159 ops/sec ±0.67% (6.302ms)
current 4 jquery 3.6: 36.54 ops/sec ±1.83% (27ms)
current 16 jquery 3.6: 9.4 ops/sec ±0.54% (106ms)
current 64 jquery 3.6: 2.21 ops/sec ±4.27% (453ms)
baseline 1 jquery 3.6: 162 ops/sec ±0.56% (6.155ms)
baseline 4 jquery 3.6: 37.26 ops/sec ±0.33% (27ms)
baseline 16 jquery 3.6: 9.41 ops/sec ±0.49% (106ms)
baseline 64 jquery 3.6: 2.22 ops/sec ±5.92% (451ms)

Comment on lines 866 to 875
printJoin(
nodes: Array<t.Node> | undefined | null,
opts: PrintJoinOptions = {},
statement?: boolean,
indent?: boolean,
separator?: PrintJoinOptions["separator"],
printTrailingSeparator?: boolean,
addNewlines?: PrintJoinOptions["addNewlines"],
iterator?: PrintJoinOptions["iterator"],
trailingCommentsLineOffset?: number,
) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Their order is similar to the frequency of use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are quite a few boolean options here, maybe we can pack them into bit flags.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally working on bit flags, but then realized the code needed was pretty much the same either way.
I'll switch it over to bit flags and see if that makes it a bit cleaner. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a bit difficult since indent===null would make sense.

@babel-bot
Copy link
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58329

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am all for changes in this PR. The temporary object style was prevailing before we migrated to TS since at that moment the IDE can't infer the parameter names.

@liuxingbaoyu liuxingbaoyu added pkg: generator PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories labels Nov 13, 2024
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More of this! :)

@nicolo-ribaudo nicolo-ribaudo merged commit 39cd56c into babel:main Dec 4, 2024
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: generator PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants