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

Hash function cleanups #493

Merged
merged 7 commits into from
Apr 5, 2023
Merged

Hash function cleanups #493

merged 7 commits into from
Apr 5, 2023

Conversation

flaub
Copy link
Member

@flaub flaub commented Apr 1, 2023

  • Move hashing functions into risc0_zkp::hash
  • Rename ConfigHash to HashFn
  • Renamed ConfigRng to Rng

* Move hashing functions into `risc0_zkp::hash`
* Rename `ConfigHash` to `HashFn`
* Renamed `ConfigRng` to `Rng`
@flaub flaub self-assigned this Apr 1, 2023
@github-actions
Copy link

github-actions bot commented Apr 1, 2023

Benchmark for Linux-cuda c4fa2f7

Click to hide benchmark
Test Base PR %
fib/100/proof 1110.0±27.72ms 797.7±8.94ms -28.14%
fib/100/run 761.9±3.25ms 404.0±2.50ms -46.97%
fib/200/proof 1142.7±5.62ms 807.2±4.80ms -29.36%
fib/200/run 766.1±3.21ms 407.2±2.64ms -46.85%

Benchmark for Linux-default c4fa2f7

Click to hide benchmark
Test Base PR %
fib/100/proof 2.3±0.01s 2.3±0.02s 0.00%
fib/100/run 328.9±6.83ms 325.9±5.26ms -0.91%
fib/200/proof 2.3±0.03s 2.3±0.01s 0.00%
fib/200/run 330.4±5.54ms 330.0±4.27ms -0.12%

Benchmark for macOS-default c4fa2f7

Click to hide benchmark
Test Base PR %
fib/100/proof 1598.4±6.41ms 1595.7±10.08ms -0.17%
fib/100/run 110.2±0.59ms 110.1±0.75ms -0.09%
fib/200/proof 1599.9±12.38ms 1598.4±6.89ms -0.09%
fib/200/run 112.3±0.62ms 112.3±1.30ms 0.00%

Benchmark for macOS-metal c4fa2f7

Click to hide benchmark
Test Base PR %
fib/100/proof 467.1±6.49ms 457.5±5.18ms -2.06%
fib/100/run 113.7±0.36ms 112.1±0.72ms -1.41%
fib/200/proof 466.4±4.33ms 465.3±8.00ms -0.24%
fib/200/run 115.3±0.29ms 113.9±0.49ms -1.21%

Copy link
Member

@tzerrell tzerrell left a comment

Choose a reason for hiding this comment

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

The API adjustments seem reasonable to me.

I can let people more involved in the implementation speak up in their reviews if they have any concerns with how the implementations were moved / adjusted.

@flaub flaub enabled auto-merge (squash) April 5, 2023 16:38
@github-actions
Copy link

github-actions bot commented Apr 5, 2023

Benchmark for Linux-cuda 5e2b3c6

Click to hide benchmark
Test Base PR %
fib/100/proof 737.9±22.78ms 727.4±19.88ms -1.42%
fib/100/run 331.4±11.36ms 314.6±8.65ms -5.07%
fib/200/proof 758.1±49.35ms 736.0±17.89ms -2.92%
fib/200/run 332.0±14.07ms 319.8±12.13ms -3.67%

Benchmark for Linux-default 5e2b3c6

Click to hide benchmark
Test Base PR %
fib/100/proof 2.3±0.03s 2.3±0.02s 0.00%
fib/100/run 333.8±15.13ms 320.2±1.03ms -4.07%
fib/200/proof 2.3±0.01s 2.3±0.03s 0.00%
fib/200/run 339.5±5.12ms 324.0±7.16ms -4.57%

Benchmark for macOS-default 5e2b3c6

Click to hide benchmark
Test Base PR %
fib/100/proof 1599.5±14.19ms 1598.6±8.43ms -0.06%
fib/100/run 110.5±0.63ms 109.9±0.51ms -0.54%
fib/200/proof 1601.8±15.74ms 1601.0±9.17ms -0.05%
fib/200/run 112.6±0.53ms 111.7±0.52ms -0.80%

Benchmark for macOS-metal 5e2b3c6

Click to hide benchmark
Test Base PR %
fib/100/proof 465.5±6.30ms 459.9±5.87ms -1.20%
fib/100/run 112.4±0.48ms 111.2±0.50ms -1.07%
fib/200/proof 468.2±6.40ms 459.5±5.23ms -1.86%
fib/200/run 114.6±0.52ms 113.3±0.60ms -1.13%

@flaub flaub merged commit 722e890 into main Apr 5, 2023
10 checks passed
@flaub flaub deleted the flaub/cleanup branch April 5, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants