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): do not copy comments #4067

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

overlookmotel
Copy link
Collaborator

@overlookmotel overlookmotel commented Jul 6, 2024

Follow-on from #4045. .from_iter() copies the Vec of comments into another Vec before converting to a boxed slice. This copy is unnecessary - just convert direct.

Copy link

graphite-app bot commented Jul 6, 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.

@github-actions github-actions bot added the A-parser Area - Parser label Jul 6, 2024
Copy link
Collaborator Author

overlookmotel commented Jul 6, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @overlookmotel and the rest of your teammates on Graphite Graphite

@overlookmotel overlookmotel marked this pull request as ready for review July 6, 2024 09:49
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jul 6, 2024
Copy link

graphite-app bot commented Jul 6, 2024

Merge activity

  • Jul 6, 5:55 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 6, 5:55 AM EDT: Boshen added this pull request to the Graphite merge queue.
  • Jul 6, 6:01 AM EDT: Boshen merged this pull request with the Graphite merge queue.

Follow-on from #4045. `.from_iter()` copies the `Vec` of comments into another `Vec` before converting to a boxed slice. This copy is unnecessary - just convert direct.
@Boshen Boshen force-pushed the 07-06-refactor_ast_inline_trivial_functions_and_shorten_code branch from 9c11c85 to 8fa98e0 Compare July 6, 2024 09:56
@Boshen Boshen force-pushed the 07-06-perf_parser_do_not_copy_comments branch from ac3b1cd to 7fe2a2f Compare July 6, 2024 09:57
@Boshen Boshen changed the base branch from 07-06-refactor_ast_inline_trivial_functions_and_shorten_code to main July 6, 2024 10:00
@graphite-app graphite-app bot merged commit 7fe2a2f into main Jul 6, 2024
23 checks passed
@graphite-app graphite-app bot deleted the 07-06-perf_parser_do_not_copy_comments branch July 6, 2024 10:01
@lucab
Copy link
Collaborator

lucab commented Jul 6, 2024

Thanks, I didn't realize there was an extra Vec involved in the parser side as I was mostly touching the AST crate.

@github-actions github-actions bot mentioned this pull request Jul 6, 2024
Boshen added a commit that referenced this pull request Jul 6, 2024
## [0.17.1] - 2024-07-06

### Bug Fixes

- aa585d3 ast_codegen, ast: Visit `ExpressionArrayElement` as
`Expression`. (#4061) (rzvxa)
- 564a75a codegen: Missing TypeParameters in TSConstructSignature
(#4063) (michaelm)
- adee728 isolated_declarations: Don't report an error for parameters if
they are ObjectPattern or ArrayPattern with an explicit type (#4065)
(michaelm)
- 1b8f208 isolated_declarations: Correct emit for private static methods
(#4064) (michaelm)
- 719fb96 minifier: Omit dce `undefined` which can be a shadowed
variable (#4073) (Boshen)
- 150f4d9 napi/transform: Display error with spanned messages (Boshen)

### Performance

- 7fe2a2f parser: Do not copy comments (#4067) (overlookmotel)

### Refactor

- 8fa98e0 ast: Inline trivial functions and shorten code (#4066)
(overlookmotel)
- 65aee19 isolated-declarations: Reorganize scope tree (#4070) (Luca
Bruno)

Co-authored-by: Boshen <Boshen@users.noreply.github.com>
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.

3 participants