Simplify lazy DefPathHash decoding by using an on-disk hash table. #82183

michaelwoerister · 2021-02-16T13:14:23Z

This PR simplifies the logic around mapping DefPathHash values encountered during incremental compilation to valid DefIds in the current session. It is able to do so by using an on-disk hash table encoding that allows for looking up values directly, i.e. without deserializing the entire table.

The main simplification comes from not having to keep track of DefPathHashes being used during the compilation session.

michaelwoerister · 2021-02-16T13:46:45Z

@bors try @rust-timer queue

rust-timer · 2021-02-16T13:46:46Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-02-16T13:46:55Z

⌛ Trying commit 3d71523c6e45ae4e9c99bd32f8625bab2bd600de with merge 6da745eb8d19cfce6764aac7f880ad7503397d14...

bors · 2021-02-16T14:47:22Z

☀️ Try build successful - checks-actions
Build commit: 6da745eb8d19cfce6764aac7f880ad7503397d14 (6da745eb8d19cfce6764aac7f880ad7503397d14)

rust-timer · 2021-02-16T14:47:24Z

Queued 6da745eb8d19cfce6764aac7f880ad7503397d14 with parent f1c47c7, future comparison URL.

rust-timer · 2021-02-16T16:26:43Z

Finished benchmarking try commit (6da745eb8d19cfce6764aac7f880ad7503397d14): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

michaelwoerister · 2021-02-16T17:05:03Z

OK, those results look promising. There's still a couple more variations to try.

michaelwoerister · 2021-02-17T16:10:00Z

Let's see if we can remove the global DefPathHash->DefId cache without a performance hit.

@bors try @rust-timer queue

rust-timer · 2021-02-17T16:10:02Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-02-17T16:10:11Z

⌛ Trying commit 975c6c821e71b3186b8d51455e3090014a19183d with merge 2a538c388dc56268456c9ff721a719cc9f630dda...

bors · 2021-02-17T17:10:39Z

☀️ Try build successful - checks-actions
Build commit: 2a538c388dc56268456c9ff721a719cc9f630dda (2a538c388dc56268456c9ff721a719cc9f630dda)

rust-timer · 2021-02-17T17:10:41Z

Queued 2a538c388dc56268456c9ff721a719cc9f630dda with parent ee88f46, future comparison URL.

rust-timer · 2021-02-17T18:48:49Z

Finished benchmarking try commit (2a538c388dc56268456c9ff721a719cc9f630dda): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

michaelwoerister · 2021-02-18T09:18:31Z

It looks like performance is almost the same without the additional caching layer (0.1% - 0.3% difference). Let's see if we can get rid of the regressions in non-incremental builds.

michaelwoerister · 2021-02-18T11:45:48Z

@bors try @rust-timer queue

rust-timer · 2021-02-18T11:45:49Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-02-18T11:45:58Z

⌛ Trying commit 4ca1a5359e9bd9b9b15ff031e13414e3ad4c092b with merge 649c77c9429915099bc407a418baad103403a54f...

bors · 2021-02-18T12:36:23Z

☀️ Try build successful - checks-actions
Build commit: 649c77c9429915099bc407a418baad103403a54f (649c77c9429915099bc407a418baad103403a54f)

rust-timer · 2021-02-18T12:36:24Z

Queued 649c77c9429915099bc407a418baad103403a54f with parent 25a2c13, future comparison URL.

rust-timer · 2021-02-18T14:14:11Z

Finished benchmarking try commit (649c77c9429915099bc407a418baad103403a54f): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

michaelwoerister · 2021-09-17T14:38:39Z

Let's give it another try with the fixed version of odht.

@bors r=wesleywiser

bors · 2021-09-17T14:38:40Z

📌 Commit 4d151d9 has been approved by wesleywiser

bors · 2021-09-17T15:07:22Z

⌛ Testing commit 4d151d9 with merge 5cccabccb9d354c6fb4fa78e0bb36faf9e8d7513...

bors · 2021-09-17T15:49:17Z

💔 Test failed - checks-actions

rust-log-analyzer · 2021-09-17T15:50:11Z

The job i686-msvc-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [rustdoc] rustdoc\macro-document-private-duplicate.rs stdout ----

