Skip to content

Make SelectionCache and EvaluationCache thread-safe#49834

Merged
bors merged 1 commit into
rust-lang:masterfrom
Zoxc:sync-trait-cache
May 9, 2018
Merged

Make SelectionCache and EvaluationCache thread-safe#49834
bors merged 1 commit into
rust-lang:masterfrom
Zoxc:sync-trait-cache

Conversation

@Zoxc

@Zoxc Zoxc commented Apr 10, 2018

Copy link
Copy Markdown
Contributor

Split out from #49558

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 10, 2018
@TimNN

TimNN commented Apr 10, 2018

Copy link
Copy Markdown
Contributor

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.
Resolving deltas: 100% (613496/613496), completed with 4872 local objects.
---
[00:00:41] configure: rust.quiet-tests     := True
---
[00:37:20] thread 'main' panicked at 'assertion failed: *old == value', librustc_data_structures/sync.rs:236:42

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)

Comment thread src/librustc/traits/select.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given the comment above, should this also use insert_same()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The inserted type doesn't implement Eq.

@nikomatsakis

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Apr 11, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 9d48917 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 11, 2018
@Zoxc

Zoxc commented Apr 11, 2018

Copy link
Copy Markdown
Contributor Author

This doesn't work since insert_evaluation_cache overwrites values.

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 11, 2018
@bors

bors commented Apr 12, 2018

Copy link
Copy Markdown
Collaborator

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

@emilyalbini emilyalbini added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 16, 2018
@Zoxc Zoxc force-pushed the sync-trait-cache branch from 9d48917 to 4ea5db3 Compare April 17, 2018 15:15
@rust-highfive

Copy link
Copy Markdown
Contributor

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.
[00:01:07] configure: rust.quiet-tests     := True
---
[00:42:45] thread 'main' panicked at 'assertion failed: *old == value', librustc_data_structures/sync.rs:241:42
---
[00:43:33] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "doc" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--no-deps" "-p" "alloc" "-p" "core" "-p" "std" "-p" "std_unicode"
[00:43:33] expected success, got: exit code: 101
[00:43:33]
[00:43:33]
[00:43:33] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap doc
[00:43:33] Build completed unsuccessfully in 0:05:47
[00:43:33] make: *** [all] Error 1
[00:43:33] Makefile:28: recipe for target 'all' failed
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:2705b080:start=1523980841568033483,finish=1523980841575288725,duration=7255242
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:294834ea
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:294834ea:start=1523980841581578671,finish=1523980841588508966,duration=6930295
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:09c17180
$ dmesg | grep -i kill
[   10.544864] init: failsafe main process (1093) killed by TERM signal

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)

@nikomatsakis

Copy link
Copy Markdown
Contributor

@Zoxc to be honest I didn't think too hard here-- and I still haven't. =) Can you spell out the problem, and do you have any ideas for how to solve?

@Zoxc

Zoxc commented Apr 25, 2018

Copy link
Copy Markdown
Contributor Author

@nikomatsakis I though maps in SelectionCache and EvaluationCache would have values which are deterministically based on their keys. That is not true for EvaluationCache at least, since it will change the value of aty::PolyTraitRef<'tcx> key to a different WithDepNode<EvaluationResult>. I'm wondering if this is expected behavior, and if so, how to make these caches thread-safe?

@shepmaster

Copy link
Copy Markdown
Member

Ping from triage, @Zoxc and @nikomatsakis ! Will you have time to make progress on this in the near future?

@Zoxc Zoxc force-pushed the sync-trait-cache branch from 4ea5db3 to 2004110 Compare May 1, 2018 14:37
@rust-highfive

