Skip to content

feat(parser)!: Build ModuleRecord directly in parser#7546

Merged
graphite-app[bot] merged 1 commit intomainfrom
11-29-feat_parser_build_modulerecord_
Nov 29, 2024
Merged

feat(parser)!: Build ModuleRecord directly in parser#7546
graphite-app[bot] merged 1 commit intomainfrom
11-29-feat_parser_build_modulerecord_

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Nov 29, 2024

This has the benefit of:

  • expose dynamic import / import meta info from parser
  • 1 less ast shallow in semantic builder
  • no ast walk in oxc's module lexer
  • some more benefits coming soon

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 29, 2024

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

Add the label “0-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 A-linter Area - Linter A-parser Area - Parser A-semantic Area - Semantic C-enhancement Category - New feature or request labels Nov 29, 2024
@Boshen Boshen changed the title feat(parser)!: build ModuleRecord feat(parser)!: Build ModuleRecord directly in parser Nov 29, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 29, 2024

CodSpeed Performance Report

Merging #7546 will degrade performances by 57.1%

Comparing 11-29-feat_parser_build_modulerecord_ (8a788b8) with main (b24beeb)

Summary

⚡ 3 improvements
❌ 3 regressions
✅ 24 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main 11-29-feat_parser_build_modulerecord_ Change
isolated-declarations[vue-id.ts] 405.9 ms 946 ms -57.1%
linter[cal.com.tsx] 1.1 s 1.1 s +4.83%
parser[RadixUIAdoptionSection.jsx] 78.3 µs 98.1 µs -20.14%
parser[cal.com.tsx] 24.7 ms 32.7 ms -24.25%
semantic[RadixUIAdoptionSection.jsx] 97.8 µs 79 µs +23.78%
semantic[cal.com.tsx] 36.9 ms 28.5 ms +29.27%

@Boshen
Copy link
Member Author

Boshen commented Nov 29, 2024

CodSpeed Performance Report

Merging #7546 will degrade performances by 57.09%

Comparing 11-29-feat_parser_build_modulerecord_ (575f984) with main (b24beeb)

Summary

⚡ 3 improvements ❌ 3 regressions ✅ 24 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main 11-29-feat_parser_build_modulerecord_ Change
isolated-declarations[vue-id.ts] 405.9 ms 945.9 ms -57.09%
linter[cal.com.tsx] 1.1 s 1.1 s +4.83%
parser[RadixUIAdoptionSection.jsx] 78.3 µs 98 µs -20.06%
parser[cal.com.tsx] 24.7 ms 32.7 ms -24.27%
semantic[RadixUIAdoptionSection.jsx] 97.8 µs 79.7 µs +22.66%
semantic[cal.com.tsx] 36.9 ms 29.1 ms +26.85%

Note:

  • Performance is not changed for the compiler pipeline
  • I will improvement performance later
  • These benchmarks are not representative for real world files, real world files has only a fiew import / export statements.

@Boshen Boshen force-pushed the 11-29-feat_parser_build_modulerecord_ branch from 575f984 to 487554a Compare November 29, 2024 14:45
@Boshen Boshen marked this pull request as ready for review November 29, 2024 14:47
@Boshen Boshen requested a review from Dunqing as a code owner November 29, 2024 14:47
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Nov 29, 2024
Copy link
Member Author

Boshen commented Nov 29, 2024

Merge activity

  • Nov 29, 9:47 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Nov 29, 9:50 AM EST: A user added this pull request to the Graphite merge queue.
  • Nov 29, 9:55 AM EST: A user merged this pull request with the Graphite merge queue.

This has the benefit of:

* expose dynamic import / import meta info from parser
* 1 less ast shallow in semantic builder
* no ast walk in oxc's module lexer
* some more benefits coming soon
@Boshen Boshen force-pushed the 11-29-feat_parser_build_modulerecord_ branch from 487554a to 8a788b8 Compare November 29, 2024 14:50
@graphite-app graphite-app bot merged commit 8a788b8 into main Nov 29, 2024
@graphite-app graphite-app bot deleted the 11-29-feat_parser_build_modulerecord_ branch November 29, 2024 14:55
This was referenced Dec 1, 2024
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-linter Area - Linter A-parser Area - Parser A-semantic Area - Semantic C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant