Remove BodyAndCache
#71044
Remove BodyAndCache
#71044
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try |
|
Awaiting bors try build completion |
|
⌛ Trying commit b117f26adbb92bd2114aa7438a1558c673060d5c with merge 0f456c9e7ee3bfbf52e3ad1a857d8c051754aa9b... |
|
@bors try- |
ecstatic-morse
force-pushed
the
body-predecessor-cache
branch
from
b117f26
to
92e2549
Compare
|
@bors try |
|
⌛ Trying commit 92e25499c622d84b35b7c66c0e0ee08253f459b5 with merge 198316ac3da8706e1bc304042111f0e4dc0b9e43... |
|
☀️ Try build successful - checks-azure |
|
Queued 198316ac3da8706e1bc304042111f0e4dc0b9e43 with parent e82734e, future comparison URL. |
|
Finished benchmarking try commit 198316ac3da8706e1bc304042111f0e4dc0b9e43, comparison URL. |
ecstatic-morse
force-pushed
the
body-predecessor-cache
branch
from
92e2549
to
7c19a98
Compare
|
@bors try |
|
Awaiting bors try build completion |
|
⌛ Trying commit 7c19a989c3531b977b09780636a3e287e3562abc with merge abfaf6666e74e407628239bae61a008f6c8c3457... |
|
☀️ Try build successful - checks-azure |
|
Queued abfaf6666e74e407628239bae61a008f6c8c3457 with parent 4d1fbac, future comparison URL. |
|
Will try another perf run after #71208. |
ecstatic-morse
force-pushed
the
body-predecessor-cache
branch
2 times, most recently
from
fd83ac3
to
84d67f9
Compare
|
#71208 had no effect on performance for |
|
@bors try @rust-timer queue |
...with a single one to `predecessors`. `predecessors_for` requires taking the lock/incrementing the `RefCell` once each call.
ecstatic-morse
force-pushed
the
body-predecessor-cache
branch
from
84d67f9
to
4e7469e
Compare
|
Failed spuriously while installing @bors r=oli-obk |
|
📌 Commit 4e7469e has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
|
📣 Toolstate changed by #71044! Tested on commit db9b05a. 💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq). |
Tested on commit rust-lang/rust@db9b05a. Direct link to PR: <rust-lang/rust#71044> 💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq). 💔 clippy-driver on linux: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
Rustup "Remove `BodyAndCache`" cc rust-lang/rust#71044 changelog: none
…he-arc, r=nikomatsakis Don't hold the predecessor cache lock longer than necessary rust-lang#71044 returns a `LockGuard` with the predecessor cache to callers of `Body::predecessors`. As a result, the lock around the predecessor cache could be held for an arbitrarily long time. This PR uses reference counting for ownership of the predecessor cache, meaning the lock is only ever held within `PredecessorCache::compute`. Checking this API for potential sources of deadlock is much easier now, since we no longer have to consider its consumers, only its internals. This required removing `predecessors_for`, since there is no equivalent to `LockGuard::map` for `Arc` and `Rc`. I believe this could be emulated with `owning_ref::{Arc,Rc}Ref`, but I don't think it's necessary. Also, we continue to return an opaque type from `Body::predecessors` with the lifetime of the `Body`, not `'static`. This depends on rust-lang#71044. Only the last two commits are new. r? @nikomatsakis
…he-arc, r=nikomatsakis Don't hold the predecessor cache lock longer than necessary rust-lang#71044 returns a `LockGuard` with the predecessor cache to callers of `Body::predecessors`. As a result, the lock around the predecessor cache could be held for an arbitrarily long time. This PR uses reference counting for ownership of the predecessor cache, meaning the lock is only ever held within `PredecessorCache::compute`. Checking this API for potential sources of deadlock is much easier now, since we no longer have to consider its consumers, only its internals. This required removing `predecessors_for`, since there is no equivalent to `LockGuard::map` for `Arc` and `Rc`. I believe this could be emulated with `owning_ref::{Arc,Rc}Ref`, but I don't think it's necessary. Also, we continue to return an opaque type from `Body::predecessors` with the lifetime of the `Body`, not `'static`. This depends on rust-lang#71044. Only the last two commits are new. r? @nikomatsakis
…he-arc, r=nikomatsakis Don't hold the predecessor cache lock longer than necessary rust-lang#71044 returns a `LockGuard` with the predecessor cache to callers of `Body::predecessors`. As a result, the lock around the predecessor cache could be held for an arbitrarily long time. This PR uses reference counting for ownership of the predecessor cache, meaning the lock is only ever held within `PredecessorCache::compute`. Checking this API for potential sources of deadlock is much easier now, since we no longer have to consider its consumers, only its internals. This required removing `predecessors_for`, since there is no equivalent to `LockGuard::map` for `Arc` and `Rc`. I believe this could be emulated with `owning_ref::{Arc,Rc}Ref`, but I don't think it's necessary. Also, we continue to return an opaque type from `Body::predecessors` with the lifetime of the `Body`, not `'static`. This depends on rust-lang#71044. Only the last two commits are new. r? @nikomatsakis
...returning to the original approach using interior mutability within
Body. This simplifies the API at the cost of some uncontended mutex locks when the parallel compiler is enabled.The current API requires you to either have a mutable reference to
Body(&mut BodyAndCache), or to compute the predecessor graph ahead of time by creating aReadOnlyBodyAndCache. This is not a good fit for, e.g., the dataflow framework, which