From 6bf0b40d9b8fe9bfc439cfb856b4846620a41ff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 5 Dec 2024 16:04:37 +0100 Subject: [PATCH] QHash: fix small performance regression when detaching with resize 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 Change-Id: I0d2076a9ded8ca816c54d6ce42d472a23bcbc9fd Reviewed-by: Lars Knoll Reviewed-by: Thiago Macieira (cherry picked from commit 6c8b6acc894e47a37c4fb443316d9c40d35a144c) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit d53feb3da2064575debba91fa7d9e6d4edc0b112) --- src/corelib/tools/qhash.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h index 62f7e9be640..3a3e1842b99 100644 --- a/src/corelib/tools/qhash.h +++ b/src/corelib/tools/qhash.h @@ -558,7 +558,11 @@ struct Data 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 + Q_ALWAYS_INLINE + void reallocationHelper(const Data &other, size_t nSpans) { for (size_t s = 0; s < nSpans; ++s) { const Span &span = other.spans[s]; @@ -566,7 +570,7 @@ struct Data if (!span.hasNode(index)) continue; 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()); Node *newNode = it.insert(); new (newNode) Node(n); @@ -578,14 +582,14 @@ struct Data { auto r = allocateSpans(numBuckets); spans = r.spans; - reallocationHelper(other, r.nSpans, false); + reallocationHelper(other, r.nSpans); } Data(const Data &other, size_t reserved) : size(other.size), seed(other.seed) { numBuckets = GrowthPolicy::bucketsForCapacity(qMax(size, reserved)); spans = allocateSpans(numBuckets).spans; size_t otherNSpans = other.numBuckets >> SpanConstants::SpanShift; - reallocationHelper(other, otherNSpans, numBuckets != other.numBuckets); + reallocationHelper(other, otherNSpans); } static Data *detached(Data *d)