Merge bitcoin/bitcoin#23115: bloom: use Span instead of std::vector for insert and contains

a11da75411 bloom: cleanup includes (fanquake)
f1ed1d3194 bloom: use constexpr where appropriate (fanquake)
2ba4ddf31d bloom: use Span instead of std::vector for `insert` and `contains` (William Casarin)

Pull request description:

  This is #18985 rebased, with the most recent comments addressed.

  > We can avoid many unnecessary std::vector allocations by changing
  CBloomFilter to take Spans instead of std::vector's for the `insert`
  and `contains` operations.

  > CBloomFilter currently converts types such as CDataStream and uint256
  to std::vector on `insert` and `contains`. This is unnecessary because
  CDataStreams and uint256 are already std::vectors internally. We just
  need a way to point to the right data within those types. Span gives
  us this ability.

ACKs for top commit:
  sipa:
    Code review ACK a11da75411
  laanwj:
    Code review ACK a11da75411

Tree-SHA512: ee9ba02c9588daa1ff51782d1953fd060839dd15aa85861b2633b6ff2398320188ddd00f01d0c99442224485364ede9f8322366de4239fc7831ebfa06bd34659
This commit is contained in:
MarcoFalke 2021-09-29 15:19:01 +02:00
commit 829c441af2
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
4 changed files with 28 additions and 54 deletions

View file

