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.

Sign up for GitHub

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

Jump to bottom

Implementation of new API, part 2 #514

Merged
merged 25 commits into from
Apr 19, 2023
Merged

Implementation of new API, part 2 #514

merged 25 commits into from
Apr 19, 2023

Conversation

flaub
Copy link
Member

@flaub flaub commented Apr 17, 2023 •
edited

This completes the new API so that execution and proving phases work together.

TODO:

  • Revive tracing and profiling support

mothran
mothran reviewed Apr 17, 2023
View reviewed changes
Copy link
Contributor

@mothran mothran left a comment

Choose a reason for hiding this comment

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

This diff appears to share large portions of the code with Part 1, would it be possible to PR this onto that branch or combine these PRs so its more clear what the new code is?

@flaub
Copy link
Member Author

flaub commented Apr 17, 2023

It's a draft/copy for me to move from one machine to another, will get cleaned up later

@flaub flaub changed the title WIP: Implementation of new API, part 2 Implementation of new API, part 2 Apr 18, 2023
@flaub flaub marked this pull request as ready for review April 18, 2023 06:39
@github-actions
Copy link

Benchmark for Linux-cuda d4083ff

Click to hide benchmark
Test Base PR %
fib/100/proof 1521.4±72.06ms 763.0±46.20ms -49.85%
fib/100/run 339.4±9.85ms 312.7±3.18ms -7.87%
fib/200/proof 1482.4±41.17ms 745.1±16.30ms -49.74%
fib/200/run 370.1±35.65ms 329.9±15.45ms -10.86%

Benchmark for Linux-default d4083ff

Click to hide benchmark
Test Base PR %
fib/100/proof 6.2±0.06s 2.3±0.03s -62.90%
fib/100/run 1420.3±13.60ms 322.0±7.88ms -77.33%
fib/200/proof 6.3±0.07s 2.3±0.01s -63.49%
fib/200/run 1425.8±1.93ms 325.7±1.88ms -77.16%

Benchmark for macOS-default d4083ff

Click to hide benchmark
Test Base PR %
fib/100/proof 3.9±0.05s 1587.8±9.05ms -59.29%
fib/100/run 686.3±1.18ms 110.4±0.60ms -83.91%
fib/200/proof 3.9±0.06s 1602.8±15.37ms -58.90%
fib/200/run 689.7±3.39ms 112.4±0.67ms -83.70%

Benchmark for macOS-metal d4083ff

Click to hide benchmark
Test Base PR %
fib/100/proof 1471.3±4.92ms 466.8±4.98ms -68.27%
fib/100/run 686.2±1.21ms 114.9±0.28ms -83.26%
fib/200/proof 1475.9±5.13ms 467.9±6.32ms -68.30%
fib/200/run 689.9±2.78ms 117.0±0.68ms -83.04%

@github-actions
Copy link

Benchmark for Linux-cuda fbf5100

Click to hide benchmark
Test Base PR %
fib/100/proof 1736.9±111.88ms 824.6±53.26ms -52.52%
fib/100/run 399.6±8.05ms 334.2±9.24ms -16.37%
fib/200/proof 1771.1±93.23ms 909.6±21.39ms -48.64%
fib/200/run 402.8±3.82ms 345.4±8.27ms -14.25%

Benchmark for Linux-default fbf5100

Click to hide benchmark
Test Base PR %
fib/100/proof 6.2±0.06s 2.3±0.03s -62.90%
fib/100/run 1427.0±15.75ms 326.0±5.22ms -77.15%
fib/200/proof 6.2±0.06s 2.3±0.01s -62.90%
fib/200/run 1427.1±3.84ms 332.9±6.28ms -76.67%

Benchmark for macOS-default fbf5100

Click to hide benchmark
Test Base PR %
fib/100/proof 3.9±0.03s 1593.6±13.42ms -59.14%
fib/100/run 686.5±1.24ms 110.9±0.17ms -83.85%
fib/200/proof 3.9±0.04s 1604.2±31.22ms -58.87%
fib/200/run 689.2±1.62ms 112.9±0.65ms -83.62%

Benchmark for macOS-metal fbf5100

