Merge pull request #11700 from vespa-engine/balder/use-shared-ptr

Balder/use shared ptr

4 files changed, 32 insertions, 30 deletions

diff --git a/searchlib/src/tests/docstore/document_store/document_store_test.cpp b/searchlib/src/tests/docstore/document_store/document_store_test.cpp
index 86c6d3ad883..f950377be4b 100644
--- a/searchlib/src/tests/docstore/document_store/document_store_test.cpp
+++ b/searchlib/src/tests/docstore/document_store/document_store_test.cpp
@@ -105,9 +105,9 @@ void verifyValue(vespalib::stringref s, const Value & v) {
 TEST("require that Value and cache entries have expected size") {
     using pair = std::pair<DocumentIdT, Value>;
     using Node = vespalib::hash_node<pair>;
-    EXPECT_EQUAL(64ul, sizeof(Value));
-    EXPECT_EQUAL(72ul, sizeof(pair));
-    EXPECT_EQUAL(80ul, sizeof(Node));
+    EXPECT_EQUAL(48ul, sizeof(Value));
+    EXPECT_EQUAL(56ul, sizeof(pair));
+    EXPECT_EQUAL(64ul, sizeof(Node));
 }
 
 TEST("require that Value can store uncompressed data") {
@@ -144,7 +144,7 @@ TEST("require that Value can store zstd compressed data") {
 
 TEST("require that Value can detect if output not equal to input") {
     Value v = createValue(S1, CompressionConfig::NONE);
-    static_cast<uint8_t *>(v.get())[8] ^= 0xff;
+    const_cast<uint8_t *>(static_cast<const uint8_t *>(v.get()))[8] ^= 0xff;
     EXPECT_FALSE(v.decompressed().second);
 }
 
diff --git a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp
index f92f5fd9581..c383358db9e 100644
--- a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp
+++ b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp
@@ -626,7 +626,7 @@ TEST("test that the integrated visit cache works.") {
     for (size_t i(1); i <= 100; i++) {
         vcs.verifyRead(i);
     }
-    constexpr size_t BASE_SZ = 22174;
+    constexpr size_t BASE_SZ = 20594;
     TEST_DO(verifyCacheStats(ds.getCacheStats(), 0, 100, 100, BASE_SZ));
     for (size_t i(1); i <= 100; i++) {
         vcs.verifyRead(i);
@@ -648,16 +648,16 @@ TEST("test that the integrated visit cache works.") {
     vcs.verifyVisit({7,9,17,19,67,88,89}, true);
     TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 99, BASE_SZ+180));
     vcs.rewrite(17);
-    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 97, BASE_SZ-680));
+    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 103, 97, BASE_SZ-671));
     vcs.verifyVisit({7,9,17,19,67,88,89}, true);
     TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 98, BASE_SZ-20));
     vcs.remove(17);
-    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 97, BASE_SZ-680));
+    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 104, 97, BASE_SZ-671));
     vcs.verifyVisit({7,9,17,19,67,88,89}, {7,9,19,67,88,89}, true);
-    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-90));
+    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 105, 98, BASE_SZ-89));
 
     vcs.verifyVisit({41, 42}, true);
-    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ+210));
+    TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 106, 99, BASE_SZ+215));
     vcs.verifyVisit({43, 44}, true);
     TEST_DO(verifyCacheStats(ds.getCacheStats(), 101, 107, 100, BASE_SZ+520));
     vcs.verifyVisit({41, 42, 43, 44}, true);
diff --git a/searchlib/src/vespa/searchlib/docstore/value.cpp b/searchlib/src/vespa/searchlib/docstore/value.cpp
index 68bb060683d..ea29c894cba 100644
--- a/searchlib/src/vespa/searchlib/docstore/value.cpp
+++ b/searchlib/src/vespa/searchlib/docstore/value.cpp
@@ -12,29 +12,29 @@ namespace search::docstore {
 
 Value::Value()
     : _syncToken(0),
+      _uncompressedCrc(0),
       _compressedSize(0),
       _uncompressedSize(0),
-      _uncompressedCrc(0),
+      _buf(),
       _compression(CompressionConfig::NONE)
 {}
 
 Value::Value(uint64_t syncToken)
     : _syncToken(syncToken),
+      _uncompressedCrc(0),
       _compressedSize(0),
       _uncompressedSize(0),
-      _uncompressedCrc(0),
+      _buf(),
       _compression(CompressionConfig::NONE)
 {}
 
-Value::Value(const Value &rhs)
-    : _syncToken(rhs._syncToken),
-      _compressedSize(rhs._compressedSize),
-      _uncompressedSize(rhs._uncompressedSize),
-      _uncompressedCrc(rhs._uncompressedCrc),
-      _compression(rhs._compression),
-      _buf(Alloc::alloc(rhs.size()))
-{
-    memcpy(get(), rhs.get(), size());
+Value::Value(const Value &rhs) = default;
+
+Value::~Value() = default;
+
+const void *
+Value::get() const {
+    return _buf ? _buf->get() : nullptr;
 }
 
 void
@@ -44,16 +44,18 @@ Value::set(vespalib::DataBuffer &&buf, ssize_t len) {
 
 void
 Value::set(vespalib::DataBuffer &&buf, ssize_t len, const CompressionConfig &compression) {
+    assert(len < std::numeric_limits<uint32_t>::max());
     //Underlying buffer must be identical to allow swap.
     vespalib::DataBuffer compressed(buf.getData(), 0u);
     vespalib::ConstBufferRef input(buf.getData(), len);
     CompressionConfig::Type type = compress(compression, input, compressed, true);
     _compressedSize = compressed.getDataLen();
+
     if (buf.getData() == compressed.getData()) {
         // Uncompressed so we can just steal the underlying buffer.
-        buf.stealBuffer().swap(_buf);
+        _buf = std::make_shared<Alloc>(buf.stealBuffer());
     } else {
-        compressed.stealBuffer().swap(_buf);
+        _buf = std::make_shared<Alloc>(compressed.stealBuffer());
     }
     assert(((type == CompressionConfig::NONE) &&
             (len == ssize_t(_compressedSize))) ||
diff --git a/searchlib/src/vespa/searchlib/docstore/value.h b/searchlib/src/vespa/searchlib/docstore/value.h
index 426bcaf0e31..f58d96e3e77 100644
--- a/searchlib/src/vespa/searchlib/docstore/value.h
+++ b/searchlib/src/vespa/searchlib/docstore/value.h
@@ -25,6 +25,7 @@ public:
     Value &operator=(Value &&rhs) = default;
 
     Value(const Value &rhs);
+    ~Value();
 
     uint64_t getSyncToken() const { return _syncToken; }
     CompressionConfig::Type getCompression() const { return _compression; }
@@ -42,16 +43,15 @@ public:
 
     size_t size() const { return _compressedSize; }
     bool empty() const { return size() == 0; }
-    operator const void *() const { return _buf.get(); }
-    const void *get() const { return _buf.get(); }
-    void *get() { return _buf.get(); }
+    operator const void *() const { return get(); }
+    const void *get() const;
 private:
-    uint64_t _syncToken;
-    size_t   _compressedSize;
-    size_t   _uncompressedSize;
-    uint64_t _uncompressedCrc;
+    uint64_t                _syncToken;
+    uint64_t                _uncompressedCrc;
+    uint32_t                _compressedSize;
+    uint32_t                _uncompressedSize;
+    std::shared_ptr<Alloc>  _buf;
     CompressionConfig::Type _compression;
-    Alloc _buf;
 };
 
 }