From b9268bef7135810281e4926e25ce56fda9096741 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 8 Nov 2023 12:26:44 -0800 Subject: [PATCH] QBitArray: add mutation operators taking rvalue QBitArrays Because we may be able to use the other side's storage and apply the operation in-place. We reuse the storage of the one that can be detached: even if we have to grow the buffer, QBitArrays are usually small so there's a good chance it's just to the extra space QArrayData usually overallocates or a simple, non-moving realloc(). Disassembly with GCC 13 and Clang 17 show the vectorisers did kick in for all four functions (inverted_inplace included). I think a hand-rolled version could squeeze a few more cycles, especially in the tail section, but I don't consider its maintenance cost to be worth it. Change-Id: I85b3fc2dd45c4693be13fffd1795bfb1b296caa6 Reviewed-by: Marc Mutz (cherry picked from commit 948fa2a427dbcdc2784abdcf87efc914417a0028) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/tools/qbitarray.cpp | 109 +++++++++++++---- src/corelib/tools/qbitarray.h | 3 + .../corelib/tools/qbitarray/tst_qbitarray.cpp | 110 +++++++++++++++++- 3 files changed, 192 insertions(+), 30 deletions(-) diff --git a/src/corelib/tools/qbitarray.cpp b/src/corelib/tools/qbitarray.cpp index a1f091a06fa..fed2bb6bd2a 100644 --- a/src/corelib/tools/qbitarray.cpp +++ b/src/corelib/tools/qbitarray.cpp @@ -562,7 +562,68 @@ QBitArray &performBitwiseOperationHelper(QBitArray &out, const QBitArray &a1, return out; } +template static Q_NEVER_INLINE +QBitArray &performBitwiseOperationInCopy(QBitArray &self, const QBitArray &other, BitwiseOp op) +{ + QBitArray tmp(std::move(self)); + self = sizedForOverwrite(tmp, other); + return performBitwiseOperationHelper(self, tmp, other, op); +} + +template static Q_NEVER_INLINE +QBitArray &performBitwiseOperationInPlace(QBitArray &self, const QBitArray &other, BitwiseOp op) +{ + if (self.size() < other.size()) + self.resize(other.size()); + return performBitwiseOperationHelper(self, self, other, op); +} + +template static +QBitArray &performBitwiseOperation(QBitArray &self, const QBitArray &other, BitwiseOp op) +{ + if (self.data_ptr().needsDetach()) + return performBitwiseOperationInCopy(self, other, op); + return performBitwiseOperationInPlace(self, other, op); +} + +// SCARY helper +enum { InCopy, InPlace }; +static auto prepareForBitwiseOperation(QBitArray &self, QBitArray &other) +{ + QByteArrayData &d1 = self.data_ptr(); + QByteArrayData &d2 = other.data_ptr(); + bool detached1 = !d1.needsDetach(); + bool detached2 = !d2.needsDetach(); + if (!detached1 && !detached2) + return InCopy; + + // at least one of the two is detached, we'll reuse its buffer + bool swap = false; + if (detached1 && detached2) { + // both are detached, so choose the larger of the two + swap = d1.allocatedCapacity() < d2.allocatedCapacity(); + } else if (detached2) { + // we can re-use other's buffer but not self's, so swap the two + swap = true; + } + if (swap) + self.swap(other); + return InPlace; +} + +template static +QBitArray &performBitwiseOperation(QBitArray &self, QBitArray &other, BitwiseOp op) +{ + auto choice = prepareForBitwiseOperation(self, other); + if (choice == InCopy) + return performBitwiseOperationInCopy(self, other, std::move(op)); + return performBitwiseOperationInPlace(self, other, std::move(op)); +} + /*! + \fn QBitArray &QBitArray::operator&=(const QBitArray &other) + \fn QBitArray &QBitArray::operator&=(QBitArray &&other) + Performs the AND operation between all bits in this bit array and \a other. Assigns the result to this bit array, and returns a reference to it. @@ -577,21 +638,20 @@ QBitArray &performBitwiseOperationHelper(QBitArray &out, const QBitArray &a1, \sa operator&(), operator|=(), operator^=(), operator~() */ +QBitArray &QBitArray::operator&=(QBitArray &&other) +{ + return performBitwiseOperation(*this, other, std::bit_and()); +} + QBitArray &QBitArray::operator&=(const QBitArray &other) { - resize(qMax(size(), other.size())); - uchar *a1 = reinterpret_cast(d.data()) + 1; - const uchar *a2 = reinterpret_cast(other.d.constData()) + 1; - qsizetype n = other.d.size() - 1; - qsizetype p = d.size() - 1 - n; - while (n-- > 0) - *a1++ &= *a2++; - while (p-- > 0) - *a1++ = 0; - return *this; + return performBitwiseOperation(*this, other, std::bit_and()); } /*! + \fn QBitArray &QBitArray::operator|=(const QBitArray &other) + \fn QBitArray &QBitArray::operator|=(QBitArray &&other) + Performs the OR operation between all bits in this bit array and \a other. Assigns the result to this bit array, and returns a reference to it. @@ -606,18 +666,20 @@ QBitArray &QBitArray::operator&=(const QBitArray &other) \sa operator|(), operator&=(), operator^=(), operator~() */ +QBitArray &QBitArray::operator|=(QBitArray &&other) +{ + return performBitwiseOperation(*this, other, std::bit_or()); +} + QBitArray &QBitArray::operator|=(const QBitArray &other) { - resize(qMax(size(), other.size())); - uchar *a1 = reinterpret_cast(d.data()) + 1; - const uchar *a2 = reinterpret_cast(other.d.constData()) + 1; - qsizetype n = other.d.size() - 1; - while (n-- > 0) - *a1++ |= *a2++; - return *this; + return performBitwiseOperation(*this, other, std::bit_or()); } /*! + \fn QBitArray &QBitArray::operator^=(const QBitArray &other) + \fn QBitArray &QBitArray::operator^=(QBitArray &&other) + Performs the XOR operation between all bits in this bit array and \a other. Assigns the result to this bit array, and returns a reference to it. @@ -632,15 +694,14 @@ QBitArray &QBitArray::operator|=(const QBitArray &other) \sa operator^(), operator&=(), operator|=(), operator~() */ +QBitArray &QBitArray::operator^=(QBitArray &&other) +{ + return performBitwiseOperation(*this, other, std::bit_xor()); +} + QBitArray &QBitArray::operator^=(const QBitArray &other) { - resize(qMax(size(), other.size())); - uchar *a1 = reinterpret_cast(d.data()) + 1; - const uchar *a2 = reinterpret_cast(other.d.constData()) + 1; - qsizetype n = other.d.size() - 1; - while (n-- > 0) - *a1++ ^= *a2++; - return *this; + return performBitwiseOperation(*this, other, std::bit_xor()); } /*! diff --git a/src/corelib/tools/qbitarray.h b/src/corelib/tools/qbitarray.h index 183be5fb5b2..ec2b3fd2d6c 100644 --- a/src/corelib/tools/qbitarray.h +++ b/src/corelib/tools/qbitarray.h @@ -103,6 +103,9 @@ public: QBitRef operator[](qsizetype i); bool operator[](qsizetype i) const { return testBit(i); } + QBitArray &operator&=(QBitArray &&); + QBitArray &operator|=(QBitArray &&); + QBitArray &operator^=(QBitArray &&); QBitArray &operator&=(const QBitArray &); QBitArray &operator|=(const QBitArray &); QBitArray &operator^=(const QBitArray &); diff --git a/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp b/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp index be98f56164a..8fd209c7e26 100644 --- a/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp +++ b/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp @@ -316,9 +316,34 @@ void tst_QBitArray::operator_andeq() QFETCH(QBitArray, input2); QFETCH(QBitArray, res); - input1&=input2; + QBitArray result = input1; + result &= input2; + QCOMPARE(result, res); + result = input1; + result &= std::move(input2); + QCOMPARE(result, res); + result = input1; + result &= detached(input2); + QCOMPARE(result, res); - QCOMPARE(input1, res); + // operation is commutative + result = input2; + result &= input1; + QCOMPARE(result, res); + result = input2; + result &= std::move(input1); + QCOMPARE(result, res); + result = input2; + result &= detached(input1); + QCOMPARE(result, res); + + // operation is idempotent + result &= result; + QCOMPARE(result, res); + result &= std::move(result); + QCOMPARE(result, res); + result &= detached(result); + QCOMPARE(result, res); } void tst_QBitArray::operator_and() @@ -397,9 +422,34 @@ void tst_QBitArray::operator_oreq() QFETCH(QBitArray, input2); QFETCH(QBitArray, res); - input1|=input2; + QBitArray result = input1; + result |= input2; + QCOMPARE(result, res); + result = input1; + result |= QBitArray(input2); + QCOMPARE(result, res); + result = input1; + result |= detached(input2); + QCOMPARE(result, res); - QCOMPARE(input1, res); + // operation is commutative + result = input2; + result |= input1; + QCOMPARE(result, res); + result = input2; + result |= QBitArray(input1); + QCOMPARE(result, res); + result = input2; + result |= detached(input1); + QCOMPARE(result, res); + + // operation is idempotent + result |= result; + QCOMPARE(result, res); + result |= QBitArray(result); + QCOMPARE(result, res); + result |= detached(result); + QCOMPARE(result, res); } void tst_QBitArray::operator_or() @@ -476,9 +526,57 @@ void tst_QBitArray::operator_xoreq() QFETCH(QBitArray, input2); QFETCH(QBitArray, res); - input1^=input2; + QBitArray result = input1; + result ^= input2; + QCOMPARE(result, res); + result = input1; + result ^= QBitArray(input2); + QCOMPARE(result, res); + result = input1; + result ^= detached(input2); + QCOMPARE(result, res); - QCOMPARE(input1, res); + // operation is commutative + result = input2; + result ^= input1; + QCOMPARE(result, res); + result = input2; + result ^= QBitArray(input1); + QCOMPARE(result, res); + result = input2; + result ^= detached(input1); + QCOMPARE(result, res); + + // XORing with oneself is nilpotent + result = input1; + result ^= input1; + QCOMPARE(result, QBitArray(input1.size())); + result = input1; + result ^= QBitArray(result); + QCOMPARE(result, QBitArray(input1.size())); + result = input1; + result ^= detached(result); + QCOMPARE(result, QBitArray(input1.size())); + + result = input2; + result ^= input2; + QCOMPARE(result, QBitArray(input2.size())); + result = input2; + result ^= QBitArray(input2); + QCOMPARE(result, QBitArray(input2.size())); + result = input2; + result ^= detached(input2); + QCOMPARE(result, QBitArray(input2.size())); + + result = res; + result ^= res; + QCOMPARE(result, QBitArray(res.size())); + result = res; + result ^= QBitArray(res); + QCOMPARE(result, QBitArray(res.size())); + result = res; + result ^= detached(res); + QCOMPARE(result, QBitArray(res.size())); } void tst_QBitArray::operator_xor()