incr.comp.: Deduplicate some DepNodes and introduce anonymous DepNodes by michaelwoerister · Pull Request #43028 · rust-lang/rust

michaelwoerister · 2017-07-03T15:35:11Z

This is a parallel PR to the pending #42769. It implements most of what is possible in terms of DepNode re-opening without having anonymous DepNodes yet (#42298).

r? @nikomatsakis

nikomatsakis · 2017-07-07T13:03:00Z

 }
 }
+
+impl_stable_hash_for!(enum ty::fast_reject::SimplifiedType {


Why SimplifiedType? Seems ok though

Because it's used in the query key of relevant_trait_impls.

nikomatsakis · 2017-07-07T16:26:31Z

@bors r+

bors · 2017-07-07T16:26:32Z

📌 Commit 4ad0609 has been approved by nikomatsakis

bors · 2017-07-08T04:22:45Z

🔒 Merge conflict

michaelwoerister · 2017-07-10T11:28:11Z

I pushed 3 more commits to this branch. They introduce anonymous DepNodes, move dep-graph management completely to the main thread and introduce caching of DepNodes in queries.

nikomatsakis · 2017-07-10T16:32:45Z

- fn new(v: usize) -> IdIndex {
+impl DepNodeIndex {
+
+ pub const INVALID: DepNodeIndex = DepNodeIndex { index: ::std::u32::MAX };


Nit: this is fine for now, but it'd be nicer to use Nonzero so that we can (efficiently) use Option<DepNodeIndex>, I suspect.

nikomatsakis · 2017-07-10T16:37:03Z

So travis is failing because the log is too big. The log seems to include some DEBUG output, which is weird?

...
[00:48:05] DEBUG:rustc::session::filesearch: rejected /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/issue-2735-2.rs-stage2-x86_64-unknown-linux-gnu.stamp
[00:48:05] DEBUG:rustc::session::filesearch: testing /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/issue-20763-1.rs-stage2-x86_64-unknown-linux-gnu.stamp
[00:48:05] DEBUG:rustc::session::filesearch: rejected /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/issue-20763-1.rs-stage2-x86_64-unknown-linux-gnu.stamp
[00:48:05] DEBUG:rustc::session::filesearch: testing /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/issue-10638.out
[00:48:05] DEBUG:rustc::session::filesearch: rejected /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/issue-10638.out
[00:48:05] DEBUG:rustc::session::filesearch: testing /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/purity-infer.err
[00:48:
The log length has exceeded the limit of 4 MB (this usually means that the test suite is raising the same exception over and over).

nikomatsakis · 2017-07-10T16:37:34Z

That said, the test that is failing probably enables the log, so i think the real problem is the test is failing for some reason:

[00:48:05] ---- [run-pass] run-pass/rustc-rust-log.rs stdout ----
[00:48:05] [00:48:05] error: compilation failed!
[00:48:05] status: exit code: 101
[00:48:05] command: /checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc /checkout/src/test/run-pass/rustc-rust-log.rs -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass --target=x86_64-unknown-linux-gnu -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/rustc-rust-log.stage2-x86_64-unknown-linux-gnu.run-pass.libaux -C prefer-dynamic -o /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/rustc-rust-log.stage2-x86_64-unknown-linux-gnu -Crpath -O -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers
[00:48:05] stdout:
[00:48:05] ------------------------------------------
[00:48:05] [00:48:05] ------------------------------------------
[00:48:05] stderr:
[00:48:05] ------------------------------------------

kennytm · 2017-07-10T16:48:21Z

@nikomatsakis The logs are part of the test's stderr.

// rustc-env:RUST_LOG=debugfnmain(){}

nikomatsakis · 2017-07-10T16:48:54Z

@kennytm right, this is why I said I suspect the real problem is that the test fails in the first place.

kennytm · 2017-07-10T17:20:23Z

Tried locally but not reproducible. Spurious I guess.

Compiler-test should probably filter the output so only the top and bottom 16 KB of text are printed out.

nikomatsakis · 2017-07-10T17:43:34Z

@kennytm

Spurious I guess.

Huh. Can we test that hypothesis? I guess we can r+... maybe if we cycle travis?

kennytm · 2017-07-10T18:21:14Z

@nikomatsakis r+ is one way. Owners of this repo could also go to Travis and restart the job, without accepting the PR.

Closing the PR and reopening it would also trigger a build.

nikomatsakis · 2017-07-10T19:33:19Z

@kennytm I am able to reproduce locally

nikomatsakis · 2017-07-10T19:50:28Z

@bors r+

bors · 2017-07-10T19:50:29Z

📌 Commit 4f1f671 has been approved by nikomatsakis

michaelwoerister · 2017-07-10T20:03:21Z

Thanks for fixing!

bors · 2017-07-10T21:18:20Z

⌛ Testing commit 4f1f671 with merge 9228d23...

@nikomatsakis

incr.comp.: Deduplicate some DepNodes and introduce anonymous DepNodes This is a parallel PR to the pending #42769. It implements most of what is possible in terms of DepNode re-opening without having anonymous DepNodes yet (#42298). r? @nikomatsakis

bors · 2017-07-11T00:01:40Z

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

Mark-Simulacrum · 2017-07-11T18:17:41Z

This produced a 20 second reduction in the regex expand benchmark: https://perf.rust-lang.org/graphs.html?start=2017-07-07T00%3A00%3A00%2B00%3A00&end=2017-07-12T00%3A00%3A00%2B00%3A00&crates=%7B%22list%22%3A%22List%22%2C%22content%22%3A%5B%22regex-0.1.80%40050-expand%22%5D%7D&phases=%7B%22list%22%3A%22All%22%2C%22content%22%3Anull%7D&group_by=crate&type=time, as well as saving 57 MB of memory.

michaelwoerister · 2017-07-12T09:30:13Z

@Mark-Simulacrum, nice! Thanks for making me aware of that. Overall I expected this patch to make things 10% slower, but I seem to have tested only the worst case. Or difference gets more pronounced when turning off debug assertions (which I mostly do for performance testing).

alexcrichton · 2017-07-20T20:48:33Z

It looks like this may have regressed another benchmark though :(

Was that expected though?

michaelwoerister · 2017-07-21T08:30:52Z

Yes, that was expected. I'm surprised it doesn't show up in other benchmarks more. This should get better again when the current (humongous) refactoring is done.

rust-highfive assigned nikomatsakis Jul 3, 2017

michaelwoerister force-pushed the dedup-dep-nodes branch from 2de78df to 4ad0609Compare July 4, 2017 13:39

michaelwoerister mentioned this pull request Jul 4, 2017
incr.comp.: Introduce the concept of anonymous DepNodes and use them for trait selection #42769
Closed

aidanhs added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 5, 2017

nikomatsakis reviewed Jul 7, 2017
View reviewed changes

michaelwoerister added 9 commits July 10, 2017 12:20

Split DepNode::ItemSignature into non-overlapping variants.
2557800

incr.comp.: Improve debug output for work products.
ca0a403

Add StableHash implementation for ty::Instance.
9808661

Allow 'tcx in define_dep_nodes! and deduplicate some DepNodes.
e00bec2

Deduplicate DepNode::ConstEval()
f8b774f

Fix some tidy errors.
13e87d1

incr.comp.: Manage dependency graph on main thread.
089860b

incr.comp.: Introduce the concept of anonymous DepNodes.
0363a23

incr.comp.: Cache DepNodes with corresponding query results.
6d049fb

michaelwoerister force-pushed the dedup-dep-nodes branch from 4ad0609 to 6d049fbCompare July 10, 2017 10:27

michaelwoerister changed the title ~~incr.comp.: Deduplicate some DepNodes~~incr.comp.: Deduplicate some DepNodes and introduce anonymous DepNodesJul 10, 2017

nikomatsakis approved these changes Jul 10, 2017
View reviewed changes

don't panic in dep_node_debug_str if self.data is None
4f1f671

bors merged commit 4f1f671 into rust-lang:masterJul 11, 2017

bors mentioned this pull request Jul 11, 2017
Downgrade ProjectionTy's TraitRef to its substs #42388
Merged

alexcrichton mentioned this pull request Aug 13, 2017
20% compiler memory usage regression in syntex_syntax incremental compile #43835
Closed

kennytm mentioned this pull request Aug 29, 2017
Migrate a slew of metadata methods to queries #44142
Merged

Uh oh!

Uh oh!

Conversation

michaelwoerister commented Jul 3, 2017

Uh oh!

nikomatsakisJul 7, 2017

Choose a reason for hiding this comment

Uh oh!

michaelwoeristerJul 7, 2017

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented Jul 7, 2017

Uh oh!

bors commented Jul 7, 2017

Uh oh!

bors commented Jul 8, 2017

Uh oh!

michaelwoerister commented Jul 10, 2017

Uh oh!

nikomatsakisJul 10, 2017

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented Jul 10, 2017

Uh oh!

nikomatsakis commented Jul 10, 2017

Uh oh!

kennytm commented Jul 10, 2017

Uh oh!

nikomatsakis commented Jul 10, 2017

Uh oh!

kennytm commented Jul 10, 2017

Uh oh!

nikomatsakis commented Jul 10, 2017

Uh oh!

kennytm commented Jul 10, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikomatsakis commented Jul 10, 2017

Uh oh!

nikomatsakis commented Jul 10, 2017

Uh oh!

bors commented Jul 10, 2017

Uh oh!

michaelwoerister commented Jul 10, 2017

Uh oh!

bors commented Jul 10, 2017

Uh oh!

bors commented Jul 11, 2017

Uh oh!

Mark-Simulacrum commented Jul 11, 2017

Uh oh!

michaelwoerister commented Jul 12, 2017

Uh oh!

alexcrichton commented Jul 20, 2017

Uh oh!

michaelwoerister commented Jul 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kennytm commented Jul 10, 2017•
edited
Loading