1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QHash: fix small performance regression when detaching with resize

Browse Source 
There are two QHashPrivate::Data constructors (and two overloads of
::detached()). Initially the Data ctor that takes a new size always
assumed a resize was happening, and any place where there _may_ be a
resize we would usually detach then rehash.

In an effort to avoid the detach+rehash and instead call detach(d, size)
without the performance overhead of rehashing the call to
reallocationHelper() in this overload of the ctor was changed to verify
that a rehash was actually needed. This had the unfortunate side-effect
of making the compiler no longer inline the reallocationHelper()
function, and it no longer expanded the if-expression with the constant
so it was doing a tight copy-loop with a potential branch in the middle.

In this patch we revert that and make the bool a template argument to
highlight that it should be a constant for better performance (but also
leaving a comment.) Also mark it Q_ALWAYS_INLINE, it has two uses and
they both take advantage of the inlining + expanding the expression.

In theory this might have had an impact on QHash::reserve() calls,
though this code is only relevant when reserve() would cause growth so
the performance regression would hopefully be small compared to all the
other work that would also be needed.

Reverts 45c137d797a85c694897e8b1c5099abacc16e2f5

Pick-to: 6.9 6.8
Change-Id: I0d2076a9ded8ca816c54d6ce42d472a23bcbc9fd
Reviewed-by: Lars Knoll <lars@knoll.priv.no>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Mårten Nordheim 2024-12-05 16:04:37 +01:00
parent ac2b0b958e
commit 6c8b6acc89
1 changed files with 8 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/corelib/tools/qhash.h
View File

@ -559,7 +559,11 @@ struct Data
seed = QHashSeed::globalSeed(); seed = QHashSeed::globalSeed();
} }
void reallocationHelper(const Data &other, size_t nSpans, bool resized) // The Resized parameter is a template param to make sure the compiler will get rid of the
// branch, for performance.
template <bool Resized>
Q_ALWAYS_INLINE
void reallocationHelper(const Data &other, size_t nSpans)
{ {
for (size_t s = 0; s < nSpans; ++s) { for (size_t s = 0; s < nSpans; ++s) {
const Span &span = other.spans[s]; const Span &span = other.spans[s];
@ -567,7 +571,7 @@ struct Data
if (!span.hasNode(index)) if (!span.hasNode(index))
continue; continue;
const Node &n = span.at(index); const Node &n = span.at(index);
auto it = resized ? findBucket(n.key) : Bucket { spans + s, index }; auto it = Resized ? findBucket(n.key) : Bucket { spans + s, index };
Q_ASSERT(it.isUnused()); Q_ASSERT(it.isUnused());
Node *newNode = it.insert(); Node *newNode = it.insert();
new (newNode) Node(n); new (newNode) Node(n);
@ -579,14 +583,14 @@ struct Data
{ {
auto r = allocateSpans(numBuckets); auto r = allocateSpans(numBuckets);
spans = r.spans; spans = r.spans;
reallocationHelper(other, r.nSpans, false); reallocationHelper<false>(other, r.nSpans);
} }
Data(const Data &other, size_t reserved) : size(other.size), seed(other.seed) Data(const Data &other, size_t reserved) : size(other.size), seed(other.seed)
{ {
numBuckets = GrowthPolicy::bucketsForCapacity(qMax(size, reserved)); numBuckets = GrowthPolicy::bucketsForCapacity(qMax(size, reserved));
spans = allocateSpans(numBuckets).spans; spans = allocateSpans(numBuckets).spans;
size_t otherNSpans = other.numBuckets >> SpanConstants::SpanShift; size_t otherNSpans = other.numBuckets >> SpanConstants::SpanShift;
reallocationHelper(other, otherNSpans, numBuckets != other.numBuckets); reallocationHelper<true>(other, otherNSpans);
} }
static Data *detached(Data *d) static Data *detached(Data *d)