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

Fixes risc0-build deadlocks on large outputs #498

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

mothran
Copy link
Contributor

@mothran mothran commented Apr 5, 2023

Previously if we had a large volume of cargo build errors it would hit the piped limit of the child process terminate. By calling cmd.status() we would invoke a second version of the subprocess. Using the spawn() return value and .wait() means we correctly pull from the right piped IO.

This might have also fixed double-invoking the cargo build child process but I am not 100% sure on that.

@mothran mothran requested review from flaub and shkoo April 5, 2023 01:23
@github-actions
Copy link

github-actions bot commented Apr 5, 2023

Benchmark for Linux-cuda 75910c6

Click to hide benchmark
Test Base PR %
fib/100/proof 855.2±40.81ms 820.6±45.43ms -4.05%
fib/100/run 337.6±9.64ms 328.1±10.15ms -2.81%
fib/200/proof 850.2±50.34ms 800.4±38.64ms -5.86%
fib/200/run 332.2±7.86ms 325.7±6.96ms -1.96%

Benchmark for Linux-default 75910c6

Click to hide benchmark
Test Base PR %
fib/100/proof 2.3±0.02s 2.3±0.02s 0.00%
fib/100/run 344.6±10.14ms 331.2±0.90ms -3.89%
fib/200/proof 2.3±0.02s 2.3±0.02s 0.00%
fib/200/run 351.8±19.91ms 334.9±2.50ms -4.80%

Benchmark for macOS-default 75910c6

Click to hide benchmark
Test Base PR %
fib/100/proof 1593.4±9.41ms 1593.2±9.39ms -0.01%
fib/100/run 110.3±0.59ms 110.3±0.35ms 0.00%
fib/200/proof 1609.2±29.00ms 1602.2±15.23ms -0.43%
fib/200/run 112.1±0.61ms 112.0±0.44ms -0.09%

Benchmark for macOS-metal 75910c6

Click to hide benchmark
Test Base PR %
fib/100/proof 464.9±4.46ms 461.5±3.17ms -0.73%
fib/100/run 113.7±0.45ms 111.9±0.89ms -1.58%
fib/200/proof 467.2±4.03ms 465.4±3.95ms -0.39%
fib/200/run 115.4±0.33ms 113.9±0.55ms -1.30%

@mothran mothran merged commit 96deb85 into main Apr 5, 2023
10 checks passed
@mothran mothran deleted the parker/fix-risc0-build-hangs branch April 5, 2023 15:41
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

2 participants