Store def_id_to_hir_id as variant in hir_owner.

[spastorino]

If hir_owner is Owner(_), the LocalDefId is pointing to an owner, so the ItemLocalId is 0.
If the HIR node does not exist, we store Phantom.
Otherwise, we store the HirId associated to the LocalDefId.

Related to #89278

r? @oli-obk

[spastorino]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit a0bcce4 with merge a3bbefe418ea277c1fac331fbeeea5eb3db663dc...

[bors]

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

[rust-timer]

Queued a3bbefe418ea277c1fac331fbeeea5eb3db663dc with parent 21b4a9c, future comparison URL.

[spastorino]

@bors retry

[bors]

⌛ Trying commit a0bcce4 with merge 59ef690361032d8c20d14ef3b2670082c06e207a...

[rust-timer]

Finished benchmarking commit (a3bbefe418ea277c1fac331fbeeea5eb3db663dc): comparison url.

Summary: This benchmark run shows 145 relevant regressions 😿 to instruction counts.

• Average relevant regression: 1.0%
• Average relevant improvement: -0.3%
• Largest regression in instruction counts: 5.8% on incr-unchanged builds of match-stress-enum check

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

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

[bors]

☀️ Try build successful - checks-actions
Build commit: 59ef690361032d8c20d14ef3b2670082c06e207a (59ef690361032d8c20d14ef3b2670082c06e207a)

[spastorino]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit d17eb78 with merge 3e36f8ea7f3c1c037ed81a875c73a0bc8429ec68...

[spastorino]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 5a299a9 with merge bf734e10c8724f96fadfd13736aea2bcf9ec3225...

[bors]

⌛ Trying commit bf1ca2e with merge dbcda59e03da05c3fd162aa12d5edfba54c9928c...

[rust-log-analyzer]

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

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

running 165 tests
...................................i..............

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/i686-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/mir-opt" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/mir-opt" "--stage-id" "stage2-i686-unknown-linux-gnu" "--suite" "mir-opt" "--mode" "mir-opt" "--target" "i686-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-12/bin/FileCheck" "--nodejs" "/usr/bin/node" "--linker" "cc" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "12.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils 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 engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hellonew hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo 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 perfjitevents 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 windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"
expected success, got: signal: 11 (core dumped)

Build completed unsuccessfully in 0:01:49

[bors]

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

[rust-timer]

Queued dbcda59e03da05c3fd162aa12d5edfba54c9928c with parent 24ae996, future comparison URL.

[rust-timer]

Finished benchmarking commit (dbcda59e03da05c3fd162aa12d5edfba54c9928c): comparison url.

Summary: This benchmark run shows 152 relevant regressions 😿 to instruction counts.

• Average relevant regression: 1.2%
• Largest regression in instruction counts: 5.5% on incr-unchanged builds of match-stress-enum check

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

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

[oli-obk]

@cjgillot we've been able to reduce the perf regression in non-clean builds significantly. I believe the problem was that the hir_owner query returned a HirId for non-owners, but that id was always thrown away (except in local_def_id_to_hir_id). So if the ignored value changed any callers of hir_owner were recomputed, even though they just threw the changed value away.

This has been fixed by changing hir_owner back to its previous signature and instead adding a second query specifically for local_def_id_to_hir_id.

There are still regressions (mostly in incremental clean builds), but I don't know how to avoid them... it seems the hir_crate query changes its output even in clean runs?

[cjgillot]

@cjgillot we've been able to reduce the perf regression in non-clean builds significantly.

This is great! The perf is still bad, but I don't think there is anything more than query system overhead, is there?

I believe the problem was that the hir_owner query returned a HirId for non-owners, but that id was always thrown away (except in local_def_id_to_hir_id). So if the ignored value changed any callers of hir_owner were recomputed, even though they just threw the changed value away.

I did not expect that. Does it come from queries call local_def_id_to_hir_id without fetching the HIR behind those ids.

This has been fixed by changing hir_owner back to its previous signature and instead adding a second query specifically for local_def_id_to_hir_id.

I had excluded this possibility because of some old PR's perf result, but I guess I did not think it through.

There are still regressions (mostly in incremental clean builds), but I don't know how to avoid them... it seems the hir_crate query changes its output even in clean runs?

hir_crate lowers HIR for the whole crate. As the AST is not hashed, it's always recomputed. hir_crate output is still hashed (it's hash is the HIR contribution to crate_hash), but contains way too much information for it's own good. One of the aims of #88186 is to change this.

[oli-obk]

Does it come from queries call local_def_id_to_hir_id without fetching the HIR behind those ids.

I did not dig in the details further. But I do know of queries that do that, since they need the HirId for obligation causes

The perf is still bad, but I don't think there is anything more than query system overhead, is there?

Yea, it looks exactly like that is the cause.

but contains way too much information for it's own good

Yea that's what I was guessing, thanks for confirming.

@bors r+ rollup=never

[bors]

📌 Commit bf1ca2e has been approved by oli-obk

[spastorino]

CI is segfaulting, unrelated to this PR.

[bors]

⌛ Testing commit bf1ca2e with merge 24b8bb1...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 24b8bb1 to master...

[rust-timer]

Finished benchmarking commit (24b8bb1): comparison url.

Summary: This benchmark run shows 161 relevant regressions 😿 to instruction counts.

• Average relevant regression: 1.2%
• Average relevant improvement: -0.3%
• Largest regression in instruction counts: 5.5% on incr-unchanged builds of match-stress-enum check

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

[rylev]

Talked with @oli-obk and @spastorino about this here. The change in this PR (along with #93301) is needed to be able to make AST -> HIR lowering incremental (as a part of the general move towards more incremental compilation). This is achieved by removing a global untracked-by-incremental map that exists in TyCtxt. While this doesn't get us much immediately, there is a strong belief that this will lead to dividends through more incremental compilation, potentially less ICEs, and potentially easier implementations for things like async functions in traits.

@rustbot label: +perf-regression-triaged