Improve base_blob<BITS>::IsNull() #16102

pull promag wants to merge 3 commits into bitcoin:master from promag:2010-05-baseblob-isnull changing 4 files +27 −7

promag commented at 3:06 PM on May 27, 2019: member

This change improves uint256::IsNull() and uint160::IsNull(), especially for the positive case.

By running the new benchmark ./src/bench/bench_bitcoin -filter=UInt256IsNull:

Before:

# Benchmark, evals, iterations, total, min, max, median
UInt256IsNull, 5, 600000000, 15.3553, 5.0397e-09, 5.35092e-09, 5.05512e-09

After:

# Benchmark, evals, iterations, total, min, max, median
UInt256IsNull, 5, 600000000, 3.56288, 1.18346e-09, 1.19433e-09, 1.18635e-09

bench: Add benchmark for uint256::IsNull 98a70ee7d3
Improve base_blob<BITS>::IsNull() 17b97f8e8e
fanquake added the label Refactoring on May 27, 2019
fixup: Rename base_blob<BITS>::WIDTH constants cd46643660

in src/uint256.h:22 in 17b97f8e8e outdated

  18 | @@ -19,6 +19,7 @@ class base_blob
  19 |  {
  20 |  protected:
  21 |      static constexpr int WIDTH = BITS / 8;
  22 | +    static constexpr int WORDS = BITS / 32;

promag commented at 3:09 PM on May 27, 2019:

err not WORDS.. not sure what should I call this.

DrahtBot commented at 12:20 PM on May 28, 2019: member


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #14734 (fix an undefined behavior in uint::SetHex by kazcw)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
laanwj commented at 9:56 AM on May 29, 2019: member

This is not safe, an uint8_t[] might not be 32 bit aligned! But even if you define data[] that way, aliasing is fraught with risk. Better to write safe, dumb code to not to have to consider UB.

I'd prefer not to do this. I doubt IsNull is much of a performance sink in the big picture anyway?
promag commented at 10:27 AM on May 29, 2019: member

Good point.
promag closed this on May 29, 2019
sipa commented at 3:41 PM on May 29, 2019: member

FWIW, this is UB even if the array is correctly aligned. You can't cast another pointer type to uint32_t pointer and use it, unless you know the actual object is a uint32_t or something compatible with it.
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Improve base_blob<BITS>::IsNull() #16102

Conflicts