Skip to content

feat(oxc_semantic): make Scoping cloneable#9953

Merged
graphite-app[bot] merged 1 commit intomainfrom
03-21-feat_make_scoping_cloneable
Mar 23, 2025
Merged

feat(oxc_semantic): make Scoping cloneable#9953
graphite-app[bot] merged 1 commit intomainfrom
03-21-feat_make_scoping_cloneable

Conversation

@IWANABETHATGUY
Copy link
Contributor

No description provided.

Copy link
Contributor Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added the A-semantic Area - Semantic label Mar 21, 2025
@IWANABETHATGUY IWANABETHATGUY changed the title feat: make Scoping cloneable feat: make Scoping cloneable Mar 21, 2025
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review March 21, 2025 12:26
@IWANABETHATGUY IWANABETHATGUY changed the title feat: make Scoping cloneable feat(oxc_semantic): make Scoping cloneable Mar 21, 2025
@github-actions github-actions bot added the C-enhancement Category - New feature or request label Mar 21, 2025
@IWANABETHATGUY
Copy link
Contributor Author

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 21, 2025

CodSpeed Instrumentation Performance Report

Merging #9953 will not alter performance

Comparing 03-21-feat_make_scoping_cloneable (b4f3d00) with main (ea3de06)

Summary

✅ 33 untouched benchmarks

@overlookmotel
Copy link
Member

overlookmotel commented Mar 21, 2025

Do you really need to clone Scoping? That is a very expensive operation. (That's just a question, not a criticism)

Also, I think Allocator::used_bytes is the method you need to get an appropriate capacity. Allocator::capacity will likely be excessive. See the doc comments on the 2 methods to understand the difference.

I designed Allocator::used_bytes just for you!

@IWANABETHATGUY
Copy link
Contributor Author

Do you really need to clone Scoping? That is a very expensive operation. (That's just a question, not a criticism)

Also, I think Allocator::used_bytes is the method you need to get an appropriate capacity. Allocator::capacity will likely be excessive. See the doc comments on the 2 methods to understand the difference.

I designed Allocator::used_bytes just for you!

Yeah, it is used for incremental build, since rolldown mutate the scoping, we need to clone the scoping at the first time to make sure it could be used in second round, I will try to eliminate it in the future, but for now there is no alternative way.

Copy link
Member

Boshen commented Mar 23, 2025

Merge activity

  • Mar 23, 5:09 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Mar 23, 5:09 AM EDT: A user added this pull request to the Graphite merge queue.
  • Mar 23, 9:11 AM UTC: The Graphite merge queue couldn't merge this PR because it was in draft mode.
  • Mar 23, 5:12 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Mar 23, 5:20 AM EDT: A user added this pull request to the Graphite merge queue.
  • Mar 23, 5:32 AM EDT: A user merged this pull request with the Graphite merge queue.

@graphite-app graphite-app bot force-pushed the 03-21-feat_make_scoping_cloneable branch from 91eb5cb to 6d59e93 Compare March 23, 2025 09:09
graphite-app bot pushed a commit that referenced this pull request Mar 23, 2025
@Boshen Boshen marked this pull request as draft March 23, 2025 09:11
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 23, 2025
@Boshen Boshen marked this pull request as ready for review March 23, 2025 09:12
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Mar 23, 2025
@graphite-app graphite-app bot force-pushed the 03-21-feat_make_scoping_cloneable branch from 0a2bfcc to b4f3d00 Compare March 23, 2025 09:23
@graphite-app graphite-app bot merged commit b4f3d00 into main Mar 23, 2025
25 checks passed
@graphite-app graphite-app bot deleted the 03-21-feat_make_scoping_cloneable branch March 23, 2025 09:32
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-semantic Area - Semantic C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants