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 GlobalCtxt implement Sync #45912

Closed
wants to merge 24 commits into from
Closed

Make GlobalCtxt implement Sync #45912

wants to merge 24 commits into from

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Nov 10, 2017

This PR makes the GlobalCtxt implement Sync, which is the first step towards making rustc thread safe so we can execute queries in multiple threads.

The changes here are mostly mechanical where Cell is replaced by LockCell, Rc is replaced by Lrc and RefCell is replaced by either Lock or RwLock. Some thread local variables are now declared with rustc_global!, which indicates that they are intended to be global to a rustc session, however they aren't changed to global variables since rustdoc has multiple threads with rustc sessions in them.

Here is a list of the larger changes:

  • src/librustc_data_structures/sync.rs has the new types other items related to thread safety.
  • src/libarena/lib.rs is changed so that there's a lock around each arena if thread safety is enabled.
  • There's a new [rust] experimental_parallel_queries entry for config.toml which enables thread safety. It is disabled by default.
  • -Z query_threads allows you to set the number of thread used to compute queries, although it is current ignored.
  • It uses my fork of owning_ref, so we can erase a OwningRef into a thread-safe type.
  • GlobalArenas and DroplessArena are combined into AllArenas which is the type drivers shoud use. This refactoring makes it easier to create thread-local arenas in the future.
  • A SameThread type is introduces in src/librustc_trans/metadata.rs which asserts that operations on the inner type all happen on the same thread. This is used to make ArchiveRO and ObjectFile Send + Sync so we can make the [u8] metadata reference into them Send + Sync.
  • ThreadCtxt is introduced. TyCtxt now points to it instead of GlobalCtxt. ThreadCtxt contains a references to a GlobalCtxt and contains the parts of the `GlobalCtxt which has to be thread local.
  • mk_attr_id in src/libsyntax/attr.rs is changed to use an AtomicUsize.
  • The err_count field of Handler in src/librustc_errors/lib.rs is now a AtomicUsize instead of Cell<usize>.

One thing to note is that Lock and RwLock implements Clone, since there's code which relies the Clone impl for RefCell. There's some risk of deadlocks due to that. Replacing RefCell with Lock also risks deadlocks, since RefCell is re-entrant for read-locks, while Lock is not.

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 10, 2017

@bors try

@bors
Copy link
Contributor

bors commented Nov 10, 2017

@Zoxc: 🔑 Insufficient privileges: and not in try users

@shepmaster shepmaster added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 10, 2017
@shepmaster
Copy link
Member

@bors try

@bors
Copy link
Contributor

bors commented Nov 10, 2017

⌛ Trying commit ad7b865 with merge d2cb23b...

bors added a commit that referenced this pull request Nov 10, 2017
WIP: Playing with adding thread safety to GlobalCtxt
@shepmaster
Copy link
Member

Assigning to...

r? @pnkfelix

@bors
Copy link
Contributor

bors commented Nov 10, 2017

💔 Test failed - status-travis

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 10, 2017

What is everyone opinion on introducing locks and Arcs in rustc in an effort to make GlobalCtxt thread safe? We could redirect these types to RefCell, Rc, etc. in production builds to avoid paying the overhead for these thread safe types.

cc @rust-lang/compiler

@eddyb
Copy link
Member

eddyb commented Nov 10, 2017

I would rather not introduce #[cfg]s into rustc like that, FWIW.

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 10, 2017

I'd like the ability to turn on and off thread safety so we can easily measure the overhead of it. That is going to require some #[cfg]s at least.

@michaelwoerister
Copy link
Member

If the #[cfg]s are very localized and they are clearly experimental, I'm not strictly against adding some. However, in such a case the effort of keeping such a patch around locally would not be too big either.

@Zoxc Zoxc force-pushed the par-query branch 2 times, most recently from 7cb9fc5 to 7c78120 Compare November 14, 2017 07:13
@bors
Copy link
Contributor

bors commented Nov 14, 2017

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

@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 14, 2017

@bors try

@bors
Copy link
Contributor

bors commented Nov 14, 2017

⌛ Trying commit 5ca1224 with merge 6849630...

bors added a commit that referenced this pull request Nov 14, 2017
WIP: Playing with adding thread safety to GlobalCtxt
@Zoxc
Copy link
Contributor Author

Zoxc commented Nov 14, 2017

@Mark-Simulacrum Can I get a perf run when bors completes?

@bors
Copy link
Contributor

bors commented Nov 14, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Nov 14, 2017

@Zoxc please break every tool in src/tools/toolstate.toml and try again.

@@ -7,3 +7,6 @@ version = "0.0.0"
name = "arena"
path = "lib.rs"
crate-type = ["dylib"]

[dependencies]
rustc_data_structures = { path = "../librustc_data_structures" }
Copy link
Member

@bjorn3 bjorn3 Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing trailing newline


[dependencies.parking_lot]
version = "0.5"
features = ["nightly", "owning_ref"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline

fn clone(&self) -> Self {
Lock::new(self.borrow().clone())
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline

@Zoxc Zoxc force-pushed the par-query branch 3 times, most recently from 7a7b7e8 to edcc51a Compare November 16, 2017 19:29
@bors
Copy link
Contributor

bors commented Nov 16, 2017

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

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.
@bors
Copy link
Contributor

bors commented Dec 12, 2017

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

bors added a commit that referenced this pull request Dec 17, 2017
Add sync module to rustc_data_structures

This PR is split out from #45912, since github apparently can't handle such large PRs.

r? @arielb1
@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 17, 2017

This is being split into smaller PRs.

@Zoxc Zoxc closed this Dec 17, 2017
bors added a commit that referenced this pull request Dec 22, 2017
Work towards thread safety in rustc

This PR is split out from #45912. It contains changes which do not require the `sync` module.
bors added a commit that referenced this pull request Feb 9, 2018
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.
bors added a commit that referenced this pull request Apr 19, 2018
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants