Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize HIR map #60246

Merged
merged 2 commits into from
May 8, 2019
Merged

Optimize HIR map #60246

merged 2 commits into from
May 8, 2019

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Apr 24, 2019

Builds on #59042

cc @ljedrz
r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 24, 2019
@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 24, 2019

@bors try

@bors
Copy link
Contributor

bors commented Apr 24, 2019

⌛ Trying commit d3bd03ce1d50238d54a70a96b22731cd76cc5e20 with merge 890cc87d1001f65c608c711d7a4d0d48129a43c1...

@rust-highfive
Copy link
Collaborator

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:001763f4:start=1556135562249149308,finish=1556135563013292812,duration=764143504
$ 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
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
travis_time:start:test_assembly
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:22:24] 
[01:22:24] running 9 tests
[01:22:24] iiiiiiiii
[01:22:24] 
[01:22:24]  finished in 0.151
[01:22:24] travis_fold:end:test_assembly

---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:22:40] 
[01:22:40] running 121 tests
[01:23:06] .iiiii...i.....i..i...i..i.i.i..i.ii...i.....i..i....i..........iiii..........i...ii...i.......ii.i. 100/121
[01:23:10] i.i......iii.i.....ii
[01:23:10] 
[01:23:10]  finished in 30.681
[01:23:10] travis_fold:end:test_debuginfo

---
Check compiletest suite=run-make-fulldeps mode=run-make (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:52:08] 
[01:52:08] running 197 tests
[01:52:37] ..................i...i................................................................i............ 100/197
[01:53:23] ...................................F.................i......................................i....
[01:53:23] failures:
[01:53:23] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:53:23] 
[01:53:23] ---- [run-make] run-make-fulldeps/pretty-print-path-suffix stdout ----
[01:53:23] ---- [run-make] run-make-fulldeps/pretty-print-path-suffix stdout ----
[01:53:23] 
[01:53:23] error: make failed
[01:53:23] status: exit code: 2
[01:53:23] command: "make"
[01:53:23] stdout:
[01:53:23] ------------------------------------------
[01:53:23] make[1]: Entering directory '/checkout/src/test/run-make-fulldeps/pretty-print-path-suffix'
[01:53:23] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix  -o /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix/foo.out -Z unpretty=hir=foo input.rs
[01:53:23] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix  -o /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix/nest_foo.out -Z unpretty=hir=nest::foo input.rs
[01:53:23] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix  -o /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix/foo_method.out -Z unpretty=hir=foo_method input.rs
[01:53:23] diff -u /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix/foo.out foo.pp
[01:53:23] --- /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/pretty-print-path-suffix/pretty-print-path-suffix/foo.out 2019-04-24 21:45:41.689211650 +0000
[01:53:23] +++ foo.pp 2019-04-24 19:52:41.805211650 +0000
[01:53:23] @@ -0,0 +1,5 @@
[01:53:23] +
[01:53:23] +pub fn foo() -> i32 { 45 } /* foo */
[01:53:23] +
[01:53:23] +
[01:53:23] +pub fn foo() -> &'static str { "i am a foo." } /* nest::foo */
[01:53:23] Makefile:4: recipe for target 'all' failed
[01:53:23] make[1]: Leaving directory '/checkout/src/test/run-make-fulldeps/pretty-print-path-suffix'
[01:53:23] ------------------------------------------
[01:53:23] stderr:
[01:53:23] ------------------------------------------
[01:53:23] ------------------------------------------
[01:53:23] warning: ignoring --out-dir flag due to -o flag
[01:53:23] 
[01:53:23] warning: ignoring --out-dir flag due to -o flag
[01:53:23] 
[01:53:23] warning: ignoring --out-dir flag due to -o flag
[01:53:23] 
[01:53:23] make[1]: *** [all] Error 1
[01:53:23] ------------------------------------------
[01:53:23] 
[01:53:23] 
[01:53:23] 
[01:53:23] 
[01:53:23] failures:
[01:53:23]     [run-make] run-make-fulldeps/pretty-print-path-suffix
[01:53:23] 
[01:53:23] test result: FAILED. 191 passed; 1 failed; 5 ignored; 0 measured; 0 filtered out
[01:53:23] 
[01:53:23] 
[01:53:23] 
[01:53:23] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--rustdoc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "--src-base" "/checkout/src/test/run-make-fulldeps" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-make" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "cc" "--cxx" "c++" "--cflags" "-ffunction-sections -fdata-sections -fPIC -m64" "--llvm-components" "aarch64 aarch64asmparser aarch64asmprinter aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils all all-targets amdgpu amdgpuasmparser amdgpuasmprinter amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils analysis arm armasmparser armasmprinter armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter binaryformat bitreader bitwriter bpf bpfasmparser bpfasmprinter bpfcodegen bpfdesc bpfdisassembler bpfinfo codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfomsf debuginfopdb demangle dlltooldriver engine executionengine fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interpreter ipo irreader lanai lanaiasmparser lanaiasmprinter lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto mc mcdisassembler mcjit mcparser mips mipsasmparser mipsasmprinter mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmprinter msp430codegen msp430desc msp430info native nativecodegen nvptx nvptxasmprinter nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit passes powerpc powerpcasmparser powerpcasmprinter powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata runtimedyld scalaropts selectiondag sparc sparcasmparser sparcasmprinter sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzasmprinter systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target transformutils vectorize windowsmanifest x86 x86asmparser x86asmprinter x86codegen x86desc x86disassembler x86info x86utils xcore xcoreasmprinter xcorecodegen xcoredesc xcoredisassembler xcoreinfo" "--llvm-cxxflags" "-I/usr/lib/llvm-6.0/include -std=c++0x -fuse-ld=gold -Wl,--no-keep-files-mapped -Wl,--no-map-whole-files -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -O2 -DNDEBUG  -fno-exceptions -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS" "--ar" "ar" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:53:23] 
[01:53:23] 
[01:53:23] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:53:23] Build completed unsuccessfully in 0:42:50
[01:53:23] Build completed unsuccessfully in 0:42:50
[01:53:23] make: *** [check] Error 1
[01:53:23] Makefile:48: recipe for target 'check' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1004690c
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Apr 24 21:46:16 UTC 2019

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)

