Also cache the stable hash of interned Predicates #94487

oli-obk · 2022-03-01T15:17:05Z

continuation of #94299

This is a small perf improvement and shares more code between Ty and Predicate

oli-obk · 2022-03-01T15:17:21Z

@bors try @rust-timer queue

rust-timer · 2022-03-01T15:17:23Z

Awaiting bors try build completion.

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

bors · 2022-03-01T15:17:29Z

⌛ Trying commit bc9aec18069faf568dcfc3d779d3a8b94be3fda8 with merge 71f04f82da88b092fafcfe3840dc6ba48fcb8586...

bors · 2022-03-01T16:52:59Z

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

rust-timer · 2022-03-01T16:53:01Z

Queued 71f04f82da88b092fafcfe3840dc6ba48fcb8586 with parent f0c4da4, future comparison URL.

rust-timer · 2022-03-01T18:23:25Z

Finished benchmarking commit (71f04f82da88b092fafcfe3840dc6ba48fcb8586): comparison url.

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

Arithmetic mean of relevant regressions: 0.9%
Arithmetic mean of relevant improvements: -0.3%
Arithmetic mean of all relevant changes: 0.9%
Largest regression in instruction counts: 4.4% on incr-full builds of deep-vector 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 · 2022-03-02T09:48:35Z

Looks like predicates aren't stable hashed often enough for this to be effective.

Maybe we can make regular hashing cheaper by hashing the stable hash instead of the contents?

oli-obk · 2022-03-03T12:27:12Z

@bors try @rust-timer queue

rust-timer · 2022-03-03T12:27:14Z

Awaiting bors try build completion.

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

bors · 2022-03-03T12:27:20Z

⌛ Trying commit 8caeae4ceb40fa79839e74209b5a28a17317de84 with merge 27dd5ee704c0b8bf49220bb45c7799038fb4b08f...

bors · 2022-03-03T14:07:00Z

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

rust-timer · 2022-03-03T14:07:01Z

Queued 27dd5ee704c0b8bf49220bb45c7799038fb4b08f with parent 2f8d1a8, future comparison URL.

michaelwoerister · 2022-03-03T15:24:02Z

cc me

rust-timer · 2022-03-03T15:47:58Z

Finished benchmarking commit (27dd5ee704c0b8bf49220bb45c7799038fb4b08f): comparison url.

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

Arithmetic mean of relevant regressions: 0.9%
Largest regression in instruction counts: 0.9% on incr-unchanged builds of ctfe-stress-4 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 · 2022-03-07T11:10:35Z

I removed the predicate change, and am still getting

