Skip to content

chore: Switch to rustc-hash#9064

Closed
jfecher wants to merge 16 commits intomasterfrom
jf/rustc-hash
Closed

chore: Switch to rustc-hash#9064
jfecher wants to merge 16 commits intomasterfrom
jf/rustc-hash

Conversation

@jfecher
Copy link
Contributor

@jfecher jfecher commented Jun 30, 2025

Description

Problem*

We depend on fxhash which is no longer maintained and has been superseded by rustc-hash

Summary*

This PR:

  • Replaces uses of fxhash with rustc-hash
  • Adds util::hash64 to provide a utility function missing from rustc-hash which we use in several crates.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@jfecher jfecher requested a review from a team June 30, 2025 17:49
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: fd66013 Previous: 11e956e Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 186 s 143 s 1.30

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@jfecher
Copy link
Contributor Author

jfecher commented Jun 30, 2025

Seems like some fairly large test suite duration regressions

@jfecher jfecher added the bench-show Display benchmark results on PR label Jun 30, 2025
@vezenovm
Copy link
Contributor

Seems like some fairly large test suite duration regressions

Some of these times such as for aztec-nr and types seem to line up with what I see in my PR #9060 (review). Makes me think that perhaps something else is up.

@vezenovm
Copy link
Contributor

vezenovm commented Jul 1, 2025

Some of these times such as for aztec-nr and types seem to line up with what I see in my PR #9060 (review). Makes me think that perhaps something else is up.

Yeah something is definitely up with the alerts #9068 (review). I am very surprised to see an alert on #9068. cc @TomAFrench

@TomAFrench
Copy link
Member

I was just taking a look at this. I'm not entirely sure what's up as there's been no bumps to the pinned commit in the period from when these branches were made.

@TomAFrench
Copy link
Member

Huh, actually it looks like it's using a commit from ~2 weeks ago as the baseline.

@TomAFrench
Copy link
Member

@vezenovm the fix is in #9069

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

ACVM Benchmarks

Details
Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
purely_sequential_opcodes 252031 ns/iter (± 811) 251347 ns/iter (± 1144) 1.00
perfectly_parallel_opcodes 220103 ns/iter (± 2297) 220657 ns/iter (± 1306) 1.00
perfectly_parallel_batch_inversion_opcodes 2783531 ns/iter (± 1155) 2784923 ns/iter (± 5526) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Compilation Time

Details
Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
private-kernel-inner 2.488 s 2.464 s 1.01
private-kernel-reset 8.214 s 8.052 s 1.02
private-kernel-tail 1.15 s 1.168 s 0.98
rollup-base-private 18.6 s 18.06 s 1.03
rollup-base-public 14.78 s 15.18 s 0.97
rollup-block-root-empty 21.48 s 21.2 s 1.01
rollup-block-root-single-tx 203 s 200 s 1.01
rollup-block-root 197 s 205 s 0.96
rollup-merge 1.292 s 1.32 s 0.98
rollup-root 1.402 s 1.458 s 0.96
semaphore-depth-10 0.764 s 0.784 s 0.97

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Artifact Size

Details
Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
private-kernel-inner 1127.4 KB 1127.4 KB 1
private-kernel-reset 2068 KB 2067.9 KB 1.00
private-kernel-tail 585 KB 585 KB 1
rollup-base-private 4934.4 KB 4934.4 KB 1
rollup-base-public 3975.6 KB 3975.6 KB 1
rollup-block-root-empty 3871.1 KB 3871.1 KB 1
rollup-block-root-single-tx 32733.4 KB 32733.4 KB 1
rollup-block-root 32752.7 KB 32752.7 KB 1
rollup-merge 176.8 KB 176.8 KB 1
rollup-root 389.5 KB 389.5 KB 1
semaphore-depth-10 636.4 KB 636.4 KB 1

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Execution Time

Details
Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
private-kernel-inner 0.025 s 0.026 s 0.96
private-kernel-reset 0.161 s 0.161 s 1
private-kernel-tail 0.01 s 0.011 s 0.91
rollup-base-private 0.297 s 0.295 s 1.01
rollup-base-public 0.18 s 0.184 s 0.98
rollup-block-root 12.9 s 13.4 s 0.96
rollup-merge 0.003 s 0.002 s 1.50
rollup-root 0.004 s 0.004 s 1
semaphore-depth-10 0.019 s 0.02 s 0.95

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Test Suite Duration

Details
Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr 100 s 101 s 0.99
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts 146 s 155 s 0.94
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 179 s 185 s 0.97
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 219 s 221 s 0.99
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib 640 s 713 s 0.90
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types 93 s 93 s 1
test_report_noir-lang_noir-bignum_ 380 s 378 s 1.01
test_report_noir-lang_noir_bigcurve_ 273 s 269 s 1.01
test_report_noir-lang_sha512_ 17 s 15 s 1.13
test_report_zkpassport_noir-ecdsa_ 119 s 116 s 1.03
test_report_zkpassport_noir_rsa_ 2 s 2 s 1

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Execution Memory

Details
Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
private-kernel-inner 202.71 MB 202.71 MB 1
private-kernel-reset 226.5 MB 226.5 MB 1
private-kernel-tail 178.04 MB 178.04 MB 1
rollup-base-private 508.01 MB 508.01 MB 1
rollup-base-public 441.22 MB 441.22 MB 1
rollup-block-root 1510 MB 1510 MB 1
rollup-merge 330.09 MB 330.09 MB 1
rollup-root 332.7 MB 332.7 MB 1
semaphore_depth_10 71.01 MB 71.01 MB 1

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Compilation Memory

Details
Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
private-kernel-inner 280.11 MB 280.19 MB 1.00
private-kernel-reset 534.01 MB 534.01 MB 1
private-kernel-tail 184.58 MB 184.58 MB 1
rollup-base-private 1400 MB 1400 MB 1
rollup-base-public 1530 MB 1530 MB 1
rollup-block-root-empty 1090 MB 1090 MB 1
rollup-block-root-single-tx 9380 MB 9380 MB 1
rollup-block-root 9390 MB 9390 MB 1
rollup-merge 333.56 MB 333.56 MB 1
rollup-root 343.6 MB 343.6 MB 1
semaphore_depth_10 106.39 MB 106.39 MB 1

This comment was automatically generated by workflow using github-action-benchmark.

@jfecher jfecher enabled auto-merge July 1, 2025 16:57
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 63818f4 Previous: d356b92 Ratio
rollup-merge 0.003 s 0.002 s 1.50

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@TomAFrench
Copy link
Member

Closing this as I've made this change in #9750

@TomAFrench TomAFrench closed this Sep 8, 2025
auto-merge was automatically disabled September 8, 2025 09:46

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bench-show Display benchmark results on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants