avm: solve concurrency issue in death tests #7504
Assignees
Comments
fcarreiro
added a commit
that referenced
this issue
Jul 17, 2024
This PR makes some changes to use the more standard `bb::parallel_for` instead of or custom solution. This is nice because we can now control/experiment with concurrency limit by using the env variable `HARDWARE_CONCURRENCY` (which defaults to the number of cpus). I also added parallel computation of logderivative inverses **in the prover** which actually made a huge difference: took it from `5.1s` to `0.1`s and has now become negligible (with the # of cpus of the mainframe). BEFORE ``` prove/check_circuit: 5120 prove/execute_log_derivative_inverse_commitments_round_ms: 532 *** prove/execute_log_derivative_inverse_round_ms: 5199 prove/execute_pcs_rounds_ms: 413 prove/execute_relation_check_rounds_ms: 1328 prove/execute_wire_commitments_round_ms: 1742 prove/gen_trace: 850 ``` AFTER ``` prove/check_circuit: 4859 prove/execute_log_derivative_inverse_commitments_round_ms: 543 *** prove/execute_log_derivative_inverse_round_ms: 162 prove/execute_pcs_rounds_ms: 381 prove/execute_relation_check_rounds_ms: 1089 prove/execute_wire_commitments_round_ms: 1608 prove/gen_trace: 755 ``` --------- WARNING: I had to update the handling of exception catching in the tests, because things get complicated w/threads. I mostly just changed the helper, but GTest does complain and we have to do sth about it eventually. > [WARNING] /mnt/user-data/facundo/aztec-packages/barretenberg/cpp/build/_deps/gtest-src/googletest/src/gtest-death-test.cc:1108:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 192 threads. See https://github.com/google/googletest/blob/main/docs/advanced.md#death-tests-and-threads for more explanation and suggested solutions, especially if this is the last message you see before your test times out. Filed [this issue](#7504).
AztecBot
pushed a commit
to AztecProtocol/barretenberg
that referenced
this issue
Jul 18, 2024
This PR makes some changes to use the more standard `bb::parallel_for` instead of or custom solution. This is nice because we can now control/experiment with concurrency limit by using the env variable `HARDWARE_CONCURRENCY` (which defaults to the number of cpus). I also added parallel computation of logderivative inverses **in the prover** which actually made a huge difference: took it from `5.1s` to `0.1`s and has now become negligible (with the # of cpus of the mainframe). BEFORE ``` prove/check_circuit: 5120 prove/execute_log_derivative_inverse_commitments_round_ms: 532 *** prove/execute_log_derivative_inverse_round_ms: 5199 prove/execute_pcs_rounds_ms: 413 prove/execute_relation_check_rounds_ms: 1328 prove/execute_wire_commitments_round_ms: 1742 prove/gen_trace: 850 ``` AFTER ``` prove/check_circuit: 4859 prove/execute_log_derivative_inverse_commitments_round_ms: 543 *** prove/execute_log_derivative_inverse_round_ms: 162 prove/execute_pcs_rounds_ms: 381 prove/execute_relation_check_rounds_ms: 1089 prove/execute_wire_commitments_round_ms: 1608 prove/gen_trace: 755 ``` --------- WARNING: I had to update the handling of exception catching in the tests, because things get complicated w/threads. I mostly just changed the helper, but GTest does complain and we have to do sth about it eventually. > [WARNING] /mnt/user-data/facundo/aztec-packages/barretenberg/cpp/build/_deps/gtest-src/googletest/src/gtest-death-test.cc:1108:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 192 threads. See https://github.com/google/googletest/blob/main/docs/advanced.md#death-tests-and-threads for more explanation and suggested solutions, especially if this is the last message you see before your test times out. Filed [this issue](AztecProtocol/aztec-packages#7504).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
From #7495
The text was updated successfully, but these errors were encountered: