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(parser): speed up parsing numbers with _ separators #4259

Merged

Conversation

overlookmotel
Copy link
Contributor

@overlookmotel overlookmotel commented Jul 14, 2024

When parsing a number which contains _ separators, rather than looping through the string once to remove separators, and then again to convert to an f64, do it all in a single loop.

Copy link

graphite-app bot commented Jul 14, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Contributor Author

overlookmotel commented Jul 14, 2024

@overlookmotel overlookmotel changed the title perf(parser): speed up parsing numbers with _ separat ors perf(parser): speed up parsing numbers with _ separators Jul 14, 2024
@overlookmotel overlookmotel marked this pull request as ready for review July 14, 2024 18:23
@overlookmotel
Copy link
Contributor Author

This is unlikely to register on benchmarks as I don't think the bench fixtures contain any numbers with _ separators. But it should be an improvement to files which do.

Copy link

codspeed-hq bot commented Jul 14, 2024

CodSpeed Performance Report

Merging #4259 will not alter performance

Comparing 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors (132fc6b) with 07-14-perf_parser_speed_up_parsing_octal_literals (b94540d)

Summary

✅ 32 untouched benchmarks

@DonIsaac DonIsaac force-pushed the 07-14-perf_parser_speed_up_parsing_octal_literals branch from 05a9c4d to e69abd6 Compare July 15, 2024 01:30
@DonIsaac DonIsaac force-pushed the 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors branch from 2bc92ca to 10d65e1 Compare July 15, 2024 01:30
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jul 15, 2024
Copy link

graphite-app bot commented Jul 15, 2024

Merge activity

  • Jul 14, 10:34 PM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 14, 10:34 PM EDT: Boshen added this pull request to the Graphite merge queue.
  • Jul 14, 10:54 PM EDT: The Graphite merge queue removed this pull request due to downstack failures on PR #4257.
  • Jul 14, 10:55 PM EDT: Boshen removed this pull request from the Graphite merge queue.
  • Jul 15, 12:07 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 15, 12:07 AM EDT: Boshen added this pull request to the Graphite merge queue.

@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_octal_literals branch from e69abd6 to ed2c1af Compare July 15, 2024 02:37
Boshen pushed a commit that referenced this pull request Jul 15, 2024
When parsing a number which contains `_` separators, rather than looping through the string once to remove separators, and then again to convert to an `f64`, do it all in a single loop.
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors branch from 10d65e1 to b3b5909 Compare July 15, 2024 02:37
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_octal_literals branch from ed2c1af to 5b3f877 Compare July 15, 2024 02:41
Boshen pushed a commit that referenced this pull request Jul 15, 2024
When parsing a number which contains `_` separators, rather than looping through the string once to remove separators, and then again to convert to an `f64`, do it all in a single loop.
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors branch from b3b5909 to f41d738 Compare July 15, 2024 02:41
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_octal_literals branch from 5b3f877 to d83b333 Compare July 15, 2024 02:50
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors branch from f41d738 to 64b3bc5 Compare July 15, 2024 02:50
Boshen pushed a commit that referenced this pull request Jul 15, 2024
When parsing a number which contains `_` separators, rather than looping through the string once to remove separators, and then again to convert to an `f64`, do it all in a single loop.
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_octal_literals branch from d83b333 to 8ea72a8 Compare July 15, 2024 02:53
Boshen pushed a commit that referenced this pull request Jul 15, 2024
When parsing a number which contains `_` separators, rather than looping through the string once to remove separators, and then again to convert to an `f64`, do it all in a single loop.
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors branch from 64b3bc5 to 026210e Compare July 15, 2024 02:53
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jul 15, 2024
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_octal_literals branch from 8ea72a8 to b94540d Compare July 15, 2024 03:42
When parsing a number which contains `_` separators, rather than looping through the string once to remove separators, and then again to convert to an `f64`, do it all in a single loop.
@Boshen Boshen force-pushed the 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors branch from 026210e to 132fc6b Compare July 15, 2024 03:43
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jul 15, 2024
@Boshen Boshen changed the base branch from 07-14-perf_parser_speed_up_parsing_octal_literals to main July 15, 2024 04:08
@Boshen Boshen merged commit a8dc4f3 into main Jul 15, 2024
28 checks passed
@Boshen Boshen deleted the 07-14-perf_parser_speed_up_parsing_numbers_with___separat_ors branch July 15, 2024 04:27
This was referenced Jul 17, 2024
Dunqing pushed a commit that referenced this pull request Jul 18, 2024
## [0.21.0] - 2024-07-18