Copy link
Copy Markdown
Contributor

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.
[00:37:03] Documenting book redirect pages (x86_64-unknown-linux-gnu)
[00:37:04] Documenting stage2 std (x86_64-unknown-linux-gnu)
[00:37:04]    Compiling core v0.0.0 (file:///checkout/src/libcore)
[00:37:04]  Documenting core v0.0.0 (file:///checkout/src/libcore)
[00:37:17] overwriting key/trait_ref: Binder(<isize as marker::Freeze>) with EvaluationResult WithDepNode { dep_node: 150890, cached_value: EvaluatedToOk }
[00:37:17]  old value: WithDepNode { dep_node: 150889, cached_value: EvaluatedToOk }
[00:37:17] thread 'main' panicked at 'assertion failed: *old == value', librustc_data_structures/sync.rs:241:42
[00:37:17] 
[00:37:17] error: internal compiler error: unexpected panic
[00:37:17] 
[00:37:17] 
[00:37:17] note: the compiler unexpectedly panicked. this is a bug.
[00:37:17] 
[00:37:17] note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
[00:37:17] 
[00:37:17] note: rustc 1.27.0-dev running on x86_64-unknown-linux-gnu
[00:37:17] 
[00:37:17] note: compiler flags: -Z force-unstable-if-unmarked -C debug-assertions=off -C overflow-checks=on -C incremental -C prefer-dynamic -C debug-assertions=y -C link-args=-Wl,-rpath,$ORIGIN/../lib --crate-type lib
[00:37:17] 
[00:37:17] note: some of the compiler flags provided by cargo are hidden
[00:37:17] error: Could not compile `core`.
[00:37:17] warning: build failed, waiting for other jobs to finish...
[00:37:49] warning: [1] cannot be resolved, ignoring it...
[00:37:49] 
---
[00:37:51] 
[00:37:55] error: build failed
[00:37:55] 
[00:37:55] 
[00:37:55] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "doc" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--no-deps" "-p" "alloc" "-p" "core" "-p" "std" "-p" "std_unicode"
[00:37:55] 
[00:37:55] 
[00:37:55] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap doc
[00:37:55] Build completed unsuccessfully in 0:05:15
[00:37:55] Build completed unsuccessfully in 0:05:15
[00:37:55] Makefile:28: recipe for target 'all' failed
[00:37:55] make: *** [all] Error 1
2174820 ./obj
2174788 ./obj/build
1566936 ./obj/build/x86_64-unknown-linux-gnu
725072 ./src
---
149120 ./src/llvm-emscripten/test
144660 ./obj/build/bootstrap/debug/incremental
128084 ./obj/build/x86_64-unknown-linux-gnu/stage1-std
123692 ./obj/build/bootstrap/debug/incremental/bootstrap-1wl4zjaz72e5d
123688 ./obj/build/bootstrap/debug/incremental/bootstrap-1wl4zjaz72e5d/s-f0msmdn1je-1ia77dq-6gvofocobazn
121820 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release
105360 ./obj/build/x86_64-unknown-linux-gnu/crate-docs
103452 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu
103448 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release

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)

@shepmaster

Copy link
Copy Markdown
Member

Ping from triage, @Zoxc ! You have test failures.

@Zoxc Zoxc force-pushed the sync-trait-cache branch from 2004110 to f741ee1 Compare May 7, 2018 15:13
@Zoxc

Zoxc commented May 7, 2018

Copy link
Copy Markdown
Contributor Author

I've created #50507 for the bug which overwrites the cache and added a fixme to use insert_same when that is fixed.

@nikomatsakis

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented May 9, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit f741ee1 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 9, 2018
@bors

bors commented May 9, 2018

Copy link
Copy Markdown
Collaborator

⌛ Testing commit f741ee1 with merge e5f80f2...

bors added a commit that referenced this pull request May 9, 2018
Make SelectionCache and EvaluationCache thread-safe

Split out from #49558

r? @nikomatsakis
@bors

bors commented May 9, 2018

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing e5f80f2 to master...

@bors bors merged commit f741ee1 into rust-lang:master May 9, 2018
@Zoxc Zoxc deleted the sync-trait-cache branch May 9, 2018 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

8 participants