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(semantic): faster search for leading comments #4140

Merged
merged 2 commits into from
Jul 10, 2024
Merged

perf(semantic): faster search for leading comments #4140

merged 2 commits into from
Jul 10, 2024

Conversation

Boshen
Copy link
Member

@Boshen Boshen commented Jul 9, 2024

fixes #4114

@leaysgur This is my wild guess, I need a second pairs of eyes to make sure this is correct.

@Boshen Boshen requested a review from leaysgur July 9, 2024 15:15
@github-actions github-actions bot added the A-semantic Area - Semantic label Jul 9, 2024
@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Jul 9, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #4140 will improve performances by ×60

Comparing perf-jsdoc (b0e3070) with main (bdcc298)

Summary

⚡ 4 improvements
✅ 25 untouched benchmarks

Benchmarks breakdown

Benchmark main perf-jsdoc Change
⚡ semantic[antd.js] 12,823.1 ms 212.3 ms ×60
⚡ semantic[cal.com.tsx] 138.8 ms 66.6 ms ×2.1
⚡ semantic[checker.ts] 2,687.5 ms 136.5 ms ×20
⚡ semantic[pdf.mjs] 90.7 ms 33.2 ms ×2.7

@rzvxa
Copy link
Contributor

rzvxa commented Jul 9, 2024

CodSpeed Performance Report

Merging #4140 will improve performances by ×59

Comparing perf-jsdoc (f5f64db) with main (714bf1d)

Summary

⚡ 4 improvements ✅ 25 untouched benchmarks

Benchmarks breakdown

Benchmark main perf-jsdoc Change
⚡ semantic[antd.js] 12,416.8 ms 211.3 ms ×59
⚡ semantic[cal.com.tsx] 135.6 ms 63.2 ms ×2.1
⚡ semantic[checker.ts] 2,601.6 ms 135.7 ms ×19
⚡ semantic[pdf.mjs] 89 ms 33.3 ms ×2.7

WTF

@Boshen Boshen mentioned this pull request Jul 9, 2024
Merged
@Boshen
Copy link
Member Author

Boshen commented Jul 9, 2024

CodSpeed Performance Report

Merging #4140 will improve performances by ×59

Comparing perf-jsdoc (f5f64db) with main (714bf1d)

Summary

⚡ 4 improvements ✅ 25 untouched benchmarks

Benchmarks breakdown

Benchmark main perf-jsdoc Change
⚡ semantic[antd.js] 12,416.8 ms 211.3 ms ×59
⚡ semantic[cal.com.tsx] 135.6 ms 63.2 ms ×2.1
⚡ semantic[checker.ts] 2,601.6 ms 135.7 ms ×19
⚡ semantic[pdf.mjs] 89 ms 33.3 ms ×2.7

WTF

Just a normal day in oxc ...

@Boshen Boshen changed the title perf(semantic): improve performance of search for leading comments perf(semantic): faster search for leading comments Jul 9, 2024
@leaysgur
Copy link
Contributor

leaysgur commented Jul 9, 2024

This is my wild guess, I need a second pairs of eyes to make sure this is correct.

LGTM!

https://github.com/oxc-project/oxc/pull/4140/files#diff-79d6155264aad0a2e14ac20754321839cbd6ef3735ab83c7238d4e2d613d45efL120-L121

Even though the code seems familiar, why didn't it work that time... 😅

@Boshen Boshen merged commit fca9706 into main Jul 10, 2024
26 checks passed
@Boshen Boshen deleted the perf-jsdoc branch July 10, 2024 01:54
@overlookmotel overlookmotel mentioned this pull request Jul 10, 2024
Closed
@github-actions github-actions bot mentioned this pull request Jul 11, 2024
Merged
Boshen added a commit that referenced this pull request Jul 11, 2024
@github-actions @Boshen
Release crates v0.20.0 (#4189)
2a169d1 
## [0.20.0] - 2024-07-11

- 5731e39 ast: [**BREAKING**] Store span details inside comment struct
(#4132) (Luca Bruno)

### Features

- 67fe75e ast, ast_codegen: Pass the `scope_id` to the `enter_scope`
event. (#4168) (rzvxa)
- 54cd04a minifier: Implement dce with var hoisting (#4160) (Boshen)
- 44a894a minifier: Implement return statement dce (#4155) (Boshen)
- 725571a napi/transformer: Add `jsx` option to force parsing with jsx
(#4133) (Boshen)

### Bug Fixes

- 48947a2 ast: Put `decorators` before everything else. (#4143) (rzvxa)
- 7a059ab cfg: Double resolution of labeled statements. (#4177) (rzvxa)
- 4a656c3 lexer: Incorrect lexing of large hex/octal/binary literals
(#4072) (DonIsaac)
- 28eeee0 parser: Fix asi error diagnostic pointing at invalid text
causing crash (#4163) (Boshen)

### Performance

- ddfa343 diagnostic: Use `Cow<'static, str>` over `String` (#4175)
(DonIsaac)
- 2203143 semantic: Store unresolved refs in a stack (#4162) (lucab)
- fca9706 semantic: Faster search for leading comments (#4140) (Boshen)

### Documentation

- bdcc298 ast: Update the note regarding the `ast_codegen` markers.
(#4149) (rzvxa)

### Refactor

- 03ad1e3 semantic: Tweak comment argument type (#4157) (lucab)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucab lucab lucab left review comments

@leaysgur leaysgur Awaiting requested review from leaysgur

Assignees
No one assigned
Labels
A-semantic Area - Semantic
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

semantic: huge performance regression when jsdoc parsing is enabled
4 participants
@Boshen @rzvxa @leaysgur @lucab