Group dep node data into a single structure #57061

Zoxc · 2018-12-22T12:51:35Z

rust-highfive · 2018-12-22T13:04:01Z

The job x86_64-gnu-llvm-6.0 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.

travis_time:end:0a437b2e:start=1545483198358649858,finish=1545483199385515573,duration=1026865715
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---

[00:03:15] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:16] tidy error: /checkout/src/librustc/dep_graph/graph.rs:465: trailing whitespace
[00:03:17] some tidy checks failed
[00:03:17] 
[00:03:17] 
[00:03:17] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:17] 
[00:03:17] 
[00:03:17] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:17] Build completed unsuccessfully in 0:00:47
[00:03:17] Build completed unsuccessfully in 0:00:47
[00:03:17] Makefile:79: recipe for target 'tidy' failed
[00:03:17] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0553cbb0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sat Dec 22 12:56:45 UTC 2018
---
travis_time:end:02d60000:start=1545483406122741310,finish=1545483406128257312,duration=5516002
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:02756856
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:086a4c38
travis_time:start:086a4c38
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:103c7e08
$ dmesg | grep -i kill

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 · 2018-12-22T13:30:05Z

@bors try

bors · 2018-12-22T13:30:17Z

⌛ Trying commit 775d3e0861c93f8e4794264948ba7ed0956d5456 with merge 2f088a6151d06e930ecb997ba0859a9c32fc3b8a...

bors · 2018-12-22T15:56:27Z

☀️ Test successful - status-travis
State: approved= try=True

Zoxc · 2018-12-22T16:39:47Z

@rust-timer build 2f088a6151d06e930ecb997ba0859a9c32fc3b8a

rust-timer · 2018-12-22T16:39:48Z

Success: Queued 2f088a6151d06e930ecb997ba0859a9c32fc3b8a with parent 9689ada, comparison URL.

rust-timer · 2018-12-22T20:05:43Z

Finished benchmarking try commit 2f088a6151d06e930ecb997ba0859a9c32fc3b8a

michaelwoerister · 2018-12-22T20:46:20Z

Looks like pretty solid wins! I don't have time to review this in detail right now. One thing I noticed: It seems like the PR undoes #49069 which @wesleywiser implemented a while ago per my request. It would be good to check if the wins here are due to the struct-of-arrays approach being a bad idea in this scenario. Maybe it's a bad trade-off, i.e. we trade faster serialization for bad cache locality during tracking.

michaelwoerister

Thanks again for the PR, @Zoxc! r=me with the nits addressed.

I'm not quite happy with factoring of functions in the hir collector. It seems that it should be possible to make this a little less complicated. But I couldn't come up with something better either.

michaelwoerister · 2018-12-30T21:00:43Z

src/librustc/hir/map/collector.rs

@@ -139,9 +182,9 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
        let node_hashes = self
            .hir_body_nodes
            .iter()
-            .fold(Fingerprint::ZERO, |fingerprint, &(def_path_hash, dep_node_index)| {
+            .fold(Fingerprint::ZERO, |fingerprint, &(def_path_hash, hash)| {


Can you rename fingerprint to something like combined_fingerprint and hash to fingerprint here? That should make it easier to understand what's going on.

michaelwoerister · 2018-12-30T21:04:05Z

src/librustc/hir/map/collector.rs

+fn input_dep_node_and_hash<'a, I>(
+    dep_graph: &DepGraph,
+    hcx: &mut StableHashingContext<'a>,
+    def_node: DepNode,


This should be dep_node I guess?

michaelwoerister · 2018-12-30T21:34:49Z

src/librustc/dep_graph/graph.rs

@@ -211,7 +199,7 @@ impl DepGraph {
                reads: SmallVec::new(),
                read_set: Default::default(),
            })),
-            |data, key, task| data.borrow_mut().complete_task(key, task))
+            |data, key, f, task| data.borrow_mut().complete_task(key, task, f))


Can you rename f to fingerprint here and at the other occurrences?

michaelwoerister · 2018-12-30T21:37:54Z

