Revert "SERVER-42712 Improve KeyString::Value copy assignemnt"

This reverts commit cb3704ac02751e6369db7333e04ed54ab748f6da.

3 files changed, 41 insertions, 65 deletions

diff --git a/src/mongo/db/storage/key_string.cpp b/src/mongo/db/storage/key_string.cpp
index 92005ed1ab2..b45be7959b1 100644
--- a/src/mongo/db/storage/key_string.cpp
+++ b/src/mongo/db/storage/key_string.cpp
@@ -2194,6 +2194,19 @@ TypeBits& TypeBits::operator=(const TypeBits& tb) {
     return *this;
 }
 
+Value& Value::operator=(const Value& other) {
+    if (&other == this) {
+        return *this;
+    }
+
+    _version = other._version;
+    _typeBits = other._typeBits;
+    _size = other._size;
+    _buffer = other._buffer;
+
+    return *this;
+}
+
 uint32_t TypeBits::readSizeFromBuffer(BufReader* reader) {
     const uint8_t firstByte = reader->peek<uint8_t>();
 
diff --git a/src/mongo/db/storage/key_string.h b/src/mongo/db/storage/key_string.h
index bd95d9771d3..ee67a88c2b5 100644
--- a/src/mongo/db/storage/key_string.h
+++ b/src/mongo/db/storage/key_string.h
@@ -286,31 +286,20 @@ private:
 
 
 /**
- * Value owns a buffer that corresponds to a completely generated KeyString::Builder with the
- * TypeBits appended.
- *
- * To optimize copy performance and space requirements of this structure, the buffer will contain
- * the full KeyString with the TypeBits appended at the end.
+ * Value owns a buffer that corresponds to a completely generated KeyString::Builder.
  */
 class Value {
 
 public:
-    Value() : _version(Version::kLatestVersion), _ksSize(0), _bufSize(0) {}
+    Value() : _version(Version::kLatestVersion), _typeBits(Version::kLatestVersion), _size(0) {}
 
-    Value(Version version, int32_t ksSize, int32_t bufSize, ConstSharedBuffer buffer)
-        : _version(version), _ksSize(ksSize), _bufSize(bufSize), _buffer(std::move(buffer)) {}
+    Value(Version version, TypeBits typeBits, size_t size, ConstSharedBuffer buffer)
+        : _version(version), _typeBits(typeBits), _size(size), _buffer(std::move(buffer)) {}
 
     Value(const Value&) = default;
     Value(Value&&) = default;
 
-    // Use a copy-and-swap, which prevents unnecessary allocation and deallocations.
-    Value& operator=(Value copy) noexcept {
-        _version = copy._version;
-        _ksSize = copy._ksSize;
-        _bufSize = copy._bufSize;
-        std::swap(_buffer, copy._buffer);
-        return *this;
-    }
+    Value& operator=(const Value& other);
 
     template <class T>
     int compare(const T& other) const;
@@ -318,25 +307,20 @@ public:
     template <class T>
     int compareWithoutRecordId(const T& other) const;
 
-    // Returns the size of the stored KeyString.
     size_t getSize() const {
-        return _ksSize;
+        return _size;
     }
 
-    // Returns whether the size of the stored KeyString is 0.
     bool isEmpty() const {
-        return _ksSize == 0;
+        return _size == 0;
     }
 
     const char* getBuffer() const {
         return _buffer.get();
     }
 
-    // Returns the stored TypeBits.
-    TypeBits getTypeBits() const {
-        const char* buf = _buffer.get() + _ksSize;
-        BufReader reader(buf, _bufSize - _ksSize);
-        return TypeBits::fromBuffer(_version, &reader);
+    const TypeBits& getTypeBits() const {
+        return _typeBits;
     }
 
     /**
@@ -376,11 +360,8 @@ public:
 
 private:
     Version _version;
-    // _ksSize is the total length that the KeyString takes up in the buffer.
-    int32_t _ksSize;
-    // _bufSize is the total length of _buffer. If this is greater than the _ksSize, then
-    // TypeBits are appended.
-    int32_t _bufSize;
+    TypeBits _typeBits;
+    size_t _size;
     ConstSharedBuffer _buffer;
 };
 
@@ -493,15 +474,7 @@ public:
     typename std::enable_if<std::is_same<T, BufBuilder>::value, Value>::type release() {
         _doneAppending();
         _transition(BuildState::kReleased);
-
-        // Before releasing, append the TypeBits.
-        int32_t ksSize = _buffer.len();
-        if (_typeBits.isAllZeros()) {
-            _buffer.appendChar(0);
-        } else {
-            _buffer.appendBuf(_typeBits.getBuffer(), _typeBits.getSize());
-        }
-        return {version, ksSize, _buffer.len(), _buffer.release()};
+        return {version, _typeBits, static_cast<size_t>(_buffer.len()), _buffer.release()};
     }
 
     /**
@@ -510,16 +483,9 @@ public:
      */
     Value getValueCopy() {
         _doneAppending();
-
-        // Create a new buffer that is a concatenation of the KeyString and its TypeBits.
-        BufBuilder newBuf(_buffer.len() + _typeBits.getSize());
+        BufBuilder newBuf(_buffer.len());
         newBuf.appendBuf(_buffer.buf(), _buffer.len());
-        if (_typeBits.isAllZeros()) {
-            newBuf.appendChar(0);
-        } else {
-            newBuf.appendBuf(_typeBits.getBuffer(), _typeBits.getSize());
-        }
-        return {version, _buffer.len(), newBuf.len(), newBuf.release()};
+        return {version, _typeBits, static_cast<size_t>(newBuf.len()), newBuf.release()};
     }
 
     void appendRecordId(RecordId loc);
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
index effc349a1cf..b44af768b5c 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp
@@ -622,10 +622,10 @@ public:
         WiredTigerItem item(keyString.getBuffer(), keyString.getSize());
         setKey(_cursor, item.Get());
 
-        const KeyString::TypeBits typeBits = keyString.getTypeBits();
-        WiredTigerItem valueItem = typeBits.isAllZeros()
+        WiredTigerItem valueItem = keyString.getTypeBits().isAllZeros()
             ? emptyItem
-            : WiredTigerItem(typeBits.getBuffer(), typeBits.getSize());
+            : WiredTigerItem(keyString.getTypeBits().getBuffer(),
+                             keyString.getTypeBits().getSize());
 
         _cursor->set_value(_cursor, valueItem.Get());
 
@@ -712,10 +712,10 @@ private:
         WiredTigerItem keyItem(newKeyString.getBuffer(), newKeyString.getSize());
         setKey(_cursor, keyItem.Get());
 
-        const KeyString::TypeBits typeBits = newKeyString.getTypeBits();
-        WiredTigerItem valueItem = typeBits.isAllZeros()
+        WiredTigerItem valueItem = newKeyString.getTypeBits().isAllZeros()
             ? emptyItem
-            : WiredTigerItem(typeBits.getBuffer(), typeBits.getSize());
+            : WiredTigerItem(newKeyString.getTypeBits().getBuffer(),
+                             newKeyString.getTypeBits().getSize());
 
         _cursor->set_value(_cursor, valueItem.Get());
 
@@ -1448,9 +1448,8 @@ Status WiredTigerIndexUnique::_insertTimestampUnsafe(OperationContext* opCtx,
     WiredTigerItem keyItem(keyString.getBuffer(), sizeWithoutRecordId);
 
     KeyString::Builder value(getKeyStringVersion(), id);
-    const KeyString::TypeBits typeBits = keyString.getTypeBits();
-    if (!typeBits.isAllZeros())
-        value.appendTypeBits(typeBits);
+    if (!keyString.getTypeBits().isAllZeros())
+        value.appendTypeBits(keyString.getTypeBits());
 
     WiredTigerItem valueItem(value.getBuffer(), value.getSize());
     setKey(c, keyItem.Get());
@@ -1486,7 +1485,7 @@ Status WiredTigerIndexUnique::_insertTimestampUnsafe(OperationContext* opCtx,
 
         if (!insertedId && id < idInIndex) {
             value.appendRecordId(id);
-            value.appendTypeBits(typeBits);
+            value.appendTypeBits(keyString.getTypeBits());
             insertedId = true;
         }
 
@@ -1503,7 +1502,7 @@ Status WiredTigerIndexUnique::_insertTimestampUnsafe(OperationContext* opCtx,
     if (!insertedId) {
         // This id is higher than all currently in the index for this key
         value.appendRecordId(id);
-        value.appendTypeBits(typeBits);
+        value.appendTypeBits(keyString.getTypeBits());
     }
 
     valueItem = WiredTigerItem(value.getBuffer(), value.getSize());
@@ -1564,10 +1563,9 @@ Status WiredTigerIndexUnique::_insertTimestampSafe(OperationContext* opCtx,
     // Now create the table key/value, the actual data record.
     WiredTigerItem keyItem(keyString.getBuffer(), keyString.getSize());
 
-    const KeyString::TypeBits typeBits = keyString.getTypeBits();
-    WiredTigerItem valueItem = typeBits.isAllZeros()
+    WiredTigerItem valueItem = keyString.getTypeBits().isAllZeros()
         ? emptyItem
-        : WiredTigerItem(typeBits.getBuffer(), typeBits.getSize());
+        : WiredTigerItem(keyString.getTypeBits().getBuffer(), keyString.getTypeBits().getSize());
     setKey(c, keyItem.Get());
     c->set_value(c, valueItem.Get());
     ret = WT_OP_CHECK(c->insert(c));
@@ -1766,10 +1764,9 @@ Status WiredTigerIndexStandard::_insert(OperationContext* opCtx,
 
     WiredTigerItem keyItem(keyString.getBuffer(), keyString.getSize());
 
-    const KeyString::TypeBits typeBits = keyString.getTypeBits();
-    WiredTigerItem valueItem = typeBits.isAllZeros()
+    WiredTigerItem valueItem = keyString.getTypeBits().isAllZeros()
         ? emptyItem
-        : WiredTigerItem(typeBits.getBuffer(), typeBits.getSize());
+        : WiredTigerItem(keyString.getTypeBits().getBuffer(), keyString.getTypeBits().getSize());
 
     setKey(c, keyItem.Get());
     c->set_value(c, valueItem.Get());