11,698,305  ???:<&rustc_middle::mir::interpret::allocation::Allocation as rustc_data_structures::stable_hasher::HashStable<rustc_query_system::ich::hcx::StableHashingContext>>::hash_stable
  -211,374  ???:<rustc_middle::ty::Ty as rustc_data_structures::stable_hasher::HashStable<rustc_query_system::ich::hcx::StableHashingContext>>::hash_stable
  -132,221  ???:<rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::try_mark_previous_green::<rustc_query_impl::plumbing::QueryCtxt>
   -57,694  ???:<rustc_query_system::dep_graph::serialized::SerializedDepGraph<rustc_middle::dep_graph::dep_node::DepKind> as rustc_serialize::serialize::Decodable<rustc_serialize::opaque::Decoder>>::decode
    36,078  /build/glibc-sMfBJT/glibc-2.31/string/../sysdeps/x86_64/strcmp.S:strcmp
    23,900  ???:<rustc_index::vec::IndexVec<rustc_middle::mir::Local, rustc_middle::mir::LocalDecl> as rustc_data_structures::stable_hasher::HashStable<rustc_query_system::ich::hcx::StableHashingContext>>::hash_stable
    21,202  ???:<core::iter::adapters::map::Map<std::collections::hash::map::Iter<rustc_hir::hir_id::ItemLocalId, rustc_middle::ty::Ty>, rustc_data_structures::stable_hasher::stable_hash_reduce<rustc_query_system::ich::hcx::StableHashingContext, (&rustc_hir::hir_id::ItemLocalId, &rustc_middle::ty::Ty), std::collections::hash::map::Iter<rustc_hir::hir_id::ItemLocalId, rustc_middle::ty::Ty>, <std::collections::hash::map::HashMap<rustc_hir::hir_id::ItemLocalId, rustc_middle::ty::Ty, core::hash::BuildHasherDefault<rustc_hash::FxHasher>> as rustc_data_structures::stable_hasher::HashStable<rustc_query_system::ich::hcx::StableHashingContext>>::hash_stable::{closure
    20,498  ???:<rustc_middle::ty::consts::Const as rustc_data_structures::stable_hasher::HashStable<rustc_query_system::ich::hcx::StableHashingContext>>::hash_stable
   -12,610  ???:<core::iter::adapters::map::Map<core::iter::adapters::map::Map<core::iter::adapters::enumerate::Enumerate<core::slice::iter::Iter<rustc_query_system::dep_graph::dep_node::DepNode<rustc_middle::dep_graph::dep_node::DepKind>>>, <rustc_index::vec::IndexVec<rustc_query_system::dep_graph::serialized::SerializedDepNodeIndex, rustc_query_system::dep_graph::dep_node::DepNode<rustc_middle::dep_graph::dep_node::DepKind>>>::iter_enumerated::{closure

Not sure what is going on. I even see an execution number diff in the ctfe stress test: https://perf.rust-lang.org/detailed-query.html?commit=27dd5ee704c0b8bf49220bb45c7799038fb4b08f&base_commit=2f8d1a835b4e7feaf625f74d0d5cb9b84dbc845a&benchmark=ctfe-stress-4-check&scenario=incr-unchanged

@bors try @rust-timer queue

let's check if this is really from this PR and not just the usual CTFE stress test noise (which I should investigate independently)

rust-timer · 2022-03-07T11:10:37Z

Awaiting bors try build completion.

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

bors · 2022-03-07T11:10:44Z

⌛ Trying commit 96132ccb086e4a808ea158ed774c6a284868b378 with merge 0480b09413a88d9278550a8f1791c9872df9eadd...

oli-obk · 2022-11-29T07:15:24Z

@bors rollup=never this is a perf improvement

fee1-dead · 2022-11-29T08:33:41Z

@oli-obk I believe rollup- is a shortcut for that.. or was I wrong?

oli-obk · 2022-11-29T08:36:49Z

I thought rollup- removed all rollup markers, basically resetting the PR to its state at opening time

fee1-dead · 2022-11-29T09:37:35Z

oh right, my bad. bors.rust-lang.org says that you were right.

bors · 2022-11-29T10:01:06Z

⌛ Testing commit 8582f96 with merge b2a158b62d60b7e0279eb25a674fcf05188bf565...

bors · 2022-11-29T10:34:13Z

💔 Test failed - checks-actions

oli-obk · 2022-11-29T10:36:21Z

@bors retry spurious freshness::move_target_directory_with_path_deps failure

 ---- freshness::move_target_directory_with_path_deps stdout ----
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build`
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build`
thread 'freshness::move_target_directory_with_path_deps' panicked at '
test failed running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build`
error: stderr did not match:
    1    +   Compiling a v0.5.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/t1155/foo/a)
    2    +   Compiling foo v0.5.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/t1155/foo)
1   3         Finished [..]


other output:

', src/tools/cargo/tests/testsuite/freshness.rs:2084:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    freshness::move_target_directory_with_path_deps

rust-log-analyzer · 2022-11-29T10:46:05Z

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

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


---- freshness::move_target_directory_with_path_deps stdout ----
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build`
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build`
thread 'freshness::move_target_directory_with_path_deps' panicked at '
error: stderr did not match:
error: stderr did not match:
    1    +   Compiling a v0.5.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/t1155/foo/a)
    2    +   Compiling foo v0.5.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/t1155/foo)
1   3         Finished [..]

other output:

', src/tools/cargo/tests/testsuite/freshness.rs:2084:10
---
test result: FAILED. 2571 passed; 1 failed; 149 ignored; 0 measured; 0 filtered out; finished in 103.51s

error: test failed, to rerun pass `--test testsuite`
Build completed unsuccessfully in 0:28:08
make: *** [Makefile:44: check-aux] Error 1

bors · 2022-11-29T16:26:43Z

⌛ Testing commit 8582f96 with merge 8196101a5b51fd4f5bb9235f6fd35822e8ce0c6d...

bors · 2022-11-29T16:31:56Z

💔 Test failed - checks-actions

oli-obk · 2022-11-29T16:34:48Z

@bors retry shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}

rust-log-analyzer · 2022-11-29T16:38:48Z

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

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

curl: (22) The requested URL returned error: 503 

error: failed to download llvm from ci

    help: old builds get deleted after a certain time
    help: if trying to compile an old commit of rustc, disable `download-ci-llvm` in config.toml:

    [llvm]
    download-ci-llvm = false
Build completed unsuccessfully in 0:00:52

bors · 2022-11-29T21:35:05Z

⌛ Testing commit 8582f96 with merge bddad59...

bors · 2022-11-30T00:42:37Z

☀️ Test successful - checks-actions
Approved by: fee1-dead
Pushing bddad59 to master...

rust-timer · 2022-11-30T03:03:58Z

Finished benchmarking commit (bddad59): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.6%, -0.2%]	12
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-0.6%, -0.2%]	12

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.2%	[0.7%, 2.3%]	9
Regressions ❌ (secondary)	3.2%	[0.9%, 5.0%]	10
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.4%	[-1.4%, -1.4%]	1
All ❌✅ (primary)	1.2%	[0.7%, 2.3%]	9

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.1%	[-7.1%, -7.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-7.1%	[-7.1%, -7.1%]	1

Also cache the stable hash of interned Predicates continuation of rust-lang#94299 This is a small perf improvement and shares more code between `Ty` and `Predicate`

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 1, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 1, 2022

rustbot added perf-regression Performance regression. 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 Mar 1, 2022

oli-obk force-pushed the stable_hash_ty branch from bc9aec1 to 8caeae4 Compare March 3, 2022 12:07

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 3, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 3, 2022

oli-obk mentioned this pull request Mar 5, 2022

Caching the stable hash of Ty within itself #94299

Merged

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 7, 2022

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 Nov 29, 2022

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 Nov 29, 2022

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 Nov 29, 2022

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 Nov 29, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 30, 2022

bors merged commit bddad59 into rust-lang:master Nov 30, 2022

rustbot added this to the 1.67.0 milestone Nov 30, 2022

rustbot removed the perf-regression Performance regression. label Nov 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Also cache the stable hash of interned Predicates #94487

Also cache the stable hash of interned Predicates #94487

oli-obk commented Mar 1, 2022 •

edited

Loading

oli-obk commented Mar 1, 2022

rust-timer commented Mar 1, 2022

bors commented Mar 1, 2022

bors commented Mar 1, 2022

rust-timer commented Mar 1, 2022

rust-timer commented Mar 1, 2022

oli-obk commented Mar 2, 2022 •

edited

Loading

oli-obk commented Mar 3, 2022

rust-timer commented Mar 3, 2022

bors commented Mar 3, 2022

bors commented Mar 3, 2022

rust-timer commented Mar 3, 2022

michaelwoerister commented Mar 3, 2022

rust-timer commented Mar 3, 2022

oli-obk commented Mar 7, 2022

rust-timer commented Mar 7, 2022

bors commented Mar 7, 2022

oli-obk commented Nov 29, 2022

fee1-dead commented Nov 29, 2022

oli-obk commented Nov 29, 2022

fee1-dead commented Nov 29, 2022

bors commented Nov 29, 2022

bors commented Nov 29, 2022

oli-obk commented Nov 29, 2022

rust-log-analyzer commented Nov 29, 2022

bors commented Nov 29, 2022

bors commented Nov 29, 2022

oli-obk commented Nov 29, 2022

rust-log-analyzer commented Nov 29, 2022

bors commented Nov 29, 2022

bors commented Nov 30, 2022

rust-timer commented Nov 30, 2022

Also cache the stable hash of interned Predicates #94487

Also cache the stable hash of interned Predicates #94487

Conversation

oli-obk commented Mar 1, 2022 • edited Loading

oli-obk commented Mar 1, 2022

rust-timer commented Mar 1, 2022

bors commented Mar 1, 2022

bors commented Mar 1, 2022

rust-timer commented Mar 1, 2022

rust-timer commented Mar 1, 2022

oli-obk commented Mar 2, 2022 • edited Loading

oli-obk commented Mar 3, 2022

rust-timer commented Mar 3, 2022

bors commented Mar 3, 2022

bors commented Mar 3, 2022

rust-timer commented Mar 3, 2022

michaelwoerister commented Mar 3, 2022

rust-timer commented Mar 3, 2022

oli-obk commented Mar 7, 2022

rust-timer commented Mar 7, 2022

bors commented Mar 7, 2022

oli-obk commented Nov 29, 2022

fee1-dead commented Nov 29, 2022

oli-obk commented Nov 29, 2022

fee1-dead commented Nov 29, 2022

bors commented Nov 29, 2022

bors commented Nov 29, 2022

oli-obk commented Nov 29, 2022

rust-log-analyzer commented Nov 29, 2022

bors commented Nov 29, 2022

bors commented Nov 29, 2022

oli-obk commented Nov 29, 2022

rust-log-analyzer commented Nov 29, 2022

bors commented Nov 29, 2022

bors commented Nov 30, 2022

rust-timer commented Nov 30, 2022

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

oli-obk commented Mar 1, 2022 •

edited

Loading

oli-obk commented Mar 2, 2022 •

edited

Loading