@ -4,20 +4,22 @@
#include <bloom.h>
#include <primitives/transaction.h>
#include <hash.h>
#include <primitives/transaction.h>
#include <random.h>
#include <script/script.h>
#include <script/standard.h>
#include <random.h>
#include <span.h>
#include <streams.h>
#include <math.h>
#include <stdlib.h>
#include <algorithm>
#include <cmath>
#include <cstdlib>
#include <limits>
#include <vector>
#define LN2SQUARED 0.4804530139182014246671025263266649717305529515945455
#define LN2 0.6931471805599453094172321214581765680755001343602552
static constexpr double LN2SQUARED = 0.4804530139182014246671025263266649717305529515945455;
static constexpr double LN2 = 0.6931471805599453094172321214581765680755001343602552;
CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweakIn, unsigned char nFlagsIn) :
/**
@ -37,13 +39,13 @@ CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, c
{
}
inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, const std::vector<unsigned char>& vDataToHash) const
inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, Span<const unsigned char> vDataToHash) const
{
// 0xFBA4C795 chosen as it guarantees a reasonable bit difference between nHashNum values.
return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash) % (vData.size() * 8);
}
void CBloomFilter::insert(const std::vector<unsigned char>& vKey)
void CBloomFilter::insert(Span<const unsigned char> vKey)
{
if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700)
return;
@ -59,17 +61,10 @@ void CBloomFilter::insert(const COutPoint& outpoint)
{
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
stream << outpoint;
std::vector<unsigned char> data(stream.begin(), stream.end());
insert(data);
insert(stream);
}
void CBloomFilter::insert(const uint256& hash)
{
std::vector<unsigned char> data(hash.begin(), hash.end());
insert(data);
}
bool CBloomFilter::contains(const std::vector<unsigned char>& vKey) const
bool CBloomFilter::contains(Span<const unsigned char> vKey) const
{
if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700)
return true;
@ -87,14 +82,7 @@ bool CBloomFilter::contains(const COutPoint& outpoint) const
{
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
stream << outpoint;
std::vector<unsigned char> data(stream.begin(), stream.end());
return contains(data);
}
bool CBloomFilter::contains(const uint256& hash) const
{
std::vector<unsigned char> data(hash.begin(), hash.end());
return contains(data);
return contains(MakeUCharSpan(stream));
}
bool CBloomFilter::IsWithinSizeConstraints() const
@ -198,7 +186,8 @@ CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const dou
}
/* Similar to CBloomFilter::Hash */
static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, const std::vector<unsigned char>& vDataToHash) {
static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, Span<const unsigned char> vDataToHash)
{
return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash);
}
@ -210,7 +199,7 @@ static inline uint32_t FastMod(uint32_t x, size_t n) {
return ((uint64_t)x * (uint64_t)n) >> 32;
}
void CRollingBloomFilter::insert(const std::vector<unsigned char>& vKey)
void CRollingBloomFilter::insert(Span<const unsigned char> vKey)
{
if (nEntriesThisGeneration == nEntriesPerGeneration) {
nEntriesThisGeneration = 0;
@ -241,13 +230,7 @@ void CRollingBloomFilter::insert(const std::vector<unsigned char>& vKey)
}
}
void CRollingBloomFilter::insert(const uint256& hash)
{
std::vector<unsigned char> vData(hash.begin(), hash.end());
insert(vData);
}
bool CRollingBloomFilter::contains(const std::vector<unsigned char>& vKey) const
bool CRollingBloomFilter::contains(Span<const unsigned char> vKey) const
{
for (int n = 0; n < nHashFuncs; n++) {
uint32_t h = RollingBloomHash(n, nTweak, vKey);
@ -261,12 +244,6 @@ bool CRollingBloomFilter::contains(const std::vector<unsigned char>& vKey) const
return true;
}
bool CRollingBloomFilter::contains(const uint256& hash) const
{
std::vector<unsigned char> vData(hash.begin(), hash.end());
return contains(vData);
}
void CRollingBloomFilter::reset()
{
nTweak = GetRand(std::numeric_limits<unsigned int>::max());

View file

@ -6,16 +6,16 @@
#define BITCOIN_BLOOM_H
#include <serialize.h>
#include <span.h>
#include <vector>
class COutPoint;
class CTransaction;
class uint256;
//! 20,000 items with fp rate < 0.1% or 10,000 items and <0.0001%
static const unsigned int MAX_BLOOM_FILTER_SIZE = 36000; // bytes
static const unsigned int MAX_HASH_FUNCS = 50;
static constexpr unsigned int MAX_BLOOM_FILTER_SIZE = 36000; // bytes
static constexpr unsigned int MAX_HASH_FUNCS = 50;
/**
* First two bits of nFlags control how much IsRelevantAndUpdate actually updates
@ -49,7 +49,7 @@ private:
unsigned int nTweak;
unsigned char nFlags;
unsigned int Hash(unsigned int nHashNum, const std::vector<unsigned char>& vDataToHash) const;
unsigned int Hash(unsigned int nHashNum, Span<const unsigned char> vDataToHash) const;
public:
/**
@ -66,13 +66,11 @@ public:
SERIALIZE_METHODS(CBloomFilter, obj) { READWRITE(obj.vData, obj.nHashFuncs, obj.nTweak, obj.nFlags); }
void insert(const std::vector<unsigned char>& vKey);
void insert(Span<const unsigned char> vKey);
void insert(const COutPoint& outpoint);
void insert(const uint256& hash);
bool contains(const std::vector<unsigned char>& vKey) const;
bool contains(Span<const unsigned char> vKey) const;
bool contains(const COutPoint& outpoint) const;
bool contains(const uint256& hash) const;
//! True if the size is <= MAX_BLOOM_FILTER_SIZE and the number of hash functions is <= MAX_HASH_FUNCS
//! (catch a filter which was just deserialized which was too big)
@ -112,10 +110,8 @@ class CRollingBloomFilter
public:
CRollingBloomFilter(const unsigned int nElements, const double nFPRate);
void insert(const std::vector<unsigned char>& vKey);
void insert(const uint256& hash);
bool contains(const std::vector<unsigned char>& vKey) const;
bool contains(const uint256& hash) const;
void insert(Span<const unsigned char> vKey);
bool contains(Span<const unsigned char> vKey) const;
void reset();

View file

@ -3,6 +3,7 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <hash.h>
#include <span.h>
#include <crypto/common.h>
#include <crypto/hmac_sha512.h>

View file

@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_key)
CBloomFilter filter(2, 0.001, 0, BLOOM_UPDATE_ALL);
filter.insert(vchPubKey);
uint160 hash = pubkey.GetID();
filter.insert(std::vector<unsigned char>(hash.begin(), hash.end()));
filter.insert(hash);
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
stream << filter;