perf.rust-lang.org test for syntex-syntax messed up

[nikomatsakis]

For some reason, we are no longer achieving 100% re-use if you cargo clean, though we were at one point. Right now I see 3 out of 50 modules re-used :(. The output is all about cross-compilation metadata still. An excerpt:

module WorkProduct(WorkProductId("syntex_syntax-attr")) is dirty because MetaData("syntex_errors/1fec731a39244a3925064ac8bb26867a2dcbcd286e07e4d86a47d1a1fa7cc43a::Handler[0]") changed or was removed
module WorkProduct(WorkProductId("core-option")) is dirty because MetaData("syntex_errors/1fec731a39244a3925064ac8bb26867a2dcbcd286e07e4d86a47d1a1fa7cc43a::Handler[0]") changed or was removed

cc @michaelwoerister

[nagisa]

We probably should test that reuse between cargo build → cargo clean && cargo build with some simple crates is always 100% and gate landing stuff on that?

[nikomatsakis]

Added a fix to #35854. I did not add a test though -- the problem @nagisa is that, afaict, this is all pretty non-deterministic. Certainly the bug I fixed, which seems to fix the problem, is pre-existing (in fact, I had noticed it already); I think I was just getting lucky when I saw full re-use. I am reluctant to add a non-deterministic test into our code base -- I'd want to find a way to make it always fail or not fail. (Otherwise, random PRs get blamed for failures not related to them.)

[nikomatsakis]

@nagisa hmm, that said, maybe the thing to do is to add some tests to perf.rust-lang.org, where we are testing the incremental compilation state! I think perf is better suited to these sorts of regressions.

[nikomatsakis]

OK, I decided what I said doesn't make sense. We should add tests for this, of course...I'll try to isolate down the problem in any case.

[nagisa]

How come cargo build && cargo clean && cargo build is non-deterministic given an isolated directory for incremental compilation used only by the test? AFAIR incremental comp does not use any sort of randomness?

Either way, something in perf sounds very useful, even if we end up having a test for a simple crate in our own test suite. It would be nice to see reuse graphs.

[eddyb]

I believe @infinity0 was talking about finding non-determinism in metadata encoding - could this be related?

[infinity0]

@nagisa @eddyb I'm not familiar enough with cargo to tell if this is related, but the non-determinism issue is #34902 if you want more details - including an actual diff of the rust.metadata.bin from libcore.

[nikomatsakis]

The problem is fixed now, afaik, so I don't think it's related to that nondeterminism.

[nikomatsakis]

This graph seems to suggest not fully fixed. I am investigating, but the output I see from running locally is as follows:

   Compiling rustc-serialize v0.3.19
   Compiling unicode-xid v0.0.3
   Compiling winapi v0.2.8
   Compiling libc v0.2.15
   Compiling winapi-build v0.1.1
   Compiling log v0.3.6
   Compiling bitflags v0.5.0
module WorkProduct(WorkProductId("build")) is dirty because Hir("build/53e418df8e77e265cb0c53e178822c1585c354caa695a2291038d0676202c62d::link[0]") changed or was removed
module WorkProduct(WorkProductId("log")) is dirty because Hir("log/a7fe374870229691fc926e4549c402007b4ff66243de367bd63cca93f203bafc::{{impl}}[34]") changed or was removed
incremental: re-using 1 out of 1 modules
incremental: re-using 3 out of 3 modules
incremental: re-using 0 out of 1 modules
module WorkProduct(WorkProductId("rustc_serialize-hex")) is dirty because Hir("rustc_serialize/9787f582344afeaad12e400392a9cfdc7cdf27af9a17cff0ce298226b3ea1256::base64[0]::{{impl}}[12]") changed or was removed
module WorkProduct(WorkProductId("rustc_serialize-json")) is dirty because Hir("rustc_serialize/9787f582344afeaad12e400392a9cfdc7cdf27af9a17cff0ce298226b3ea1256::serialize[0]::Decoder[0]") changed or was removed
module WorkProduct(WorkProductId("rustc_serialize-base64")) is dirty because Hir("rustc_serialize/9787f582344afeaad12e400392a9cfdc7cdf27af9a17cff0ce298226b3ea1256::hex[0]::{{impl}}[3]") changed or was removed
incremental: re-using 0 out of 1 modules

[nikomatsakis]

Better dump:

   Compiling winapi v0.2.8
     Running `rustc /home/nmatsakis/.cargo/registry/src/github.com-1ecc6299db9ec823/winapi-0.2.8/src/lib.rs --crate-name winapi --crate-type lib -g -C metadata=0889532d327ff4e2 -C extra-filename=-0889532d327ff4e2 --out-dir /home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --emit=dep-info,link -L dependency=/home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --cap-lints allow -Z incremental=incr -Z incremental-info`
incremental: re-using 1 out of 1 modules
   Compiling unicode-xid v0.0.3
     Running `rustc /home/nmatsakis/.cargo/registry/src/github.com-1ecc6299db9ec823/unicode-xid-0.0.3/src/lib.rs --crate-name unicode_xid --crate-type lib -g --cfg feature=\"default\" -C metadata=6613e5c0eda58b13 -C extra-filename=-6613e5c0eda58b13 --out-dir /home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --emit=dep-info,link -L dependency=/home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --cap-lints allow -Z incremental=incr -Z incremental-info`
incremental: re-using 3 out of 3 modules
   Compiling winapi-build v0.1.1
     Running `rustc /home/nmatsakis/.cargo/registry/src/github.com-1ecc6299db9ec823/winapi-build-0.1.1/src/lib.rs --crate-name build --crate-type lib -g -C metadata=493a7b0628804707 -C extra-filename=-493a7b0628804707 --out-dir /home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --emit=dep-info,link -L dependency=/home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --cap-lints allow -Z incremental=incr -Z incremental-info`
module WorkProduct(WorkProductId("build")) is dirty because Hir("build/53e418df8e77e265cb0c53e178822c1585c354caa695a2291038d0676202c62d::link[0]") changed or was removed
incremental: re-using 0 out of 1 modules
   Compiling rustc-serialize v0.3.19
     Running `rustc /home/nmatsakis/.cargo/registry/src/github.com-1ecc6299db9ec823/rustc-serialize-0.3.19/src/lib.rs --crate-name rustc_serialize --crate-type lib -g -C metadata=3561541d79c18212 -C extra-filename=-3561541d79c18212 --out-dir /home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --emit=dep-info,link -L dependency=/home/nmatsakis/versioned/rustc-benchmarks/syntex-0.42.2-incr-clean/target/debug/deps --cap-lints allow -Z incremental=incr -Z incremental-info`
module WorkProduct(WorkProductId("rustc_serialize-hex")) is dirty because Hir("rustc_serialize/9787f582344afeaad12e400392a9cfdc7cdf27af9a17cff0ce298226b3ea1256::base64[0]::{{impl}}[12]") changed or was removed
module WorkProduct(WorkProductId("rustc_serialize-json")) is dirty because Hir("rustc_serialize/9787f582344afeaad12e400392a9cfdc7cdf27af9a17cff0ce298226b3ea1256::serialize[0]::Decoder[0]") changed or was removed
module WorkProduct(WorkProductId("rustc_serialize-base64")) is dirty because Hir("rustc_serialize/9787f582344afeaad12e400392a9cfdc7cdf27af9a17cff0ce298226b3ea1256::serialize[0]::Decoder[0]") changed or was removed

[michaelwoerister]

#36025 fixes quite a few things in the ICH.

[nikomatsakis]

Indeed testing with a build #36025 seems to achieve full re-use.

[jonas-schievink]

This seems to be fixed on the newest nightly

[nikomatsakis]

It doesn't seem to be fixed on perf.rust-lang.org though. The syntex-0.42.2-incr-clean build time hasn't improved. Might be a problem with the test itself.

e.g., see this graph

[nikomatsakis]

(This btw is believed to be a problem with the perf.rust-lang.org setup.)

[Mark-Simulacrum]

The graph looks flat for the entire time, as far as I can tell. @nnethercote: You've worked closely with most of the benchmarks as far as I know, can you comment on this?

[nnethercote]

I'm not at all familiar with incremental compilation and I'm not sure what question you're asking me.

The only suggestion I can make is to ensure that the script is building the benchmark in exactly the way you expect. Here's the output of make clean ; make ; make touch ; make on my machine, which is what compare.py and process.sh basically do (albeit in slightly different ways). Note that make clean deletes the incr/ directory -- is that what you want?

$ make clean
rm -rf incr/*
cargo clean

$ make
RUSTFLAGS="-Z incremental=incr" \
    cargo rustc -p syntex_syntax --  -Z incremental-info
   Compiling log v0.3.6
   Compiling winapi-build v0.1.1
   Compiling libc v0.2.15
   Compiling bitflags v0.5.0
   Compiling rustc-serialize v0.3.19
   Compiling winapi v0.2.8
   Compiling unicode-xid v0.0.3
   Compiling kernel32-sys v0.2.2
   Compiling term v0.4.4
   Compiling syntex_pos v0.42.0
   Compiling syntex_errors v0.42.0
   Compiling syntex_syntax v0.42.0
incremental: re-using 0 out of 50 modules

$ make touch
cargo clean -p syntex_syntax

$ make
RUSTFLAGS="-Z incremental=incr" \
    cargo rustc -p syntex_syntax --  -Z incremental-info
   Compiling syntex_syntax v0.42.0
incr. comp. session directory: 103 files hard-linked
incr. comp. session directory: 0 files copied
incremental: re-using 50 out of 50 modules

[nikomatsakis]

Seems like we expect to pass -Z time-passes somehow, right? that must be getting lost now?

[nnethercote]

Seems like we expect to pass -Z time-passes somehow, right? that must be getting lost now?

You have to do that separately by setting CARGO_RUSTC_OPTS. That's what process.sh does.

[nikomatsakis]

@nnethercote ok I mean that output looks fine. My point is just that I think that process.sh uses the timing from -Z time-passes to figure out how long things took, so maybe the zeroes result from us failing to propagate some environment variables or something?

[michaelwoerister]

I was able to reproduce the strangely fast re-compilation by using an old Cargo version. Maybe it doesn't understand the -p flag? @nrc, can you try updating cargo on the test machine?

[nikomatsakis]

I think my pending PRs will sidestep any problems and fix this infrastructure, which was quite flawed (I didn't understand how the tests worked):

• do not fail when the incremental messages appear rust-lang-nursery/rustc-timing-scripts#1
• extend process.sh to support patch stages rust-lang-deprecated/rustc-benchmarks#22

[nikomatsakis]

I'm going to close this issue, basically because all of perf is kind of messed up right now, though @Mark-Simulacrum has been making great progress here.