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

Update benchmark to skip chunk sizes that doesn't work with the algorithm #2146

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andrewhop
Copy link
Contributor

Description of changes:

AES XTS requires at least 16 bytes of input, all other algorithms support 1 or more bytes. This change also updates the remaining algorithm benchmarks to support any sized chunks.

Testing:

Updated run_benchmark_build_tests.sh to test every libcrypto benchmark with 1 to 20,000 bytes.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

…ithm.

Use dynamically allocated input/scratch space to support any chunk size.
@andrewhop andrewhop requested a review from a team as a code owner January 29, 2025 00:10
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.97%. Comparing base (ea58b3f) to head (4fd39fd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2146      +/-   ##
==========================================
- Coverage   78.97%   78.97%   -0.01%     
==========================================
  Files         611      611              
  Lines      105500   105500              
  Branches    14938    14938              
==========================================
- Hits        83316    83315       -1     
- Misses      21531    21533       +2     
+ Partials      653      652       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +97 to +105
LD_LIBRARY_PATH="${install_dir}/aws-lc-fips-2021-10-20/lib" "${BUILD_ROOT}/tool/aws-lc-fips-2021" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/aws-lc-fips-2022-11-02/lib" "${BUILD_ROOT}/tool/aws-lc-fips-2022" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/aws-lc-fips-2024-09-27/lib" "${BUILD_ROOT}/tool/aws-lc-fips-2022" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/openssl-${openssl_1_0_2_branch}/lib" "${BUILD_ROOT}/tool/open102" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/openssl-${openssl_1_1_1_branch}/lib" "${BUILD_ROOT}/tool/open111" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/openssl-${openssl_3_1_branch}/lib64" "${BUILD_ROOT}/tool/open31" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/openssl-${openssl_3_2_branch}/lib64" "${BUILD_ROOT}/tool/open32" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/openssl-${openssl_master_branch}/lib64" "${BUILD_ROOT}/tool/openmaster" -timeout_ms 10 -chunks 1,2,16,256,20000
LD_LIBRARY_PATH="${install_dir}/boringssl" "${BUILD_ROOT}/tool/boringssl" -timeout_ms 10 -chunks 1,2,16,256,20000
Copy link
Contributor

@justsmth justsmth Jan 29, 2025

Choose a reason for hiding this comment

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

NP: Could these chunk sizes just be set to the default so that we don't have to specify them for every invocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to change the actual default chunk size, I picked these values to test interesting inputs that previously caused the benchmark to fail.

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.

3 participants