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.

Sign up for GitHub

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

Jump to bottom

perf: Improve parser performance for typescript #16072

Merged
merged 5 commits into from Jan 10, 2024
Merged

perf: Improve parser performance for typescript #16072

merged 5 commits into from Jan 10, 2024

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Oct 29, 2023 •
edited

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

The ts parsing time is reduced by ~20%, and the general js parsing time is increased by ~2%.
If we want general js not to be affected, maybe we can manually replace all getters/setters.

PS F:\babel\benchmark\babel-parser\real-case-ts> node .\bench.mjs
current 1 typescript parser.ts: 67.9 ops/sec ±1.01% (15ms)
baseline 1 typescript parser.ts: 55.09 ops/sec ±1.22% (18ms)
current 1 typescript parser.ts: 68.58 ops/sec ±0.79% (15ms)
baseline 1 typescript parser.ts: 55.88 ops/sec ±0.84% (18ms)
PS F:\babel\benchmark\babel-parser\real-case-ts> node .\bench.mjs
current 1 typescript parser.ts: 66.91 ops/sec ±0.98% (15ms)
baseline 1 typescript parser.ts: 53.97 ops/sec ±1.14% (19ms)
current 1 typescript parser.ts: 68.01 ops/sec ±0.75% (15ms)
baseline 1 typescript parser.ts: 55.14 ops/sec ±0.81% (18ms)

PS F:\babel\benchmark\babel-parser\real-case> node .\bench.mjs
current 4 jquery 3.6: 24.43 ops/sec ±1.95% (41ms)
baseline 4 jquery 3.6: 25.48 ops/sec ±1.11% (39ms)
current 4 jquery 3.6: 25.01 ops/sec ±1.7% (40ms)
baseline 4 jquery 3.6: 25.2 ops/sec ±1.35% (40ms)
PS F:\babel\benchmark\babel-parser\real-case> node .\bench.mjs
current 4 jquery 3.6: 24.46 ops/sec ±1.78% (41ms)
baseline 4 jquery 3.6: 24.89 ops/sec ±1.46% (40ms)
current 4 jquery 3.6: 24.61 ops/sec ±1.67% (41ms)
baseline 4 jquery 3.6: 25.16 ops/sec ±1.26% (40ms)

@liuxingbaoyu liuxingbaoyu added pkg: parser PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories labels Oct 29, 2023
@babel-bot
Copy link
Collaborator

babel-bot commented Oct 29, 2023 •
edited

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

@liuxingbaoyu
Copy link
Member Author

liuxingbaoyu commented Oct 31, 2023 •
edited

f1f8472 (#16072)
This commit will only have a slight performance impact on errors and will not affect overall performance, but it simplifies our code.

@liuxingbaoyu liuxingbaoyu force-pushed the perf-ts-parser branch 2 times, most recently from c8b3fc8 to f1f8472 Compare October 31, 2023 06:14
liuxingbaoyu
liuxingbaoyu commented Oct 31, 2023
View reviewed changes
packages/babel-parser/src/tokenizer/state.ts Outdated
@@ -152,20 +279,36 @@ export default class State {
}

clone(skipArrays?: boolean): State {
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually we don't use skipArrays anywhere. :)

Copy link
Member

Choose a reason for hiding this comment

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

Can we just remove it then? :)

nicolo-ribaudo
nicolo-ribaudo reviewed Nov 13, 2023
View reviewed changes
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.

I think given how popular TS is, the perf change is ok.

packages/babel-parser/src/tokenizer/state.ts
state.context = maybeCopy(this.context);
state.firstInvalidTemplateEscapePos = this.firstInvalidTemplateEscapePos;
state.strictErrors = this.strictErrors;
state.tokensLength = this.tokensLength;
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we can somehow type-check this list to make sure we don't forget to add props? If not, can you add a comment at the beginning of State's body saying that when adding a new property we must also add it here?

packages/babel-parser/src/parser/comments.ts
Comment on lines +105 to +108
const { commentsLen } = this.state;
if (this.comments.length != commentsLen) this.comments.length = commentsLen;
this.comments.push(comment);
this.state.commentsLen++;
Copy link
Member

Choose a reason for hiding this comment

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

Can it happen that we clone the state, push a comment to .comments, reset to the previous state, then never call .addComment again and so we never do this.comments.length = commentsLen removing the new extra comment?

nicolo-ribaudo
nicolo-ribaudo approved these changes Jan 9, 2024
View reviewed changes
packages/babel-parser/src/tokenizer/state.ts Outdated
@@ -152,20 +279,36 @@ export default class State {
}

clone(skipArrays?: boolean): State {
Copy link
Member

Choose a reason for hiding this comment

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

Can we just remove it then? :)

packages/babel-parser/src/util/scope.ts Outdated
Comment on lines 199 to 201
// In strict mode, scope.functions will always be empty.
// Modules are strict by default, but the `scriptMode` option
// can overwrite this behavior.
Copy link
Member

Choose a reason for hiding this comment

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

This comments can be removed, right?

JLHwung
JLHwung approved these changes Jan 9, 2024
View reviewed changes
Copy link
Contributor

@JLHwung JLHwung left a comment •
edited

Choose a reason for hiding this comment

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

Change summary for future reviewers:

  1. Pack tokenizer boolean properties into bit flags
  2. The tokenizer state access .lastTokStart is replaced by .lastTokStartLoc.index
  3. Replace .raise({ at }) by .raise(at) to reduce cost of intermediate object literal
  4. Specify each state properties in parser state clone to avoid dynamic property access
  5. Merge multiple names set in parser scope tracker into a map

@liuxingbaoyu liuxingbaoyu merged commit 0085301 into babel:main Jan 10, 2024
48 checks passed
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Apr 10, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JLHwung JLHwung JLHwung approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees
No one assigned
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@liuxingbaoyu @babel-bot @JLHwung @nicolo-ribaudo