src/librustc/hir/map/collector.rs

+    (dep_node_index, hash)
+}
+
+fn hir_dep_nodes<'a, I>(


Could you please rename this to alloc_hir_dep_nodes?

Zoxc · 2018-12-31T08:31:19Z

@bors r=michaelwoerister

bors · 2018-12-31T08:31:20Z

📌 Commit 2738f2c has been approved by michaelwoerister

Zoxc · 2018-12-31T08:36:56Z

@bors r-

Zoxc · 2018-12-31T09:35:07Z

@bors r=michaelwoerister

bors · 2018-12-31T09:35:08Z

📌 Commit 2738f2c has been approved by michaelwoerister

bors · 2018-12-31T10:55:52Z

⌛ Testing commit 2738f2c with merge 433ef82...

@michaelwoerister

Group dep node data into a single structure r? @michaelwoerister

bors · 2018-12-31T13:37:13Z

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing 433ef82 to master...

rust-highfive assigned michaelwoerister Dec 22, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 22, 2018

Zoxc force-pushed the graph-refactor branch from e9a9cc9 to 775d3e0 Compare December 22, 2018 13:16

michaelwoerister reviewed Dec 30, 2018

View reviewed changes

Zoxc added 3 commits December 31, 2018 09:15

Group dep node data into a single structure

a426d47

Use entry API to avoid double lookup when interning dep nodes

18d6b37

Address comments

2738f2c

Zoxc force-pushed the graph-refactor branch from e2e471e to 2738f2c Compare December 31, 2018 08:20

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 Dec 31, 2018

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 Dec 31, 2018

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 Dec 31, 2018

bors added a commit that referenced this pull request Dec 31, 2018

Auto merge of #57061 - Zoxc:graph-refactor, r=michaelwoerister

433ef82

Group dep node data into a single structure r? @michaelwoerister

bors merged commit 2738f2c into rust-lang:master Dec 31, 2018

Zoxc deleted the graph-refactor branch December 31, 2018 13:42

This was referenced Dec 31, 2018

Clean up and optimize OpenTask / read_index #57114

Merged

Parallelize and optimize parts of HIR map creation #57232

Merged

michaelwoerister mentioned this pull request Jan 7, 2019

[do not merge] Revert "Convert SerializedDepGraph to be a struct-of-arrays" #57070

Closed

tgnottingham mentioned this pull request Nov 2, 2020

Use PackedFingerprint in DepNode to reduce memory consumption #78646

Merged

Group dep node data into a single structure #57061

Group dep node data into a single structure #57061

Uh oh!

Conversation

Zoxc commented Dec 22, 2018

Uh oh!

rust-highfive commented Dec 22, 2018

Uh oh!

Zoxc commented Dec 22, 2018

Uh oh!

bors commented Dec 22, 2018

Uh oh!

bors commented Dec 22, 2018

Uh oh!

Zoxc commented Dec 22, 2018

Uh oh!

rust-timer commented Dec 22, 2018

Uh oh!

rust-timer commented Dec 22, 2018

Uh oh!

michaelwoerister commented Dec 22, 2018

Uh oh!

michaelwoerister left a comment

Choose a reason for hiding this comment

Uh oh!

michaelwoerister Dec 30, 2018

Choose a reason for hiding this comment

Uh oh!

michaelwoerister Dec 30, 2018

Choose a reason for hiding this comment

Uh oh!

michaelwoerister Dec 30, 2018

Choose a reason for hiding this comment

Uh oh!

michaelwoerister Dec 30, 2018

Choose a reason for hiding this comment

Uh oh!

Zoxc commented Dec 31, 2018

Uh oh!

bors commented Dec 31, 2018

Uh oh!

Zoxc commented Dec 31, 2018

Uh oh!

Zoxc commented Dec 31, 2018

Uh oh!

bors commented Dec 31, 2018

Uh oh!

bors commented Dec 31, 2018

Uh oh!

bors commented Dec 31, 2018

Uh oh!

Uh oh!