Click to hide benchmark
Test Base PR %
fib/100/proof 1477.5±6.07ms 469.7±7.86ms -68.21%
fib/100/run 686.2±1.21ms 113.4±0.49ms -83.47%
fib/200/proof 1484.8±7.16ms 467.9±5.38ms -68.49%
fib/200/run 689.6±2.24ms 115.3±0.31ms -83.28%

@github-actions
Copy link

Benchmark for Linux-cuda 25e3808

Click to hide benchmark
Test Base PR %
fib/100/proof 1795.0±92.07ms 857.8±53.37ms -52.21%
fib/100/run 400.5±4.89ms 347.9±12.35ms -13.13%
fib/200/proof 1728.5±149.60ms 859.3±53.77ms -50.29%
fib/200/run 406.5±6.64ms 344.8±11.45ms -15.18%

Benchmark for Linux-default 25e3808

Click to hide benchmark
Test Base PR %
fib/100/proof 6.2±0.05s 2.3±0.02s -62.90%
fib/100/run 1414.7±3.20ms 321.7±1.42ms -77.26%
fib/200/proof 6.2±0.05s 2.3±0.01s -62.90%
fib/200/run 1423.5±2.15ms 331.2±6.50ms -76.73%

Benchmark for macOS-default 25e3808

Click to hide benchmark
Test Base PR %
fib/100/proof 3.9±0.05s 1598.8±12.35ms -59.01%
fib/100/run 686.8±2.04ms 111.2±0.90ms -83.81%
fib/200/proof 3.9±0.03s 1608.9±21.11ms -58.75%
fib/200/run 688.6±0.47ms 113.2±0.80ms -83.56%

Benchmark for macOS-metal 25e3808

Click to hide benchmark
Test Base PR %
fib/100/proof 1477.0±3.36ms 469.1±11.00ms -68.24%
fib/100/run 687.7±1.77ms 114.1±1.75ms -83.41%
fib/200/proof 1481.2±6.05ms 469.2±4.27ms -68.32%
fib/200/run 689.6±1.93ms 116.1±1.34ms -83.16%

@github-actions
Copy link

Benchmark for Linux-cuda b5545f3

Click to hide benchmark
Test Base PR %
fib/100/proof 1480.2±30.15ms 734.8±11.10ms -50.36%
fib/100/run 312.7±2.37ms 309.4±3.53ms -1.06%
fib/200/proof 1499.6±30.49ms 753.7±29.64ms -49.74%
fib/200/run 319.8±4.00ms 312.8±5.64ms -2.19%

Benchmark for Linux-default b5545f3

Click to hide benchmark
Test Base PR %
fib/100/proof 6.2±0.06s 2.3±0.02s -62.90%
fib/100/run 1413.1±6.00ms 319.2±2.40ms -77.41%
fib/200/proof 6.2±0.05s 2.3±0.01s -62.90%
fib/200/run 1418.4±3.05ms 323.9±3.84ms -77.16%

Benchmark for macOS-default b5545f3

Click to hide benchmark
Test Base PR %
fib/100/proof 3.9±0.05s 1592.0±4.85ms -59.18%
fib/100/run 686.1±1.25ms 110.8±0.66ms -83.85%
fib/200/proof 3.9±0.05s 1595.8±9.42ms -59.08%
fib/200/run 689.5±2.30ms 112.8±0.57ms -83.64%

Benchmark for macOS-metal b5545f3

Click to hide benchmark
Test Base PR %
fib/100/proof 1474.6±5.04ms 462.4±4.28ms -68.64%
fib/100/run 686.1±1.21ms 113.8±0.55ms -83.41%
fib/200/proof 1476.4±5.78ms 465.2±4.04ms -68.49%
fib/200/run 689.4±2.10ms 115.7±0.50ms -83.22%

mothran
mothran reviewed Apr 18, 2023
View reviewed changes
risc0/zkvm/src/session.rs Outdated Show resolved Hide resolved
@flaub @mothran
Update risc0/zkvm/src/session.rs
404f63c 
Co-authored-by: Parker Thompson <parker@risczero.com>
mothran
mothran reviewed Apr 18, 2023
View reviewed changes
risc0/zkvm/src/session.rs Outdated Show resolved Hide resolved
@github-actions
Copy link

Benchmark for Linux-cuda 22a12ca