Some tests failed in compiletest suite=rustdoc mode=rustdoc host=i686-pc-windows-msvc target=i686-pc-windows-msvc
error: htmldocck failed!
status: exit code: 1
command: "C:\\hostedtoolcache\\windows\\Python\\3.9.7\\x64\\python3.exe" "D:\\a\\rust\\rust\\src/etc/htmldocck.py" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\test\\rustdoc\\macro-document-private-duplicate" "D:\\a\\rust\\rust\\src/test\\rustdoc\\macro-document-private-duplicate.rs"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
12: @has check failed
 `PATTERN` did not match
 // @has macro_document_private_duplicate/macro.a_macro.html 'Doc 1.'
19: @!has check failed
 `PATTERN` did not match
 // @!has macro_document_private_duplicate/macro.a_macro.html 'Doc 2.'
Encountered 2 errors

------------------------------------------

---
test result: FAILED. 462 passed; 1 failed; 7 ignored; 0 measured; 0 filtered out; finished in 73.62s



command did not execute successfully: "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0-tools-bin\\compiletest.exe" "--compile-lib-path" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\bin" "--run-lib-path" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\lib\\rustlib\\i686-pc-windows-msvc\\lib" "--rustc-path" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\bin\\rustc.exe" "--rustdoc-path" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2\\bin\\rustdoc.exe" "--src-base" "D:\\a\\rust\\rust\\src/test\\rustdoc" "--build-base" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\test\\rustdoc" "--stage-id" "stage2-i686-pc-windows-msvc" "--suite" "rustdoc" "--mode" "rustdoc" "--target" "i686-pc-windows-msvc" "--host" "i686-pc-windows-msvc" "--llvm-filecheck" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\llvm\\build\\bin\\FileCheck.exe" "--nodejs" "C:\\Program Files\\nodejs\\node" "--npm" "C:\\Program Files\\nodejs\\npm" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\native\\rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\native\\rust-test-helpers" "--docck-python" "C:\\hostedtoolcache\\windows\\Python\\3.9.7\\x64\\python3.exe" "--lldb-python" "C:\\hostedtoolcache\\windows\\Python\\3.9.7\\x64\\python3.exe" "--gdb" "C:\\msys64\\usr\\bin\\gdb" "--llvm-version" "13.0.0-rust-1.57.0-nightly" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker dwp engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit orcshared orctargetprocess passes powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo webassemblyutils windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"


Build completed unsuccessfully in 0:36:53
Build completed unsuccessfully in 0:36:53
make: *** [Makefile:72: ci-subset-1] Error 1

wesleywiser · 2021-09-18T12:17:06Z

Seems unrelated?

@bors retry

bors · 2021-09-18T14:37:42Z

⌛ Testing commit 4d151d9 with merge d6cd2c6...

bors · 2021-09-18T17:18:23Z

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing d6cd2c6 to master...

rust-timer · 2021-09-18T22:12:44Z

Finished benchmarking commit (d6cd2c6): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

Very large improvement in instruction counts (up to -7.7% on incr-unchanged builds of deeply-nested)
Moderate regression in instruction counts (up to 1.9% on incr-full builds of coercions)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

pnkfelix · 2021-09-21T20:40:56Z

Visiting for weekly performance triage: the wins here massively outweigh the few slight losses.

@rustbot label: +perf-regression-triaged

local_def_path_hash_to_def_id is used by Debug impl for DepNode and it looks for DefPathHash inside the current compilation. During incremental compilation we are going through nodes that belong to a previous compilation and might not be present and a simple attempt to print such node with tracing::debug (try_mark_parent_green does it for example) results in a otherwise avoidable panic Panic was added in rust-lang#82183, specifically in 2b60338, with a comment "We only use this mapping for cases where we know that it must succeed.", but I'm not sure if this property holds when we traverse nodes from the old compilation in order to figure out if they are valid or not

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 16, 2021

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 16, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 17, 2021

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 17, 2021

michaelwoerister force-pushed the lazier-defpathhash-loading2 branch from 8ec80dc to 4ca1a53 Compare February 18, 2021 11:45

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 18, 2021

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 Sep 17, 2021

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 17, 2021

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 Sep 18, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 18, 2021

bors merged commit d6cd2c6 into rust-lang:master Sep 18, 2021

rustbot added this to the 1.57.0 milestone Sep 18, 2021

This was referenced Sep 18, 2021

Make AST->HIR lowering incremental #88186

Closed

Protect crate metadata from corruption via SHA-256 hash #87896

Closed

camelid mentioned this pull request Sep 19, 2021

Run hello world error on m1 mbp(macOS 12.0 (21A5506j)) #89085

Closed

m-ou-se mentioned this pull request Sep 20, 2021

zvariant fails to compile on latest nightly: size cannot be statically determined #89119

Closed

rustbot added the perf-regression-triaged The performance regression has been triaged. label Sep 21, 2021

petrochenkov mentioned this pull request Sep 24, 2021

Test src\test\rustdoc\macro-document-private-duplicate.rs fails spuriously #89228

Open

DropDemBits mentioned this pull request Mar 12, 2024

ICE: called Option::unwrap() on a None value at def_path_hash_map.rs:23:85 #107226

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify lazy DefPathHash decoding by using an on-disk hash table. #82183

Simplify lazy DefPathHash decoding by using an on-disk hash table. #82183

michaelwoerister commented Feb 16, 2021 •

edited

Loading

michaelwoerister commented Feb 16, 2021

rust-timer commented Feb 16, 2021

bors commented Feb 16, 2021

bors commented Feb 16, 2021

rust-timer commented Feb 16, 2021

rust-timer commented Feb 16, 2021

michaelwoerister commented Feb 16, 2021

michaelwoerister commented Feb 17, 2021

rust-timer commented Feb 17, 2021

bors commented Feb 17, 2021

bors commented Feb 17, 2021

rust-timer commented Feb 17, 2021

rust-timer commented Feb 17, 2021

michaelwoerister commented Feb 18, 2021

michaelwoerister commented Feb 18, 2021

rust-timer commented Feb 18, 2021

bors commented Feb 18, 2021

bors commented Feb 18, 2021

rust-timer commented Feb 18, 2021

rust-timer commented Feb 18, 2021

michaelwoerister commented Sep 17, 2021

bors commented Sep 17, 2021

bors commented Sep 17, 2021

bors commented Sep 17, 2021

rust-log-analyzer commented Sep 17, 2021

wesleywiser commented Sep 18, 2021

bors commented Sep 18, 2021

bors commented Sep 18, 2021

rust-timer commented Sep 18, 2021

pnkfelix commented Sep 21, 2021

Simplify lazy DefPathHash decoding by using an on-disk hash table. #82183

Simplify lazy DefPathHash decoding by using an on-disk hash table. #82183

Conversation

michaelwoerister commented Feb 16, 2021 • edited Loading

michaelwoerister commented Feb 16, 2021

rust-timer commented Feb 16, 2021

bors commented Feb 16, 2021

bors commented Feb 16, 2021

rust-timer commented Feb 16, 2021

rust-timer commented Feb 16, 2021

michaelwoerister commented Feb 16, 2021

michaelwoerister commented Feb 17, 2021

rust-timer commented Feb 17, 2021

bors commented Feb 17, 2021

bors commented Feb 17, 2021

rust-timer commented Feb 17, 2021

rust-timer commented Feb 17, 2021

michaelwoerister commented Feb 18, 2021

michaelwoerister commented Feb 18, 2021

rust-timer commented Feb 18, 2021

bors commented Feb 18, 2021

bors commented Feb 18, 2021

rust-timer commented Feb 18, 2021

rust-timer commented Feb 18, 2021

michaelwoerister commented Sep 17, 2021

bors commented Sep 17, 2021

bors commented Sep 17, 2021

bors commented Sep 17, 2021

rust-log-analyzer commented Sep 17, 2021

wesleywiser commented Sep 18, 2021

bors commented Sep 18, 2021

bors commented Sep 18, 2021

rust-timer commented Sep 18, 2021

pnkfelix commented Sep 21, 2021

michaelwoerister commented Feb 16, 2021 •

edited

Loading