@bors
Copy link
Contributor

bors commented Apr 24, 2019

☀️ Try build successful - checks-travis
Build commit: 890cc87d1001f65c608c711d7a4d0d48129a43c1

@Xanewok
Copy link
Member

Xanewok commented Apr 24, 2019

@rust-timer build 890cc87d1001f65c608c711d7a4d0d48129a43c1

@rust-timer
Copy link
Collaborator

Success: Queued 890cc87d1001f65c608c711d7a4d0d48129a43c1 with parent e305df1, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 890cc87d1001f65c608c711d7a4d0d48129a43c1

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 25, 2019

Seems like this approach works well. There's still some slight memory regressions though.

@Zoxc Zoxc changed the title Optimize HIR map [WIP] Optimize HIR map Apr 25, 2019
@rust-highfive
Copy link
Collaborator

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:005982c6:start=1556216519294202627,finish=1556216521438908215,duration=2144705588
$ 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
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:04:18] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:19] tidy error: /checkout/src/librustc/hir/map/mod.rs:1215: trailing whitespace
[00:04:20] some tidy checks failed
[00:04:20] 
[00:04:20] 
[00:04:20] 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:04:20] 
[00:04:20] 
[00:04:20] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:20] Build completed unsuccessfully in 0:00:45
[00:04:20] Build completed unsuccessfully in 0:00:45
[00:04:20] make: *** [tidy] Error 1
[00:04:20] Makefile:67: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0ec45cf8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Thu Apr 25 18:26:33 UTC 2019
---
travis_time:end:0d80e979:start=1556216793919279285,finish=1556216793924162685,duration=4883400
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0d377e70
$ 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:10837696
travis_time:start:10837696
$ 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:1634df17
$ 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 Zoxc changed the title [WIP] Optimize HIR map Optimize HIR map Apr 25, 2019
Copy link
Member

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

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