Click to hide benchmark
Test Base PR %
fib/100/proof 1590.3±171.51ms 816.6±43.67ms -48.65%
fib/100/run 349.5±10.91ms 318.0±1.76ms -9.01%
fib/200/proof 1524.4±54.39ms 755.0±45.32ms -50.47%
fib/200/run 356.1±36.35ms 345.4±17.27ms -3.00%

Benchmark for Linux-default 22a12ca

Click to hide benchmark
Test Base PR %
fib/100/proof 6.2±0.07s 2.3±0.02s -62.90%
fib/100/run 1418.1±3.93ms 323.2±2.96ms -77.21%
fib/200/proof 6.2±0.06s 2.3±0.01s -62.90%
fib/200/run 1435.3±13.80ms 330.0±4.74ms -77.01%

Benchmark for macOS-default 22a12ca

Click to hide benchmark
Test Base PR %
fib/100/proof 3.9±0.05s 1589.6±6.71ms -59.24%
fib/100/run 686.2±1.13ms 111.2±0.38ms -83.79%
fib/200/proof 3.9±0.04s 1604.1±24.46ms -58.87%
fib/200/run 689.4±1.24ms 113.3±0.85ms -83.57%

Benchmark for macOS-metal 22a12ca

Click to hide benchmark
Test Base PR %
fib/100/proof 1476.4±7.16ms 463.3±5.47ms -68.62%
fib/100/run 687.4±2.24ms 113.3±0.84ms -83.52%
fib/200/proof 1486.9±6.71ms 466.6±3.52ms -68.62%
fib/200/run 689.5±2.27ms 114.9±0.53ms -83.34%

mothran
mothran approved these changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@mothran mothran left a comment

Choose a reason for hiding this comment

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

Looking great! Makes sense to continue follow up work in additional PRs.

@github-actions
Copy link

Benchmark for Linux-cuda a156d54

Click to hide benchmark
Test Base PR %
fib/100/proof 1805.3±155.07ms 850.6±48.52ms -52.88%
fib/100/run 424.5±4.75ms 342.6±21.44ms -19.29%
fib/200/proof 1705.6±150.11ms 871.6±73.53ms -48.90%
fib/200/run 345.1±6.82ms 328.9±3.20ms -4.69%

Benchmark for Linux-default a156d54

Click to hide benchmark
Test Base PR %
fib/100/proof 6.2±0.05s 2.3±0.03s -62.90%
fib/100/run 1411.7±3.77ms 327.7±0.72ms -76.79%
fib/200/proof 6.2±0.05s 2.3±0.02s -62.90%
fib/200/run 1423.0±3.06ms 336.1±1.52ms -76.38%

Benchmark for macOS-default a156d54

Click to hide benchmark
Test Base PR %
fib/100/proof 3.9±0.05s 1596.0±9.56ms -59.08%
fib/100/run 686.7±2.27ms 111.6±0.62ms -83.75%
fib/200/proof 3.9±0.04s 1600.2±7.61ms -58.97%
fib/200/run 690.5±4.15ms 113.4±0.70ms -83.58%

Benchmark for macOS-metal a156d54

Click to hide benchmark
Test Base PR %
fib/100/proof 1476.8±5.49ms 469.3±7.02ms -68.22%
fib/100/run 687.2±2.49ms 115.0±0.86ms -83.27%
fib/200/proof 1479.9±6.81ms 469.4±4.63ms -68.28%
fib/200/run 689.3±1.34ms 116.9±0.64ms -83.04%

SchmErik
SchmErik approved these changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@SchmErik SchmErik left a comment

Choose a reason for hiding this comment

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

LGTM!

@flaub
Copy link
Member Author

flaub commented Apr 19, 2023

Thanks for all the comments!

@flaub flaub merged commit 0e60c27 into main Apr 19, 2023
10 checks passed
@flaub flaub deleted the flaub/api2 branch April 19, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mothran mothran mothran approved these changes

@SchmErik SchmErik SchmErik approved these changes

@shkoo shkoo Awaiting requested review from shkoo

@jbruestle jbruestle Awaiting requested review from jbruestle

@tzerrell tzerrell Awaiting requested review from tzerrell

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@flaub @mothran @SchmErik