- d7ab0b8 semantic: [**BREAKING**] Simplify node creation (#4226)
(lucab)

### Features

- af4dc01 ast: Align ts ast scope with typescript (#4253) (Dunqing)
- 83c2c62 codegen: Add option for choosing quotes; remove slow
`choose_quot` method (#4219) (Boshen)
- 5d17675 mangler: Add debug mode (#4314) (Boshen)
- e3e663b mangler: Initialize crate and integrate into minifier (#4197)
(Boshen)
- c818472 minifier: Dce conditional expression `&&` or `||` (#4190)
(Boshen)
- 8a190eb oxc: Export `oxc_mangler` (Boshen)
- 20cdb1f semantic: Align class scope with typescript (#4195) (Dunqing)
- 92ee774 semantic: Add `ScopeFlags::CatchClause` for use in CatchClause
(#4205) (Dunqing)
- 205c259 sourcemap: Support SourceMapBuilder#token_chunks (#4220)
(underfin)
- 7eb960d transformer: Decode xml character entity `&#xhhhh` and
`&#nnnn;` (#4235) (Boshen)

### Bug Fixes

- bf3d8d3 codegen: Print annotation comment inside parens for new and
call expressions (#4290) (Boshen)
- 084ab76 codegen: Use `ryu-js` for f64 to string (Boshen)
- e167ef7 codegen: Print parenthesis properly (#4245) (Boshen)
- c65198f codegen: Choose the right quote for jsx attribute string
(#4236) (Boshen)
- be82c28 codegen: Print `JSXAttributeValue::StringLiteral` directly
(#4231) (Boshen)
- 3df9e69 mangler: No shorthand `BindingProperty`; handle var hoisting
and export variables (#4319) (Boshen)
- f144082 minifier: RemoveDeadCode should visit nested expression
(#4268) (underfin)
- 66b455a oxc_codegen: Avoid print same pure comments multiple time
(#4230) (IWANABETHATGUY)
- 9a87e41 parser: Avoid crashing on invalid const modifier (#4267)
(lucab)
- 641a78b parser: Fix tests for number parsing (#4254) (overlookmotel)
- 9badac0 semantic: Avoid var hosting insert the var variable to the
`CatchClause` scope (#4337) (Dunqing)
- 95e15b6 semantic: Incorrect resolve references for `ExportSpecifier`
(#4320) (Dunqing)
- c362bf7 semantic: Incorrect resolve references for
`TSInterfaceHeritage` (#4311) (Dunqing)
- 351ecf2 semantic: Incorrect resolve references for `TSTypeQuery`
(#4310) (Dunqing)
- 1108f2a semantic: Resolve references to the incorrect symbol (#4280)
(Dunqing)
- 22d56bd semantic: Do not resolve references after `FormalParameters`
in TS type (#4241) (overlookmotel)- 1c117eb Avoid print extra semicolon
after accessor property (#4199) (IWANABETHATGUY)

### Performance

- a8dc4f3 parser: Speed up parsing numbers with `_` separators (#4259)
(overlookmotel)
- b94540d parser: Speed up parsing octal literals (#4258)
(overlookmotel)
- a7b328c parser: Faster parsing decimal numbers (#4257) (overlookmotel)
- f9d3f2e semantic: Inline ast record functions (#4272) (overlookmotel)
- 8fad7db semantic: Reduce `AstNodeId` to `u32` (#4264) (overlookmotel)
- 23743db semantic: Do not record ast nodes for cfg if cfg disabled
(#4263) (overlookmotel)
- da69076 semantic: Reduce overhead of cfg recording ast nodes (#4262)
(overlookmotel)
- cb15303 semantic: Reduce memory copies (#4216) (overlookmotel)
- ef4c1f4 semantic: Reduce lookups (#4214) (overlookmotel)
- f23e54f semantic: Recycle unresolved references hash maps (#4213)
(overlookmotel)
- 2602ce2 semantic: Reuse existing map of unresolved refs (#4206)
(lucab)

### Refactor

- 2c7bb9f ast: Pass final `ScopeFlags` into `visit_function` (#4283)
(overlookmotel)
- 3e099fe ast: Move `enter_scope` after `visit_binding_identifier`
(#4246) (Dunqing)
- aab7aaa ast/visit: Fire node events as the outermost one. (#4203)
(rzvxa)
- d1c4be0 codegen: Clean up annotation_comment (Boshen)
- 06197b8 codegen: Separate tests (Boshen)
- aa22073 codegen: Improve print API (#4196) (Boshen)
- c5731a5 semantic: Remove defunct code setting ScopeFlags twice (#4286)
(overlookmotel)
- 16698bc semantic: Move function/class-specific code into specific
visitors (#4278) (overlookmotel)
- ee16668 semantic: Rename function param (#4277) (overlookmotel)
- 25f0771 semantic: Alter syntax of `control_flow!` macro (#4275)
(overlookmotel)
- 639fd48 semantic: Comment why extra CFG enabled check (#4274)
(overlookmotel)
- c418bf5 semantic: Directly record `current_node_id` when adding a
scope (#4265) (Dunqing)
- ace4f1f semantic: Update the order of `visit_function` and `Visit`
fields in the builder to be consistent (#4248) (Dunqing)
- 8bfeabf semantic: Simplify adding `SymbolFlags::Export` (#4249)
(Dunqing)
- dc2b3c4 semantic: Add strict mode in scope flags for class definitions
(#4156) (Dunqing)
- 81ed588 semantic: Convert scope fields to IndexVecs (#4208) (lucab)
- bbe5ded semantic: Set `current_scope_id` to `scope_id` in
`enter_scope` (#4193) (Dunqing)
- 7f1addd semantic: Correct scope in CatchClause (#4192) (Dunqing)
- fc0b17d syntax: Turn the `AstNodeId::dummy` into a constant field.
(#4308) (rzvxa)
- a197e01 transformer/typescript: Remove unnecessary code (#4321)
(Dunqing)
- 1458d81 visit: Add `#[inline]` to empty functions (#4330)
(overlookmotel)

Co-authored-by: Boshen <[email protected]>
@github-actions github-actions bot mentioned this pull request Jul 18, 2024
Dunqing pushed a commit that referenced this pull request Jul 18, 2024
## [0.21.0] - 2024-07-18

- d7ab0b8 semantic: [**BREAKING**] Simplify node creation (#4226)
(lucab)

### Features

- af4dc01 ast: Align ts ast scope with typescript (#4253) (Dunqing)
- 83c2c62 codegen: Add option for choosing quotes; remove slow
`choose_quot` method (#4219) (Boshen)
- 5d17675 mangler: Add debug mode (#4314) (Boshen)
- e3e663b mangler: Initialize crate and integrate into minifier (#4197)
(Boshen)
- c818472 minifier: Dce conditional expression `&&` or `||` (#4190)
(Boshen)
- 8a190eb oxc: Export `oxc_mangler` (Boshen)
- 20cdb1f semantic: Align class scope with typescript (#4195) (Dunqing)
- 92ee774 semantic: Add `ScopeFlags::CatchClause` for use in CatchClause
(#4205) (Dunqing)
- 205c259 sourcemap: Support SourceMapBuilder#token_chunks (#4220)
(underfin)
- 7eb960d transformer: Decode xml character entity `&#xhhhh` and
`&#nnnn;` (#4235) (Boshen)

### Bug Fixes

- bf3d8d3 codegen: Print annotation comment inside parens for new and
call expressions (#4290) (Boshen)
- 084ab76 codegen: Use `ryu-js` for f64 to string (Boshen)
- e167ef7 codegen: Print parenthesis properly (#4245) (Boshen)
- c65198f codegen: Choose the right quote for jsx attribute string
(#4236) (Boshen)
- be82c28 codegen: Print `JSXAttributeValue::StringLiteral` directly
(#4231) (Boshen)
- 3df9e69 mangler: No shorthand `BindingProperty`; handle var hoisting
and export variables (#4319) (Boshen)
- f144082 minifier: RemoveDeadCode should visit nested expression
(#4268) (underfin)
- 66b455a oxc_codegen: Avoid print same pure comments multiple time
(#4230) (IWANABETHATGUY)
- 9a87e41 parser: Avoid crashing on invalid const modifier (#4267)
(lucab)
- 641a78b parser: Fix tests for number parsing (#4254) (overlookmotel)
- 9badac0 semantic: Avoid var hosting insert the var variable to the
`CatchClause` scope (#4337) (Dunqing)
- 95e15b6 semantic: Incorrect resolve references for `ExportSpecifier`
(#4320) (Dunqing)
- c362bf7 semantic: Incorrect resolve references for
`TSInterfaceHeritage` (#4311) (Dunqing)
- 351ecf2 semantic: Incorrect resolve references for `TSTypeQuery`
(#4310) (Dunqing)
- 1108f2a semantic: Resolve references to the incorrect symbol (#4280)
(Dunqing)
- 22d56bd semantic: Do not resolve references after `FormalParameters`
in TS type (#4241) (overlookmotel)- 1c117eb Avoid print extra semicolon
after accessor property (#4199) (IWANABETHATGUY)

### Performance

- a8dc4f3 parser: Speed up parsing numbers with `_` separators (#4259)
(overlookmotel)
- b94540d parser: Speed up parsing octal literals (#4258)
(overlookmotel)
- a7b328c parser: Faster parsing decimal numbers (#4257) (overlookmotel)
- f9d3f2e semantic: Inline ast record functions (#4272) (overlookmotel)
- 8fad7db semantic: Reduce `AstNodeId` to `u32` (#4264) (overlookmotel)
- 23743db semantic: Do not record ast nodes for cfg if cfg disabled
(#4263) (overlookmotel)
- da69076 semantic: Reduce overhead of cfg recording ast nodes (#4262)
(overlookmotel)
- cb15303 semantic: Reduce memory copies (#4216) (overlookmotel)
- ef4c1f4 semantic: Reduce lookups (#4214) (overlookmotel)
- f23e54f semantic: Recycle unresolved references hash maps (#4213)
(overlookmotel)
- 2602ce2 semantic: Reuse existing map of unresolved refs (#4206)
(lucab)

### Refactor

- 2c7bb9f ast: Pass final `ScopeFlags` into `visit_function` (#4283)
(overlookmotel)
- 3e099fe ast: Move `enter_scope` after `visit_binding_identifier`
(#4246) (Dunqing)
- aab7aaa ast/visit: Fire node events as the outermost one. (#4203)
(rzvxa)
- d1c4be0 codegen: Clean up annotation_comment (Boshen)
- 06197b8 codegen: Separate tests (Boshen)
- aa22073 codegen: Improve print API (#4196) (Boshen)
- c5731a5 semantic: Remove defunct code setting ScopeFlags twice (#4286)
(overlookmotel)
- 16698bc semantic: Move function/class-specific code into specific
visitors (#4278) (overlookmotel)
- ee16668 semantic: Rename function param (#4277) (overlookmotel)
- 25f0771 semantic: Alter syntax of `control_flow!` macro (#4275)
(overlookmotel)
- 639fd48 semantic: Comment why extra CFG enabled check (#4274)
(overlookmotel)
- c418bf5 semantic: Directly record `current_node_id` when adding a
scope (#4265) (Dunqing)
- ace4f1f semantic: Update the order of `visit_function` and `Visit`
fields in the builder to be consistent (#4248) (Dunqing)
- 8bfeabf semantic: Simplify adding `SymbolFlags::Export` (#4249)
(Dunqing)
- dc2b3c4 semantic: Add strict mode in scope flags for class definitions
(#4156) (Dunqing)
- 81ed588 semantic: Convert scope fields to IndexVecs (#4208) (lucab)
- bbe5ded semantic: Set `current_scope_id` to `scope_id` in
`enter_scope` (#4193) (Dunqing)
- 7f1addd semantic: Correct scope in CatchClause (#4192) (Dunqing)
- fc0b17d syntax: Turn the `AstNodeId::dummy` into a constant field.
(#4308) (rzvxa)
- a197e01 transformer/typescript: Remove unnecessary code (#4321)
(Dunqing)
- 1458d81 visit: Add `#[inline]` to empty functions (#4330)
(overlookmotel)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-merge Merge with Graphite Merge Queue A-parser Area - Parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants