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

feat(estree/compat): Remove dependency on rayon #9393

Merged
merged 10 commits into from
Aug 12, 2024

Conversation

dsherret
Copy link
Contributor

@dsherret dsherret commented Aug 7, 2024

Description: Adds a concurrent feature to swc_estree_compat so this can be used without rayon.

Related issue (if exists): None

@dsherret dsherret requested a review from a team as a code owner August 7, 2024 13:03
Copy link

changeset-bot bot commented Aug 7, 2024

🦋 Changeset detected

Latest commit: 214f7d5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@kdy1
Copy link
Member

kdy1 commented Aug 7, 2024

CI failed

Copy link

codspeed-hq bot commented Aug 7, 2024

CodSpeed Performance Report

Merging #9393 will degrade performances by 4.29%

Comparing dsherret:swc_estree_compat (214f7d5) with main (1bf467d)

Summary

⚡ 3 improvements
❌ 1 regressions
✅ 174 untouched benchmarks

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

Benchmarks breakdown

Benchmark main dsherret:swc_estree_compat Change
es/full/bugs-1 558.8 µs 536.4 µs +4.18%
common/allocator/alloc/cached-no-scope/1000000 108.6 ms 113.5 ms -4.29%
parse_and_babelify_jquery 113.9 ms 110.6 ms +3.01%
parse_and_babelify_underscore 15.7 ms 15.2 ms +3.51%

@dsherret
Copy link
Contributor Author

dsherret commented Aug 7, 2024

It seems to be an existing issue on main that the CI is now running because of the new concurrent feature.

% git switch main
Switched to branch 'main'
% cargo test -p swc_estree_compat --bench babelify                      
    Blocking waiting for file lock on build directory

...

Gnuplot not found, using plotters backend
Testing babelify-only
Success

Testing parse_and_babelify_angular
Success

Testing parse_and_babelify_backbone
Success

Testing parse_and_babelify_jquery
Success

Testing parse_and_babelify_jquery_mobile

thread '<unknown>' has overflowed its stack
fatal runtime error: stack overflow
error: test failed, to rerun pass `-p swc_estree_compat --bench babelify`

Caused by:
  process didn't exit successfully: `/Users/david/dev/swc/target/debug/deps/babelify-b3c33d587b84ea1e` (signal: 6, SIGABRT: process abort signal)

@kdy1
Copy link
Member

kdy1 commented Aug 7, 2024

The main branch works, and I think it works because rayon creates another stack. You may use stacker to adjust stack size dynamically.

@dsherret
Copy link
Contributor Author

dsherret commented Aug 7, 2024

The main branch works

Are you sure? It's failing for me.

% cargo test -p swc_estree_compat --bench babelify                      
...
Testing parse_and_babelify_jquery_mobile

thread '<unknown>' has overflowed its stack
fatal runtime error: stack overflow
error: test failed, to rerun pass `-p swc_estree_compat --bench babelify`

Caused by:
  process didn't exit successfully: `/Users/david/dev/swc/target/debug/deps/babelify-b3c33d587b84ea1e` (signal: 6, SIGABRT: process abort signal)
swc % git status                                                            
On branch main

@kdy1
Copy link
Member

kdy1 commented Aug 7, 2024

CI for main branch passes

@kdy1
Copy link
Member

kdy1 commented Aug 7, 2024

@kdy1
Copy link
Member

kdy1 commented Aug 7, 2024

The default stack size of linux is different from the one of macos

@dsherret
Copy link
Contributor Author

dsherret commented Aug 7, 2024

I'm just saying it's an existing issue on main (not CI). I can look into fixing it tomorrow, though I'm not sure exactly how to do that (I'll look at how stacker is used elsewhere).

@kdy1
Copy link
Member

kdy1 commented Aug 9, 2024

I'll take on this on Monday

@kdy1 kdy1 self-assigned this Aug 9, 2024
@kdy1 kdy1 added this to the Planned milestone Aug 9, 2024
@dsherret
Copy link
Contributor Author

dsherret commented Aug 9, 2024

Ok thanks! I tried a bit and couldn't figure it out 😅

@kdy1 kdy1 changed the title feat(estree/compat): Add concurrent feature feat(estree/compat): Remove dependency on rayon Aug 12, 2024
@kdy1 kdy1 enabled auto-merge (squash) August 12, 2024 02:42
@kdy1 kdy1 merged commit 34d1b27 into swc-project:main Aug 12, 2024
154 checks passed
@dsherret dsherret deleted the swc_estree_compat branch August 12, 2024 15:14
@kdy1 kdy1 modified the milestones: Planned, v1.7.11 Aug 14, 2024
@swc-project swc-project locked as resolved and limited conversation to collaborators Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants