Add indirection for the unlikely stuff to keep the likely members close and tight.

3 files changed, 88 insertions, 78 deletions

diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp
index a4d9c778bec..b24b1d97788 100644
--- a/document/src/tests/documenttestcase.cpp
+++ b/document/src/tests/documenttestcase.cpp
@@ -36,10 +36,10 @@ TEST(DocumentTest, testSizeOf)
     EXPECT_EQ(32u, sizeof(vespalib::GrowableByteBuffer));
     EXPECT_EQ(88ul, sizeof(IdString));
     EXPECT_EQ(104ul, sizeof(DocumentId));
-    EXPECT_EQ(240ul, sizeof(Document));
-    EXPECT_EQ(104ul, sizeof(StructFieldValue));
+    EXPECT_EQ(224ul, sizeof(Document));
+    EXPECT_EQ(88ul, sizeof(StructFieldValue));
     EXPECT_EQ(24ul, sizeof(StructuredFieldValue));
-    EXPECT_EQ(56ul, sizeof(SerializableArray));
+    EXPECT_EQ(40ul, sizeof(SerializableArray));
 }
 
 TEST(DocumentTest, testFieldPath)
diff --git a/document/src/vespa/document/fieldvalue/serializablearray.cpp b/document/src/vespa/document/fieldvalue/serializablearray.cpp
index 3851d8e2354..963fbc4dbe9 100644
--- a/document/src/vespa/document/fieldvalue/serializablearray.cpp
+++ b/document/src/vespa/document/fieldvalue/serializablearray.cpp
@@ -26,24 +26,20 @@ public:
 
 }
 
-SerializableArray::SerializableArray()
-    : _serializedCompression(CompressionConfig::NONE),
-      _uncompressedLength(0)
-{
-}
+SerializableArray::SerializableArray() = default;
 
 SerializableArray::SerializableArray(EntryMap entries, ByteBuffer::UP buffer,
                                      CompressionConfig::Type comp_type, uint32_t uncompressed_length)
     : _entries(std::move(entries)),
-      _owned(),
-      _serializedCompression(comp_type)
+      _unlikely()
 {
 
-    if (CompressionConfig::isCompressed(_serializedCompression)) {
-        _compSerData = std::move(buffer);
-        _uncompressedLength = uncompressed_length;
+    if (CompressionConfig::isCompressed(comp_type)) {
+        _unlikely = std::make_unique<Unlikely>();
+        _unlikely->_compSerData = std::move(buffer);
+        _unlikely->_serializedCompression = comp_type;
+        _unlikely->_uncompressedLength = uncompressed_length;
     } else {
-        _uncompressedLength = buffer->getRemaining();
         _uncompSerData = std::move(buffer);
     }
 }
@@ -54,23 +50,36 @@ SerializableArray::~SerializableArray() = default;
 
 namespace {
 
-serializablearray::BufferMap &
-ensure(std::unique_ptr<serializablearray::BufferMap> &owned) {
+template <typename T>
+T &
+ensure(std::unique_ptr<T> &owned) {
     if (!owned) {
-        owned = std::make_unique<serializablearray::BufferMap>();
+        owned = std::make_unique<T>();
     }
     return *owned;
 }
 
 }
 
-SerializableArray::SerializableArray(const SerializableArray& other)
-    : _entries(other._entries),
-      _owned(),
-      _uncompSerData(other._uncompSerData.get() ? new ByteBuffer(*other._uncompSerData) : nullptr),
-      _compSerData(other._compSerData.get() ? new ByteBuffer(*other._compSerData) : nullptr),
-      _serializedCompression(other._serializedCompression),
-      _uncompressedLength(other._uncompressedLength)
+SerializableArray::Unlikely::Unlikely()
+    : _owned(),
+      _compSerData(),
+      _serializedCompression(CompressionConfig::NONE),
+      _uncompressedLength(0)
+{ }
+SerializableArray::Unlikely::~Unlikely() = default;
+
+SerializableArray::Unlikely::Unlikely(const Unlikely & rhs)
+    : _owned(),
+      _compSerData(rhs._compSerData ? new ByteBuffer(*rhs._compSerData) : nullptr),
+      _serializedCompression(rhs._serializedCompression),
+      _uncompressedLength(rhs._uncompressedLength)
+{ }
+
+SerializableArray::SerializableArray(const SerializableArray& rhs)
+    : _entries(rhs._entries),
+      _unlikely(rhs._unlikely ? new Unlikely(*rhs._unlikely) : nullptr),
+      _uncompSerData(rhs._uncompSerData ? new ByteBuffer(*rhs._uncompSerData) : nullptr)
 {
     for (size_t i(0); i < _entries.size(); i++) {
         Entry & e(_entries[i]);
@@ -78,14 +87,11 @@ SerializableArray::SerializableArray(const SerializableArray& other)
             // Pointing to a buffer in the _owned structure.
             ByteBuffer::UP buf(ByteBuffer::copyBuffer(e.getBuffer(_uncompSerData.get()), e.size()));
             e.setBuffer(buf->getBuffer());
-            ensure(_owned)[e.id()] = std::move(buf);
+            ensure(_unlikely->_owned)[e.id()] = std::move(buf);
         } else {
             // If not it is relative to the buffer _uncompSerData, and hence it is valid as is.
         }
     }
-    if (_uncompSerData.get()) {
-        LOG_ASSERT(_uncompressedLength == _uncompSerData->getRemaining());
-    }
 }
 
 SerializableArray &
@@ -99,15 +105,15 @@ void SerializableArray::clear()
 {
     _entries.clear();
     _uncompSerData.reset();
-    _compSerData.reset();
-    _serializedCompression = CompressionConfig::NONE;
-    _uncompressedLength = 0;
+    _unlikely.reset();
 }
 
 void
 SerializableArray::invalidate()
 {
-    _compSerData.reset();
+    if (_unlikely) {
+        _unlikely->_compSerData.reset();
+    }
 }
 
 void
@@ -115,7 +121,7 @@ SerializableArray::set(int id, ByteBuffer::UP buffer)
 {
     maybeDecompress();
     Entry e(id, buffer->getRemaining(), buffer->getBuffer());
-    ensure(_owned)[id] = std::move(buffer);
+    ensure(ensure(_unlikely)._owned)[id] = std::move(buffer);
     auto it = find(id);
     if (it == _entries.end()) {
         _entries.push_back(e);
@@ -185,8 +191,8 @@ SerializableArray::clear(int id)
     auto it  = find(id);
     if (it != _entries.end()) {
         _entries.erase(it);
-        if (_owned) {
-            _owned->erase(id);
+        if (_unlikely && _unlikely->_owned) {
+            _unlikely->_owned->erase(id);
         }
         invalidate();
     }
@@ -198,45 +204,42 @@ SerializableArray::deCompress() // throw (DeserializeException)
     using vespalib::compression::decompress;
     // will only do this once
 
-    LOG_ASSERT(_compSerData);
-    LOG_ASSERT(!_uncompSerData);
+    assert(_unlikely && _unlikely->_compSerData);
+    assert(!_uncompSerData);
+    assert(CompressionConfig::isCompressed(_unlikely->_serializedCompression));
+    uint32_t uncompressedLength = _unlikely->_uncompressedLength;
 
-    if (_serializedCompression == CompressionConfig::NONE ||
-        _serializedCompression == CompressionConfig::UNCOMPRESSABLE)
-    {
-        _uncompSerData = std::move(_compSerData);
-        LOG_ASSERT(_uncompressedLength == _uncompSerData->getRemaining());
-    } else {
-        auto newSerialization = std::make_unique<ByteBuffer>(vespalib::alloc::Alloc::alloc(_uncompressedLength), _uncompressedLength);
-        vespalib::DataBuffer unCompressed(newSerialization->getBuffer(), newSerialization->getLength());
-        unCompressed.clear();
-        try {
-            decompress(_serializedCompression,
-                       _uncompressedLength,
-                       vespalib::ConstBufferRef(_compSerData->getBufferAtPos(), _compSerData->getRemaining()),
-                       unCompressed,
-                       false);
-        } catch (const std::runtime_error & e) {
-            throw DeserializeException(
-                make_string( "Document was compressed with code unknown code %d", _serializedCompression),
-                VESPA_STRLOC);
-        }
+    auto newSerialization = std::make_unique<ByteBuffer>(vespalib::alloc::Alloc::alloc(uncompressedLength), uncompressedLength);
+    vespalib::DataBuffer unCompressed(newSerialization->getBuffer(), newSerialization->getLength());
+    unCompressed.clear();
+    try {
+        decompress(_unlikely->_serializedCompression,
+                   uncompressedLength,
+                   vespalib::ConstBufferRef(_unlikely->_compSerData->getBufferAtPos(), _unlikely->_compSerData->getRemaining()),
+                   unCompressed,
+                   false);
+    } catch (const std::runtime_error & e) {
+        throw DeserializeException(
+            make_string( "Document was compressed with code unknown code %d", _unlikely->_serializedCompression),
+            VESPA_STRLOC);
+    }
 
-        if (unCompressed.getDataLen() != (size_t)_uncompressedLength) {
-            throw DeserializeException(
-                    make_string("Did not decompress to the expected length: had %u, wanted %d, got %zu",
-                                _compSerData->getRemaining(), _uncompressedLength, unCompressed.getDataLen()),
-                    VESPA_STRLOC);
-        }
-        assert(newSerialization->getBuffer() == unCompressed.getData());
-        _uncompSerData = std::move(newSerialization);
-        LOG_ASSERT(_uncompressedLength == _uncompSerData->getRemaining());
+    if (unCompressed.getDataLen() != (size_t)uncompressedLength) {
+        throw DeserializeException(
+                make_string("Did not decompress to the expected length: had %u, wanted %d, got %zu",
+                            _unlikely->_compSerData->getRemaining(), uncompressedLength, unCompressed.getDataLen()),
+                VESPA_STRLOC);
     }
+    assert(newSerialization->getBuffer() == unCompressed.getData());
+    _uncompSerData = std::move(newSerialization);
+    LOG_ASSERT(uncompressedLength == _uncompSerData->getRemaining());
 }
 
 vespalib::compression::CompressionInfo
 SerializableArray::getCompressionInfo() const {
-    return CompressionInfo(_uncompressedLength, _compSerData->getRemaining());
+    return _unlikely
+           ? CompressionInfo(_unlikely->_uncompressedLength, _unlikely->_compSerData->getRemaining())
+           : CompressionInfo(_uncompSerData->getRemaining(), CompressionConfig::NONE);
 }
 
 const char *
diff --git a/document/src/vespa/document/fieldvalue/serializablearray.h b/document/src/vespa/document/fieldvalue/serializablearray.h
index 11f228b3224..d4e9a4f3f5e 100644
--- a/document/src/vespa/document/fieldvalue/serializablearray.h
+++ b/document/src/vespa/document/fieldvalue/serializablearray.h
@@ -128,21 +128,23 @@ public:
     /** Deletes all stored attributes. */
     void clear();
 
-    CompressionConfig::Type getCompression() const { return _serializedCompression; }
+    CompressionConfig::Type getCompression() const {
+        return _unlikely ? _unlikely->_serializedCompression : CompressionConfig::NONE;
+    }
     CompressionInfo getCompressionInfo() const;
 
     bool empty() const { return _entries.empty(); }
 
     const ByteBuffer* getSerializedBuffer() const {
-        return CompressionConfig::isCompressed(_serializedCompression)
-            ? _compSerData.get()
+        return CompressionConfig::isCompressed(getCompression())
+            ? _unlikely->_compSerData.get()
             : _uncompSerData.get();
     }
 
     const EntryMap & getEntries() const { return _entries; }
 private:
     bool shouldDecompress() const {
-        return _compSerData.get() && !_uncompSerData.get();
+        return _unlikely && _unlikely->_compSerData && !_uncompSerData;
     }
     bool maybeDecompressAndCatch() const {
         if ( shouldDecompress() ) {
@@ -159,17 +161,22 @@ private:
     }
     void deCompress(); // throw (DeserializeException);
 
+    struct Unlikely {
+        /** The buffers we own. */
+        Unlikely();
+        Unlikely(const Unlikely &);
+        ~Unlikely();
+        std::unique_ptr<serializablearray::BufferMap> _owned;
+        ByteBufferUP             _compSerData;
+        CompressionConfig::Type  _serializedCompression;
+        uint32_t                 _uncompressedLength;
+    };
     /** Contains the stored attributes, with reference to the real data.. */
-    EntryMap _entries;
-    /** The buffers we own. */
-    std::unique_ptr<serializablearray::BufferMap> _owned;
+    EntryMap                  _entries;
+    std::unique_ptr<Unlikely> _unlikely;
 
     /** Data we deserialized from, if applicable. */
     ByteBufferUP             _uncompSerData;
-    ByteBufferUP             _compSerData;
-    CompressionConfig::Type  _serializedCompression;
-
-    uint32_t     _uncompressedLength;
 
     VESPA_DLL_LOCAL void invalidate();
     VESPA_DLL_LOCAL EntryMap::const_iterator find(int id) const;