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

Make parallelism optional for singlepass backend #2262

Merged
merged 2 commits into from
Apr 26, 2021
Merged

Make parallelism optional for singlepass backend #2262

merged 2 commits into from
Apr 26, 2021

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Apr 26, 2021

Description

closes #2261

Review

  • Add a short description of the change to the CHANGELOG.md file

@matklad matklad changed the title || Make parallelism optional for singlepass backend Apr 26, 2021
@matklad
Copy link
Contributor Author

matklad commented Apr 26, 2021

Something I haven't thought about before writing the code: should this be a compile time or runtime feature? I just brute-codded compile-time, I guess we can add runtime feature if there's a need for that as well, and, even with a runtime feature, ability to not compile rayon is a plus.

In other words, the current solution is what works today, and it's forward compatible with the potentially more general future soln.

@syrusakbary
Copy link
Member

This looks great! It should be good to merge, before then can we make this strategy also available in the other two compilers so they all have the same feature?

Thanks!

@matklad
Copy link
Contributor Author

matklad commented Apr 26, 2021

before then can we make this strategy also available in the other two compilers so they all have the same feature?

I'd rather keep this PR scoped to singlepass backend, as that's my immediate need (and I feel that singlethreadedness makes the most sense for the singlepass) :-)

@nlewycky
Copy link
Contributor

(and I feel that singlethreadedness makes the most sense for the singlepass) :-)

FWIW, I work on all three compilers and I keep a local patch around to make them single-threaded because otherwise I can't debug a bug in any of them. My patch is much worse than yours though, I was adding a Mutex inside the parallel part. Just throwing in my 2¢ that it makes sense for the other compilers too.

@syrusakbary
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Apr 26, 2021
2262: Make parallelism optional for singlepass backend r=syrusakbary a=matklad

# Description

closes #2261

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@bors
Copy link
Contributor

bors bot commented Apr 26, 2021

Timed out.

@syrusakbary
Copy link
Member

Merging manually, tests are passing

@syrusakbary syrusakbary merged commit a00f3ee into wasmerio:master Apr 26, 2021
@matklad matklad deleted the || branch April 28, 2021 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider making rayon dependency of singlepass backend optional
3 participants