refactor: avoid double hashing in SourceLocationHasher #32939

pull l0rinc wants to merge 1 commits into bitcoin:master from l0rinc:l0rinc/loggin-hasher changing 1 files +6 −6
  1. l0rinc commented at 12:46 pm on July 10, 2025: contributor

    Feeds the file name string directly into the hasher instead of pre-hashing it first. The resulting hash changes of course, but that doesn’t affect us since it’s only stored in-memory.

    Fixes: #32604 (review)

    A few other related nits were also addressed here:

    • Comment was redundant, CSipHasher is used in other hashers all over the codebase
    • static_cast<size_t> skewed the whole method, changed to shorter functional-style cast
    • struct formatting is aligned with class formatting - see https://github.com/bitcoin/bitcoin/pull/32813
  2. DrahtBot added the label Refactoring on Jul 10, 2025
  3. DrahtBot commented at 12:47 pm on July 10, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32939.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

  4. refactor: avoid double hashing in `SourceLocationHasher`
    Feeds the file name string directly into the hasher instead of pre-hashing it first. The resulting hash changes of course, but that doesn't affect us since it's only stored in-memory.
    
    Fixes: https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2188644325
    
    A few other related nits were also addressed here:
    * Comment was redundant, `CSipHasher` is used in other hashers all over the codebase
    * `static_cast<size_t>` skewed the whole method, changed to shorter functional-style cast
    * struct formatting is aligned with class formatting - see https://github.com/bitcoin/bitcoin/pull/32813
    8a618fba55
  5. l0rinc force-pushed on Jul 10, 2025
  6. in src/logging.h:48 in 8a618fba55
    41@@ -42,14 +42,14 @@ struct SourceLocationEqual {
    42     }
    43 };
    44 
    45-struct SourceLocationHasher {
    46+struct SourceLocationHasher
    47+{
    48     size_t operator()(const std::source_location& s) const noexcept
    49     {
    50-        // Use CSipHasher(0, 0) as a simple way to get uniform distribution.
    


    stickies-v commented at 1:10 pm on July 10, 2025:
    As we’ve already discussed here, I don’t think it would be an improvement to remove this comment. Using CSipHasher over std::hash is non-trivial and this comment helped me understand the code.
  7. stickies-v commented at 1:11 pm on July 10, 2025: contributor
    Since there are quite a few follow-ups in #32604, it might make sense to do them all in one (or at least larger, related PRs) instead of carving it out into dozens of small ones?
  8. l0rinc closed this on Jul 10, 2025


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-11 12:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me