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

WIP: Parallelize passes using rayon #46564

Closed
wants to merge 17 commits into from
Closed

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Dec 7, 2017

This builds on #46193 and #45912 and actually makes code run in parallel. This is not quite ready yet since rustc is not yet completely thread safe. It also uses a rough fork of rayon which uses fibers/stackful coroutines.

@Zoxc Zoxc added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Dec 7, 2017
@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 7, 2017

@bors try

@bors
Copy link
Contributor

bors commented Dec 7, 2017

⌛ Trying commit c3e21ab7c517fd303077036df3b211cd28bf7b0e with merge 4f1b9ac80d435420ca47d60344b959db6e8e4f51...

@bors
Copy link
Contributor

bors commented Dec 7, 2017

💔 Test failed - status-travis

@bors
Copy link
Contributor

bors commented Dec 7, 2017

☔ The latest upstream changes (presumably #46509) made this pull request unmergeable. Please resolve the merge conflicts.

@Zoxc Zoxc force-pushed the rayon-queries branch 2 times, most recently from 9d8a65b to 47f3a8f Compare December 8, 2017 21:20
@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 10, 2017

@bors try

@bors
Copy link
Contributor

bors commented Dec 10, 2017

⌛ Trying commit 086a5fb with merge 463435e9a0879e3db40c9ed64f12a146f4937b16...

@bors
Copy link
Contributor

bors commented Dec 10, 2017

💔 Test failed - status-travis

@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 10, 2017

@bors try

@bors
Copy link
Contributor

bors commented Dec 10, 2017

⌛ Trying commit 26a0515 with merge 8ad7c34...

bors added a commit that referenced this pull request Dec 10, 2017
WIP: Parallelize passes using rayon

This builds on #46193 and #45912 and actually makes code run in parallel. This is not quite ready yet since `rustc` is not yet completely thread safe. It also uses a rough fork of rayon which uses fibers/stackful coroutines.
@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 10, 2017

@Mark-Simulacrum I'd also like a perf run here when the try build is complete

@bors
Copy link
Contributor

bors commented Dec 10, 2017

☀️ Test successful - status-travis
State: approved= try=True

@Mark-Simulacrum
Copy link
Member

Perf run queued.

@mark-i-m
Copy link
Member

What are the specs of the machine on which this is running? Does it have the cores to actually take advantage of this PR?

@Mark-Simulacrum
Copy link
Member

In general, yes, it should. I don't have the exact specs right now, though -- it has ~8 cores IIRC.

@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 12, 2017

@Mark-Simulacrum I'd also like to know the specs of the machine and also I'd like to know how it is configured; does the CPU run with constant frequency? are there any background services running?

Note that the try build is with thread safety and the rayon thread pool disabled.

@mark-i-m
Copy link
Member

Note that the try build is with thread safety and the rayon thread pool disabled.

Ah, that makes more sense... We were measuring the overhead of thread-safety.

@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 12, 2017

Lets try that again.

Note that the try build is with (thread safety and the rayon thread pool) disabled.

It is just measuring what the cost of merging this with everything disabled is.

@bors
Copy link
Contributor

bors commented Dec 14, 2017

☔ The latest upstream changes (presumably #46335) made this pull request unmergeable. Please resolve the merge conflicts.

@Zoxc Zoxc force-pushed the rayon-queries branch from a14eda1 to be5b986 Compare June 5, 2018 21:40
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Jun 7, 2018

☔ The latest upstream changes (presumably #50699) made this pull request unmergeable. Please resolve the merge conflicts.

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2018
@mrhota
Copy link
Contributor

mrhota commented Jun 19, 2018

@Zoxc what's the story with this PR? is it dead?

@Zoxc
Copy link
Contributor Author

Zoxc commented Jun 21, 2018

An alternative approach which doesn't use fibers is merged in master. This still parallelizes more code though. That should be cherry picked out and analyzed (it may not be beneficial to parallelize all passes).

@mikumikuchan
Copy link

sorry I mis clicked "approve" and I don't know why some how it succeeded. please discard my approval.

@mark-i-m
Copy link
Member

@mikumikuchan No worries! That's the Github code review feature. The rust project instead uses @bors commands, which have an ACL (which I'm not on :P ).

@TimNN
Copy link
Contributor

TimNN commented Jul 3, 2018

Ping from triage, @Zoxc: What is the status of this?

@TimNN
Copy link
Contributor

TimNN commented Jul 10, 2018

Ping from triage, @Zoxc: we haven't heard from you for a while, so we are closing this PR. Feel free to re-open it in the future. Thanks for your contribution!

@TimNN TimNN closed this Jul 10, 2018
@TimNN TimNN added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.