-
Notifications
You must be signed in to change notification settings - Fork 1.1k
test: split monolithic ellswift test into independent cases #1788
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
test: split monolithic ellswift test into independent cases #1788
Conversation
real-or-random
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK f12e2e0
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f12e2e0, I have reviewed the code and it looks OK. Tested on Ubuntu 25.10.
Happy to re-ACK once #1788 (comment) is addressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is log on my machine:
$ ./build/bin/tests -j=16 -t=ellswift -log=1
iterations = 16
jobs = 16. Parallel execution.
random seed = 3ccb907b6b59142af5e562fdf21e2a62
Running ellswift_encoding_test_vectors_tests..
Running ellswift_xdh_test_vectors_tests..
Running ellswift_decoding_test_vectors_tests..
Running ellswift_compute_shared_secret_tests..
Running ellswift_encode_decode_roundtrip_tests..
Running test_ellswift_xdh_correctness..
Running ellswift_hash_init_tests..
Running ellswift_create_tests..
Test ellswift_hash_init_tests PASSED (0.000 sec)
Test ellswift_xdh_test_vectors_tests PASSED (0.002 sec)
Test ellswift_decoding_test_vectors_tests PASSED (0.002 sec)
Test ellswift_encoding_test_vectors_tests PASSED (0.012 sec)
Test ellswift_create_tests PASSED (1.424 sec)
Test ellswift_compute_shared_secret_tests PASSED (2.448 sec)
Test ellswift_encode_decode_roundtrip_tests PASSED (2.702 sec)
Test test_ellswift_xdh_correctness PASSED (4.505 sec)
Total execution time: 4.509 seconds
Could we re-arrange test cases to run the longest ones first?
No behavior changes. Refactors the previously monolithic ElligatorSwift test into isolated, independent test cases. Doing so allows the test suite to execute these cases in parallel rather than sequentially. Overall, seen 35-40% tests time reduction locally. This is quite useful for the Debug build with no optimizations, which is noticeably slow. #### Local Debug-build Results (7 jobs): - master: 138.0 seconds. - this PR: 89.3 seconds. (~1.55× speedup, ~35% reduction) #### Local Release-build Results (7 jobs): - master: 9.5 seconds. - this PR: 5.9 seconds. (~1.61× speedup, ~38% reduction)
f12e2e0 to
d822b29
Compare
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK d822b29.
We could do it, but it wouldn’t make much of a difference atm. The ellswift module sits deep in the test registry (see). We should move the module up too. Which is really a small change. Yet, it might be worth considering adding |
theStack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK d822b29
Nice!
$ ./build/bin/tests -t=ellswift -log=1 -j=8
iterations = 16
jobs = 8. Parallel execution.
random seed = 81780fc09f616932c95e8182acab17d9
Running ellswift_encoding_test_vectors_tests..
Running ellswift_decoding_test_vectors_tests..
Running ellswift_xdh_test_vectors_tests..
Running ellswift_create_tests..
Running ellswift_compute_shared_secret_tests..
Running ellswift_encode_decode_roundtrip_tests..
Running ellswift_xdh_correctness_tests..
Running ellswift_hash_init_tests..
Test ellswift_hash_init_tests PASSED (0.000 sec)
Test ellswift_xdh_test_vectors_tests PASSED (0.001 sec)
Test ellswift_decoding_test_vectors_tests PASSED (0.002 sec)
Test ellswift_encoding_test_vectors_tests PASSED (0.012 sec)
Test ellswift_create_tests PASSED (1.157 sec)
Test ellswift_compute_shared_secret_tests PASSED (2.049 sec)
Test ellswift_encode_decode_roundtrip_tests PASSED (2.156 sec)
Test ellswift_xdh_correctness_tests PASSED (3.674 sec)
Total execution time: 3.675 seconds
No behavior changes.
Refactors the previously monolithic ElligatorSwift test into isolated,
independent test cases. Doing so allows the test suite to execute
these cases in parallel rather than sequentially.
Overall, I'm seeing 35-40% tests time reduction locally.
This is quite useful for Debug builds with no optimizations,
which are noticeably slow.
Local Debug-build Results (7 jobs):
(~1.55× speedup, ~35% reduction)
Local Release-build Results (7 jobs):
(~1.61× speedup, ~38% reduction)
rp5 Release-build results (5 jobs):
(~1.62× speedup, ~38% reduction)