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

bench against libb2 #458

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

bench against libb2 #458

wants to merge 12 commits into from

Conversation

msprotz
Copy link
Collaborator

@msprotz msprotz commented Apr 20, 2024

As part of python/cpython#99108 I would like to gather some data as to the relative performance of HACL*'s blake2 implementation against libb2.

To that effect, I added cmake voodoo, and a few benchmarks.

Sadly, I'm getting a segfault, and I can't debug because I don't know how to build the benchmarks with debugging info.

@franziskuskiefer can you take a look at tell me i) whether you see any issue and/or ii) how to build benchmarks in debug mode so that I can at least try to figure out what's going on?

thanks!

@msprotz msprotz requested a review from a team as a code owner April 20, 2024 00:31
@cla-bot cla-bot bot added the cla-signed label Apr 20, 2024
@coveralls
Copy link

coveralls commented Apr 20, 2024

Pull Request Test Coverage Report for Build 9588066057

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 48.135%

Totals Coverage Status
Change from base Build 9565668261: 0.0%
Covered Lines: 29403
Relevant Lines: 61085

💛 - Coveralls

@franziskuskiefer
Copy link
Member

@franziskuskiefer can you take a look at tell me i) whether you see any issue and/or ii) how to build benchmarks in debug mode so that I can at least try to figure out what's going on?

There may be a symbol clash with all the blake2b names? Other than that, this should work.

There's no flag to add debug symbols for benchmarks right now. You could add one. Otherwise, just throw in a -g in the CMakeLists.txt to get them quickly.

@msprotz
Copy link
Collaborator Author

msprotz commented Apr 22, 2024

Ok I've just gotten segfaults on my machine, but indeed, might be symbol clashes... if the code is the same between libb2 and blake2-ref (with the exception that the former does runtime cpu detection), then it might be easier to just benchmark blake2-ref with #defines for working around the symbol conflicts (or a good old sed).

Copy link

cla-bot bot commented Jun 18, 2024

We require contributors to sign our Contributor License Agreement https://github.com/cryspen/hacl/blob/main/CLA.md ensuring that the contribution can be licensed under Apache 2.0 and MIT. In order for us to review and merge your code, please mention @cryspen/core in a comment below to get yourself added.

@cla-bot cla-bot bot removed the cla-signed label Jun 18, 2024
@cla-bot cla-bot bot added the cla-signed label Jun 18, 2024
Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

lgtm with that one comment.

@@ -288,28 +344,6 @@ HACL_blake2b_32_streaming(benchmark::State& state)

BENCHMARK(HACL_blake2b_32_streaming)->Setup(DoSetup);

static void
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to drop these two?
@karthikbhargavan looks like you did this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

6 participants