Make incremental compilation thread-safe by Zoxc · Pull Request #49732 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation42 Commits3 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Zoxc

@TimNN

Your PR failed on Travis. 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:00:46] configure: rust.quiet-tests     := True
---
[00:07:31] error[E0433]: failed to resolve. Use of undeclared type or module `Lock`
[00:07:31]     --> librustc/ty/context.rs:1815:24
[00:07:31]      |
[00:07:31] 1815 |                 task: &Lock::new(OpenTask::Ignore),
[00:07:31]      |                        ^^^^ Use of undeclared type or module `Lock`
[00:07:31]
[00:07:31] error[E0412]: cannot find type `Lock` in this scope
[00:07:31]     --> librustc/ty/context.rs:1728:23
[00:07:31]      |
[00:07:31] 1728 |         pub task: &'a Lock<OpenTask>,
[00:07:31]      |                       ^^^^ not found in this scope
[00:07:31] help: possible candidates are found in other modules, you can import them into scope
---
[00:08:10]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name rustc librustc/lib.rs --color always --error-format json --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 -C metadata=b061046bb39a474d -C extra-filename=-b061046bb39a474d --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps --extern tempdir=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libtempdir-728fcbc86125f341.rlib --extern graphviz=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libgraphviz-01fc77d929e8bdd6.so --extern backtrace=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libbacktrace-a7efadae36f54a3c.rlib --extern jobserver=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libjobserver-3c385cb05f9c08fa.rlib --extern flate2=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libflate2-0249ed74490015f9.rlib --extern byteorder=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libbyteorder-f04ca02424a68c4e.rlib --extern rustc_apfloat=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_apfloat-b23276248c684a7d.rlib --extern rustc_errors=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_errors-d823127e323fa243.so --extern syntax=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax-67023beb7edf290b.so --extern rustc_back=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_back-c188df8980be19c3.so --extern fmt_macros=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libfmt_macros-5106459147843225.so --extern syntax_pos=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax_pos-069d1433ad059ff1.so --extern lazy_static=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/liblazy_static-3c1d922a28d91411.rlib --extern arena=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libarena-2e4ce590106ede04.so --extern bitflags=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libbitflags-e01ce88b04783514.rlib --extern rustc_const_math=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_const_math-2cbdd37d611532dc.so --extern proc_macro=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libproc_macro-414d7e12269f768c.so --extern log=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/liblog-7eebd272275b441b.rlib --extern rustc_data_structures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_data_structures-1e725c22de2888b8.so --extern serialize=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-f27fded04dd31022.so --extern serialize=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-f27fded04dd31022.rlib -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/backtrace-sys-751752dec0960570/out/.libs -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/miniz-sys-07c608814ba8d6ba/out` (exit code: 101)
[00:08:10] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:08:10] expected success, got: exit code: 101
[00:08:10] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1085:9
[00:08:10] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:08:10] travis_fold🔚stage0-rustc
[00:08:10] travis_time🔚stage0-rustc:start=1523026037209707648,finish=1523026246774016214,duration=209564308566
[00:08:10] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:08:10] Build completed unsuccessfully in 0:03:44
[00:08:10] Makefile:28: recipe for target 'all' failed
[00:08:10] make: *** [all] Error 1
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time🔚390c1fc4:start=1523026247263406863,finish=1523026247271580522,duration=8173659
travis_fold🔚after_failure.2
travis_fold:start:after_failure.3
travis_time:start:0163f860
$ 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🔚0163f860:start=1523026247279544820,finish=1523026247286696899,duration=7152079
travis_fold🔚after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04ed6ee0
$ dmesg | grep -i kill
[   10.726120] init: failsafe main process (1094) 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.

@bors

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

@Zoxc Zoxc changed the title[WIP] Make incremental compilation thread-safe Make incremental compilation thread-safe

Apr 11, 2018

Zoxc

if did_allocation {
// Only the thread which did the allocation emits the error messages
// FIXME: Ensure that these are printed before returning for all threads.

Choose a reason for hiding this comment

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

I left this FIXME as future work and added it to #48685

@Zoxc

I've cleaned this up a bit now.

@Zoxc

@bors

bors added a commit that referenced this pull request

Apr 12, 2018

@bors

@bors

@Zoxc

@Mark-Simulacrum

@pietroalbini

@Mark-Simulacrum

We are a little behind on collecting master commits due to an error on the collector, but should get to this soon-ish. Please ping me if I haven't said anything in a day or two...

@pietroalbini

@Mark-Simulacrum

Hm, looks like we're still backlogged. I've made the commit we need to compare against have a higher priority.

@Zoxc

@Mark-Simulacrum

ignore nll-check for now, I think it was semi-broken because we removed the -Znll flag. I've since fixed, that, we can rerun try to be certain...

michaelwoerister

bug!("forbidden edge {:?} -> {:?} created",
source,
target)
ty::tls::with_context_opt(|icx

Choose a reason for hiding this comment

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

In theory all reads should always come from the same thread. I wonder if we could get rid of this lock.

Choose a reason for hiding this comment

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

That's not true. Queries may use multiple threads internally and use queries there.

Choose a reason for hiding this comment

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

A particular query invocation should only ever write to one vec and sub-invocations must get their own vec, so the same vec should never be shared among two invocations. Otherwise we would have a logical bug.

Choose a reason for hiding this comment

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

A particular query invocation can write to that one vec using multiple threads.

Choose a reason for hiding this comment

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

Oh, you mean when the query would use parallelism internally?

Choose a reason for hiding this comment

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

Yeah.

Choose a reason for hiding this comment

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

As a minor optimization, the Lock could be moved into OpenTask maybe. Then there'd at least be no locking for Ignore and EvalAlways.

michaelwoerister
/// When we load, there may be `.o` files, cached mir, or other such
/// things available to us. If we find that they are not dirty, we
/// load the path to the file storing those work-products here into
/// this map. We can later look for and extract that data.
previous_work_products: RefCell<FxHashMap<WorkProductId, WorkProduct>>,
previous_work_products: RwLock<FxHashMap<WorkProductId, WorkProduct>>,

Choose a reason for hiding this comment

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

We should refactor this to not be mutable at all.

michaelwoerister
/// Work-products that we generate in this run.
work_products: RefCell<FxHashMap<WorkProductId, WorkProduct>>,
work_products: RwLock<FxHashMap<WorkProductId, WorkProduct>>,

Choose a reason for hiding this comment

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

This could be moved out of the dep-graph completely.

Choose a reason for hiding this comment

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

Moved where?

Choose a reason for hiding this comment

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

If I didn't overlook something then this list could be threaded through as a return value: save_trans_partition() -> copy_module_artifacts_into_incr_comp_cache() -> OngoingCrateTranslation::join().
Doesn't need to happen in this PR though.

michaelwoerister

arg: A,
no_tcx: bool,
task: fn(C, A) -> R,
get_task: fn(DepNode) -> OpenTask,

Choose a reason for hiding this comment

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

Could you rename this to create_task?

Choose a reason for hiding this comment

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

Done.

michaelwoerister

no_tcx: bool,
task: fn(C, A) -> R,
get_task: fn(DepNode) -> OpenTask,
get_index: fn(&Lock,

Choose a reason for hiding this comment

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

And this to finish_task_and_alloc_depnode? (I take full responsibility for this name :))

Choose a reason for hiding this comment

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

Or finish_task_and_alloc_depnode, which would be more consistent with the other method names in this module.

Choose a reason for hiding this comment

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

Done.

michaelwoerister

pub(super) fn pop_task(&mut self, key: DepNode) -> DepNodeIndex {
let popped_node = self.task_stack.pop().unwrap();
pub(super) fn complete_task(&mut self, key: DepNode, task: OpenTask) -> DepNodeIndex {

Choose a reason for hiding this comment

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

Now that raii.rs is gone this can probably be private.

Choose a reason for hiding this comment

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

Done.

Choose a reason for hiding this comment

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

👍

@rust-highfive

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:01:36] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/getopts/0.2.17/download`, got 503
[00:01:36] error: unable to get packages from source
[00:01:36] 
[00:01:36] Caused by:
[00:01:36]   failed to get 200 response from `https://crates.io/api/v1/crates/getopts/0.2.17/download`, got 503
[00:01:36] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:36] Build completed unsuccessfully in 0:00:47
[00:01:36] make: *** [prepare] Error 1
[00:01:36] Makefile:81: recipe for target 'prepare' failed
[00:01:36]  Downloading serde_json v1.0.15
[00:01:36] warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/serde_json/1.0.15/download`, got 503
[00:01:37] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/serde_json/1.0.15/download`, got 503
[00:01:37] error: unable to get packages from source
[00:01:37] error: unable to get packages from source
[00:01:37] 
[00:01:37] Caused by:
[00:01:37]   failed to get 200 response from `https://crates.io/api/v1/crates/serde_json/1.0.15/download`, got 503
[00:01:37] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:37] Build completed unsuccessfully in 0:00:00
[00:01:37] Makefile:81: recipe for target 'prepare' failed
[00:01:37] make: *** [prepare] Error 1
[00:01:37]  Downloading num_cpus v1.8.0
[00:01:37]  Downloading num_cpus v1.8.0
[00:01:37] warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/num_cpus/1.8.0/download`, got 503
[00:01:37] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/num_cpus/1.8.0/download`, got 503
[00:01:38] error: unable to get packages from source
[00:01:38] Caused by:
[00:01:38] Caused by:
[00:01:38]   failed to get 200 response from `https://crates.io/api/v1/crates/num_cpus/1.8.0/download`, got 503
[00:01:38] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:38] Build completed unsuccessfully in 0:00:00
[00:01:38] Makefile:81: recipe for target 'prepare' failed
[00:01:38] make: *** [prepare] Error 1
[00:01:38]  Downloading serde_derive v1.0.40
[00:01:38] warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/serde_derive/1.0.40/download`, got 503
[00:01:38] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/serde_derive/1.0.40/download`, got 503
[00:01:38] error: unable to get packages from source
[00:01:38] error: unable to get packages from source
[00:01:38] 
[00:01:38] Caused by:
[00:01:38]   failed to get 200 response from `https://crates.io/api/v1/crates/serde_derive/1.0.40/download`, got 503
[00:01:38] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:38] Build completed unsuccessfully in 0:00:00
[00:01:38] make: *** [prepare] Error 1
[00:01:38] Makefile:81: recipe for target 'prepare' failed
[00:01:39]  Downloading lazy_static v0.2.11
[00:01:39] warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/lazy_static/0.2.11/download`, got 503
[00:01:39] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/lazy_static/0.2.11/download`, got 503
[00:01:39] error: unable to get packages from source
[00:01:39] error: unable to get packages from source
[00:01:39] 
[00:01:39] Caused by:
[00:01:39]   failed to get 200 response from `https://crates.io/api/v1/crates/lazy_static/0.2.11/download`, got 503
[00:01:39] failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml --locked
[00:01:39] Build completed unsuccessfully in 0:00:00
[00:01:39] make: *** [prepare] Error 1
[00:01:39] Makefile:81: recipe for target 'prepare' failed
[00:01:39] The command has failed after 5 attempts.

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 1.
travis_time:start:2b0b4cb4
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_fold:start:after_failure.4
travis_time:start:145e86e8
$ dmesg | grep -i kill
[   10.436234] init: failsafe main process (1094) killed by TERM signal
[   41.766948] init: plymouth-upstart-bridge main process (510) killed by TERM signal
travis_fold🔚after_failure.4

Done. Your build exited with 1.

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)

@Zoxc

I added refactoring previous_work_products and `work_products to #48685, since they didn't seem trivial to do. I just ended up looking at how to refactor the HIR map...

@michaelwoerister

I added refactoring previous_work_products and `work_products to #48685, since they didn't seem trivial to do. I just ended up looking at how to refactor the HIR map...

👍

@Zoxc

@Zoxc

@michaelwoerister

OK, I think this looks great! @Zoxc, I'll let you decide if you want to move the Lock into OpenTask or keep things as they are now. r=me when you are done.

@Zoxc

@Zoxc

@bors

📌 Commit 3f802ee has been approved by michaelwoerister

@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 25, 2018

@bors

bors added a commit that referenced this pull request

Apr 26, 2018

@bors

@bors

@bors bors mentioned this pull request

Apr 26, 2018

@Zoxc Zoxc deleted the sync-dep-graph branch

April 26, 2018 09:27

Labels

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.