IIUC this fixes a regression introduced in #59042 and doesn't look like a very invasive change (limited only to one data structure) - should we not land it?

@@ -160,6 +161,8 @@ impl Forest {
}
}

pub(super) type HirMap<'hir> = [Vec<Option<IndexVec<ItemLocalId, Option<Entry<'hir>>>>>; 2];
Copy link
Member

Choose a reason for hiding this comment

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

It takes a while to decipher what the structure looks like and why it looks this way - it'd be great if we could add a doc-comment here with an explanation

fn all_ids<'a>(&'a self) -> impl Iterator<Item = HirId> + 'a {
let map = &self.map;
let spaces = [DefIndexAddressSpace::Low, DefIndexAddressSpace::High].iter().cloned();
spaces.flat_map(move |space| {
Copy link
Member

Choose a reason for hiding this comment

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

It'd be great to also add a comment here and/or rustfmt the function body

@bors
Copy link
Contributor

bors commented May 6, 2019

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

@@ -160,6 +161,8 @@ impl Forest {
}
}

pub(super) type HirMap<'hir> = [Vec<Option<IndexVec<ItemLocalId, Option<Entry<'hir>>>>>; 2];
Copy link
Member

Choose a reason for hiding this comment

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

hir::map::Map is the HIR map. Can you rename this type alias to HirEntryMap or something?

Copy link
Member

@eddyb eddyb left a comment

Choose a reason for hiding this comment

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

This looks great, r=me modulo nits!

@Zoxc
Copy link
Contributor Author

Zoxc commented May 8, 2019

@bors r=eddyb

@bors
Copy link
Contributor

bors commented May 8, 2019

📌 Commit d33db6e has been approved by eddyb

@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 May 8, 2019
@bors
Copy link
Contributor

bors commented May 8, 2019

⌛ Testing commit d33db6e with merge d7f4a86...

bors added a commit that referenced this pull request May 8, 2019
@bors
Copy link
Contributor

bors commented May 8, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: eddyb
Pushing d7f4a86 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 8, 2019
@bors bors merged commit d33db6e into rust-lang:master May 8, 2019
@bors bors mentioned this pull request May 8, 2019
return Some(idx)
}
}
fn matces_suffix(&self, hir: HirId) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

*matches

}
};

self.all_ids().filter(move |hir| nodes.matces_suffix(*hir)).map(move |hir| {
Copy link
Member

Choose a reason for hiding this comment

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

*matches

@bjorn3
Copy link
Member

bjorn3 commented May 8, 2019

I am a little confused about the order of comments on this PR: bors merges (1h ago) -> me reviewing it (few min ago) -> Zoxc gives r+ (3h ago).

@Zoxc
Copy link
Contributor Author

Zoxc commented May 8, 2019

@bjorn3 It's github new feature, time-travelling comments.

@Zoxc Zoxc deleted the hir-map-vec branch May 8, 2019 16:41
@bjorn3
Copy link
Member

bjorn3 commented May 8, 2019

When looking at my notifications list, it said bors was the last commenter :)

@Zoxc
Copy link
Contributor Author

Zoxc commented May 8, 2019

You can see that I will post my comment in 3 hours. Github simply sorts comments by time!

@bjorn3
Copy link
Member

bjorn3 commented May 8, 2019

On mobile it says 2 hours ago though.

@Zoxc
Copy link
Contributor Author

Zoxc commented May 14, 2019

Perf result: https://perf.rust-lang.org/compare.html?start=b92d360c6cf029bd98c154cb510ec9e11b39bad6&end=d7f4a864e13dc88d7a28c0b742da844af4e3abc4&stat=wall-time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants