refactor: C++ implementation of ROOT file hasher#5614
Open
paulgessinger wants to merge 7 commits into
Open
Conversation
paulgessinger
force-pushed
the
refactor/cpp-root-hash
branch
2 times, most recently
from
3d31bbc to
9ef3bf9
Compare
Contributor
paulgessinger
force-pushed
the
refactor/cpp-root-hash
branch
from
9ef3bf9 to
eaa35a2
Compare
paulgessinger
force-pushed
the
refactor/cpp-root-hash
branch
from
eaa35a2 to
a333c0b
Compare
benjaminhuth
reviewed
Jul 1, 2026
benjaminhuth
left a comment
benjaminhuth
left a comment
Member
There was a problem hiding this comment.
Looks good, a few comments. Do we know how much faster it is?
Member
Author
|
@benjaminhuth i've found the C++ version runs in "constant" time (clearly it doesn't, but the file size in my benchmarks it didn't move). On the smallest files the perf is the same, for larger files I get speedups of 2x+ |
Member
Author
|
@benjaminhuth here are the benchmarks: |
Member
|
nice. 2s to 300ms is some good speedup. |
paulgessinger
force-pushed
the
refactor/cpp-root-hash
branch
from
db0d750 to
1f196f9
Compare
Collapse the two duplicated Hasher class definitions into one, guarding only the backend member and the two method bodies, and hoist the shared "using Digest" out of the backend #if/#else so only kDigestSize stays guarded there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Use std::vector<std::byte> instead of std::string for the raw-byte buffer fed to the hasher, matching the std::span<const std::byte> consumer and expressing the intent (raw bytes, not text). A small appendBytes helper replaces the repeated reinterpret_cast/append and drops the std::as_bytes laundering at the call sites. Byte content is unchanged, so hashes are identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the hand-rolled trim helper with boost::algorithm::trim_copy at the two firstTemplateArg call sites. Behaviourally equivalent for the type-name inputs parsed here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the firstTemplateArg helper into a detail header (RootFileHasherHelpers.hpp) so it can be exercised directly, and add a Boost unit test covering simple, nested, top-level-comma, whitespace and no-template cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
paulgessinger
force-pushed
the
refactor/cpp-root-hash
branch
from
1f196f9 to
88718a5
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an implementation of the ROOT file hashing in C++. The hashes are not identical, but the guarantees should be the same. It should be strictly faster than the Ptyhon implementation.
--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat,fix,refactor,docs,choreandbuildtypes.