Simplify by avoid both DocumentSize and PersistedDocumentSize. That is the same.

7 files changed, 153 insertions, 164 deletions

diff --git a/persistence/src/tests/dummyimpl/dummypersistence_test.cpp b/persistence/src/tests/dummyimpl/dummypersistence_test.cpp
index 21cf7cb9ae3..3b30f5e29ca 100644
--- a/persistence/src/tests/dummyimpl/dummypersistence_test.cpp
+++ b/persistence/src/tests/dummyimpl/dummypersistence_test.cpp
@@ -9,7 +9,6 @@
 #include <vespa/vdslib/state/clusterstate.h>
 #include <vespa/config-stor-distribution.h>
 
-
 using namespace storage::spi;
 using namespace storage;
 using document::test::makeBucketSpace;
@@ -21,7 +20,7 @@ struct Fixture {
     BucketContent content;
 
     void insert(DocumentId id, Timestamp timestamp, int meta_flags) {
-        content.insert(DocEntry::UP(new DocEntry(timestamp, meta_flags, id)));
+        content.insert(DocEntry::create(timestamp, meta_flags, id));
     }
 
     Fixture() {
diff --git a/persistence/src/tests/spi/clusterstatetest.cpp b/persistence/src/tests/spi/clusterstatetest.cpp
index 9d03cb982c8..205661ba930 100644
--- a/persistence/src/tests/spi/clusterstatetest.cpp
+++ b/persistence/src/tests/spi/clusterstatetest.cpp
@@ -262,62 +262,42 @@ TEST(ClusterStateTest, node_maintenance_state_is_set_independent_of_bucket_space
 }
 
 TEST(DocEntryTest, test_basics) {
-    EXPECT_EQ(40u, sizeof(DocEntry));
+    EXPECT_EQ(24, sizeof(DocEntry));
 }
 
 TEST(DocEntryTest, test_meta_only) {
-    DocEntry e(Timestamp(9), 0);
-    EXPECT_EQ(9, e.getTimestamp());
-    EXPECT_FALSE(e.isRemove());
-    EXPECT_EQ(40, e.getSize());
-    EXPECT_EQ(0, e.getDocumentSize());
-    EXPECT_EQ(0, e.getPersistedDocumentSize());
-    EXPECT_EQ(nullptr, e.getDocument());
-    EXPECT_EQ(nullptr, e.getDocumentId());
-
-    e.setPersistedDocumentSize(7);
-    EXPECT_EQ(40, e.getSize());
-    EXPECT_EQ(0, e.getDocumentSize());
-    EXPECT_EQ(7, e.getPersistedDocumentSize());
-
-    DocEntry r(Timestamp(666), 1);
-    EXPECT_EQ(666, r.getTimestamp());
-    EXPECT_TRUE(r.isRemove());
+    DocEntry::UP e = DocEntry::create(Timestamp(9), 0);
+    EXPECT_EQ(9, e->getTimestamp());
+    EXPECT_FALSE(e->isRemove());
+    EXPECT_EQ(24, e->getSize());
+    EXPECT_EQ(0, e->getDocumentSize());
+    EXPECT_EQ(nullptr, e->getDocument());
+    EXPECT_EQ(nullptr, e->getDocumentId());
+
+    DocEntry::UP r = DocEntry::create(Timestamp(666), 1);
+    EXPECT_EQ(666, r->getTimestamp());
+    EXPECT_TRUE(r->isRemove());
 }
 
 TEST(DocEntryTest, test_docid_only) {
-    DocEntry e(Timestamp(9), 0, DocumentId("id:test:test::1"));
-    EXPECT_EQ(9, e.getTimestamp());
-    EXPECT_FALSE(e.isRemove());
-    EXPECT_EQ(56, e.getSize());
-    EXPECT_EQ(16, e.getDocumentSize());
-    EXPECT_EQ(16, e.getPersistedDocumentSize());
-    EXPECT_EQ(nullptr, e.getDocument());
-    EXPECT_NE(nullptr, e.getDocumentId());
-
-    e.setPersistedDocumentSize(7);
-    EXPECT_EQ(56, e.getSize());
-    EXPECT_EQ(16, e.getDocumentSize());
-    EXPECT_EQ(7, e.getPersistedDocumentSize());
-
+    DocEntry::UP e = DocEntry::create(Timestamp(9), 0, DocumentId("id:test:test::1"));
+    EXPECT_EQ(9, e->getTimestamp());
+    EXPECT_FALSE(e->isRemove());
+    EXPECT_EQ(48, e->getSize());
+    EXPECT_EQ(16, e->getDocumentSize());
+    EXPECT_EQ(nullptr, e->getDocument());
+    EXPECT_NE(nullptr, e->getDocumentId());
 }
 
 TEST(DocEntryTest, test_document_only) {
     document::TestDocMan testDocMan;
-    DocEntry e(Timestamp(9), 0, testDocMan.createRandomDocument(0, 1000));
-    EXPECT_EQ(9, e.getTimestamp());
-    EXPECT_FALSE(e.isRemove());
-    EXPECT_EQ(672, e.getSize());
-    EXPECT_EQ(632, e.getDocumentSize());
-    EXPECT_EQ(632, e.getPersistedDocumentSize());
-    EXPECT_NE(nullptr, e.getDocument());
-    EXPECT_NE(nullptr, e.getDocumentId());
-
-    e.setPersistedDocumentSize(7);
-    EXPECT_EQ(672, e.getSize());
-    EXPECT_EQ(632, e.getDocumentSize());
-    EXPECT_EQ(7, e.getPersistedDocumentSize());
-
+    DocEntry::UP e = DocEntry::create(Timestamp(9), 0, testDocMan.createRandomDocument(0, 1000));
+    EXPECT_EQ(9, e->getTimestamp());
+    EXPECT_FALSE(e->isRemove());
+    EXPECT_EQ(664, e->getSize());
+    EXPECT_EQ(632, e->getDocumentSize());
+    EXPECT_NE(nullptr, e->getDocument());
+    EXPECT_NE(nullptr, e->getDocumentId());
 }
 
 }
diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
index 57b6b282e60..ec68f02f8b0 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
@@ -258,14 +258,14 @@ verifyDocs(const std::vector<DocAndTimestamp>& wanted,
             }
             EXPECT_EQ(wanted[wantedIdx].timestamp, entry.getTimestamp());
             size_t serSize = wanted[wantedIdx].doc->serialize().size();
-            EXPECT_EQ(serSize + sizeof(DocEntry), size_t(entry.getSize()));
+            EXPECT_EQ(serSize + entry.getOwnSize(), size_t(entry.getSize()));
             EXPECT_EQ(serSize, size_t(entry.getDocumentSize()));
             ++wantedIdx;
         } else {
             // Remove-entry
             EXPECT_TRUE(entry.getDocumentId() != 0);
             size_t serSize = entry.getDocumentId()->getSerializedSize();
-            EXPECT_EQ(serSize + sizeof(DocEntry), size_t(entry.getSize()));
+            EXPECT_EQ(serSize + entry.getOwnSize(), size_t(entry.getSize()));
             EXPECT_EQ(serSize, size_t(entry.getDocumentSize()));
             if (removes.find(entry.getDocumentId()->toString()) == removes.end()) {
                 FAIL() << "Got unexpected remove entry for document id "
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
index 142b7faad9d..19c03a97f05 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -25,6 +25,7 @@ using vespalib::make_string;
 using std::binary_search;
 using std::lower_bound;
 using document::FixedBucketSpaces;
+using document::FieldSet;
 using storage::spi::test::cloneDocEntry;
 using storage::spi::test::equal;
 
@@ -434,7 +435,7 @@ DummyPersistence::putAsync(const Bucket& b, Timestamp t, Document::SP doc, Conte
         }
     } else {
         LOG(spam, "Inserting document %s", doc->toString(true).c_str());
-        auto entry = std::make_unique<DocEntry>(t, NONE, Document::UP(doc->clone()));
+        auto entry = DocEntry::create(t, NONE, Document::UP(doc->clone()));
         (*bc)->insert(std::move(entry));
         bc.reset();
         onComplete->onComplete(std::make_unique<Result>());
@@ -492,7 +493,7 @@ DummyPersistence::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentI
         }
         DocEntry::SP entry((*bc)->getEntry(id));
         numRemoves += (entry.get() && !entry->isRemove()) ? 1 : 0;
-        auto remEntry = std::make_unique<DocEntry>(t, REMOVE_ENTRY, id);
+        auto remEntry = DocEntry::create(t, REMOVE_ENTRY, id);
 
         if ((*bc)->hasTimestamp(t)) {
             (*bc)->eraseEntry(t);
@@ -504,7 +505,7 @@ DummyPersistence::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentI
 }
 
 GetResult
-DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const DocumentId& did, Context&) const
+DummyPersistence::get(const Bucket& b, const FieldSet& fieldSet, const DocumentId& did, Context&) const
 {
     DUMMYPERSISTENCE_VERIFY_INITIALIZED;
     LOG(debug, "get(%s, %s)",
@@ -521,8 +522,8 @@ DummyPersistence::get(const Bucket& b, const document::FieldSet& fieldSet, const
             return GetResult::make_for_tombstone(entry->getTimestamp());
         } else {
             Document::UP doc(entry->getDocument()->clone());
-            if (fieldSet.getType() != document::FieldSet::Type::ALL) {
-                document::FieldSet::stripFields(*doc, fieldSet);
+            if (fieldSet.getType() != FieldSet::Type::ALL) {
+                FieldSet::stripFields(*doc, fieldSet);
             }
             return GetResult(std::move(doc), entry->getTimestamp());
         }
@@ -652,18 +653,13 @@ DummyPersistence::iterate(IteratorId id, uint64_t maxByteSize, Context& ctx) con
             if (currentSize != 0 && currentSize + size > maxByteSize) break;
             currentSize += size;
             if (!entry->isRemove()
-                && it->_fieldSet->getType() != document::FieldSet::Type::ALL)
+                && it->_fieldSet->getType() != FieldSet::Type::ALL)
             {
                 assert(entry->getDocument());
                 // Create new document with only wanted fields.
-                Document::UP filtered(
-                        document::FieldSet::createDocumentSubsetCopy(
-                                *entry->getDocument(),
-                                *it->_fieldSet));
-                DocEntry::UP ret(new DocEntry(entry->getTimestamp(),
-                                              entry->getFlags(),
-                                              std::move(filtered),
-                                              entry->getPersistedDocumentSize()));
+                Document::UP filtered(FieldSet::createDocumentSubsetCopy(*entry->getDocument(), *it->_fieldSet));
+                auto ret = DocEntry::create(entry->getTimestamp(), entry->getFlags(),
+                                            std::move(filtered), entry->getDocumentSize());
                 entries.push_back(std::move(ret));
             } else {
                 // Use entry as-is.
diff --git a/persistence/src/vespa/persistence/spi/docentry.cpp b/persistence/src/vespa/persistence/spi/docentry.cpp
index a48f0656b23..e586b4e414f 100644
--- a/persistence/src/vespa/persistence/spi/docentry.cpp
+++ b/persistence/src/vespa/persistence/spi/docentry.cpp
@@ -4,73 +4,75 @@
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/vespalib/objects/nbostream.h>
 #include <sstream>
-#include <cassert>
 
 namespace storage::spi {
 
-DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc)
-    : _timestamp(t),
-      _metaFlags(metaFlags),
-      _persistedDocumentSize(doc->serialize().size()),
-      _size(_persistedDocumentSize + sizeof(DocEntry)),
-      _documentId(),
-      _document(std::move(doc))
-{ }
+namespace {
+
+class DocEntryWithId final : public DocEntry {
+public:
+    DocEntryWithId(Timestamp t, int metaFlags, const DocumentId &docId);
+    ~DocEntryWithId();
+    vespalib::string toString() const override;
+    const DocumentId* getDocumentId() const override { return _documentId.get(); }
+private:
+    SizeType getOwnSize() const override{ return sizeof(DocEntryWithId); }
+    DocumentIdUP _documentId;
+};
+
+class DocEntryWithDoc final : public DocEntry {
+public:
+    DocEntryWithDoc(Timestamp t, int metaFlags, DocumentUP doc);
+
+    /**
+     * Constructor that can be used by providers that already know
+     * the serialized size of the document, so the potentially expensive
+     * call to getSerializedSize can be avoided. This value shall be the size of the document _before_
+     * any field filtering is performed.
+     */
+    DocEntryWithDoc(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize);
+    ~DocEntryWithDoc();
+    vespalib::string toString() const override;
+    const Document* getDocument() const override { return _document.get(); }
+    const DocumentId* getDocumentId() const override { return &_document->getId(); }
+    DocumentUP releaseDocument() override { return std::move(_document); }
+private:
+    SizeType getOwnSize() const override { return sizeof(DocEntryWithDoc); }
+    DocumentUP _document;
+};
 
-DocEntry::DocEntry(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize)
-    : _timestamp(t),
-      _metaFlags(metaFlags),
-      _persistedDocumentSize(serializedDocumentSize),
-      _size(_persistedDocumentSize + sizeof(DocEntry)),
-      _documentId(),
+DocEntryWithDoc::DocEntryWithDoc(Timestamp t, int metaFlags, DocumentUP doc)
+    : DocEntry(t, metaFlags, doc->serialize().size()),
       _document(std::move(doc))
 { }
 
-DocEntry::DocEntry(Timestamp t, int metaFlags, const DocumentId& docId)
-    : _timestamp(t),
-      _metaFlags(metaFlags),
-      _persistedDocumentSize(docId.getSerializedSize()),
-      _size(_persistedDocumentSize + sizeof(DocEntry)),
-      _documentId(std::make_unique<DocumentId>(docId)),
-      _document()
+DocEntryWithDoc::DocEntryWithDoc(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize)
+    : DocEntry(t, metaFlags, serializedDocumentSize),
+      _document(std::move(doc))
 { }
 
-DocEntry::DocEntry(Timestamp t, int metaFlags)
-    : _timestamp(t),
-      _metaFlags(metaFlags),
-      _persistedDocumentSize(0),
-      _size(sizeof(DocEntry)),
-      _documentId(),
-      _document()
+DocEntryWithId::DocEntryWithId(Timestamp t, int metaFlags, const DocumentId& docId)
+    : DocEntry(t, metaFlags, docId.getSerializedSize()),
+      _documentId(std::make_unique<DocumentId>(docId))
 { }
 
-DocEntry::~DocEntry() = default;
+DocEntryWithId::~DocEntryWithId() = default;
+DocEntryWithDoc::~DocEntryWithDoc() = default;
 
-const DocumentId*
-DocEntry::getDocumentId() const {
-    return (_document ? &_document->getId() : _documentId.get());
-}
-
-DocumentUP
-DocEntry::releaseDocument() {
-    return std::move(_document);
-}
-
-DocEntry::SizeType
-DocEntry::getDocumentSize() const
+vespalib::string
+DocEntryWithId::toString() const
 {
-    assert(_size >= sizeof(DocEntry));
-    return _size - sizeof(DocEntry);
+    std::ostringstream out;
+    out << "DocEntry(" << getTimestamp() << ", " << getFlags() << ", " << *_documentId << ")";
+    return out.str();
 }
 
 vespalib::string
-DocEntry::toString() const
+DocEntryWithDoc::toString() const
 {
     std::ostringstream out;
-    out << "DocEntry(" << _timestamp << ", " << _metaFlags << ", ";
-    if (_documentId) {
-        out << *_documentId;
-    } else if (_document.get()) {
+    out << "DocEntry(" << getTimestamp() << ", " << getFlags() << ", ";
+    if (_document.get()) {
         out << "Doc(" << _document->getId() << ")";
     } else {
         out << "metadata only";
@@ -79,6 +81,40 @@ DocEntry::toString() const
     return out.str();
 }
 
+}
+
+DocEntry::UP
+DocEntry::create(Timestamp t, int metaFlags) {
+    return std::make_unique<DocEntry>(t, metaFlags);
+}
+DocEntry::UP
+DocEntry::create(Timestamp t, int metaFlags, const DocumentId &docId) {
+    return std::make_unique<DocEntryWithId>(t, metaFlags, docId);
+}
+DocEntry::UP
+DocEntry::create(Timestamp t, int metaFlags, DocumentUP doc) {
+    return std::make_unique<DocEntryWithDoc>(t, metaFlags, std::move(doc));
+}
+DocEntry::UP
+DocEntry::create(Timestamp t, int metaFlags, DocumentUP doc, SizeType serializedDocumentSize) {
+    return std::make_unique<DocEntryWithDoc>(t, metaFlags, std::move(doc), serializedDocumentSize);
+}
+
+DocEntry::~DocEntry() = default;
+
+DocumentUP
+DocEntry::releaseDocument() {
+    return {};
+}
+
+vespalib::string
+DocEntry::toString() const
+{
+    std::ostringstream out;
+    out << "DocEntry(" << _timestamp << ", " << _metaFlags << ", metadata only)";
+    return out.str();
+}
+
 std::ostream &
 operator << (std::ostream & os, const DocEntry & r) {
     return os << r.toString();
diff --git a/persistence/src/vespa/persistence/spi/docentry.h b/persistence/src/vespa/persistence/spi/docentry.h
index 8109c7f04a5..90be8eedc0c 100644
--- a/persistence/src/vespa/persistence/spi/docentry.h
+++ b/persistence/src/vespa/persistence/spi/docentry.h
@@ -24,37 +24,16 @@ enum DocumentMetaFlags {
 
 class DocEntry {
 public:
-    typedef uint32_t SizeType;
-private:
-    Timestamp    _timestamp;
-    int          _metaFlags;
-    SizeType     _persistedDocumentSize;
-    SizeType     _size;
-    DocumentIdUP _documentId;
-    DocumentUP   _document;
-public:
+    using SizeType = uint32_t;
     using UP = std::unique_ptr<DocEntry>;
     using SP = std::shared_ptr<DocEntry>;
 
-    DocEntry(Timestamp t, int metaFlags, DocumentUP doc);
-
-    /**
-     * Constructor that can be used by providers that already know
-     * the serialized size of the document, so the potentially expensive
-     * call to getSerializedSize can be avoided.
-     */
-    DocEntry(Timestamp t, int metaFlags, DocumentUP doc, size_t serializedDocumentSize);
-    DocEntry(Timestamp t, int metaFlags, const DocumentId& docId);
-
-    DocEntry(Timestamp t, int metaFlags);
+    DocEntry(Timestamp t, int metaFlags) : DocEntry(t, metaFlags, 0) { }
     DocEntry(const DocEntry &) = delete;
     DocEntry & operator=(const DocEntry &) = delete;
     DocEntry(DocEntry &&) = delete;
     DocEntry & operator=(DocEntry &&) = delete;
-    ~DocEntry();
-    const Document* getDocument() const { return _document.get(); }
-    const DocumentId* getDocumentId() const;
-    DocumentUP releaseDocument();
+    virtual ~DocEntry();
     bool isRemove() const { return (_metaFlags & REMOVE_ENTRY); }
     Timestamp getTimestamp() const { return _timestamp; }
     int getFlags() const { return _metaFlags; }
@@ -62,33 +41,34 @@ public:
      * @return In-memory size of this doc entry, including document instance.
      *     In essence: serialized size of document + sizeof(DocEntry).
      */
-    SizeType getSize() const { return _size; }
+    SizeType getSize() const { return _size + getOwnSize() ; }
+    virtual SizeType getOwnSize() const { return sizeof(DocEntry); }
     /**
      * If entry contains a document, returns its serialized size.
      * If entry contains a document id, returns the serialized size of
      * the id alone.
      * Otherwise (i.e. metadata only), returns zero.
      */
-    SizeType getDocumentSize() const;
-    /**
-     * Return size of document as it exists in persisted form. By default
-     * this will return the serialized size of the entry's document instance,
-     * but for persistence providers that are able to provide this information
-     * efficiently, this value can be set explicitly to provide better statistical
-     * tracking for e.g. visiting operations in the service layer.
-     * If explicitly set, this value shall be the size of the document _before_
-     * any field filtering is performed.
-     */
-    SizeType getPersistedDocumentSize() const { return _persistedDocumentSize; }
-    /**
-     * Set persisted size of document. Optional.
-     * @see getPersistedDocumentSize
-     */
-    void setPersistedDocumentSize(SizeType persistedDocumentSize) {
-        _persistedDocumentSize = persistedDocumentSize;
-    }
+    SizeType getDocumentSize() const { return _size; }
 
-    vespalib::string toString() const;
+    virtual vespalib::string toString() const;
+    virtual const Document* getDocument() const { return nullptr; }
+    virtual const DocumentId* getDocumentId() const { return nullptr; }
+    virtual DocumentUP releaseDocument();
+    static UP create(Timestamp t, int metaFlags);
+    static UP create(Timestamp t, int metaFlags, const DocumentId &docId);
+    static UP create(Timestamp t, int metaFlags, DocumentUP doc);
+    static UP create(Timestamp t, int metaFlags, DocumentUP doc, SizeType serializedDocumentSize);
+protected:
+    DocEntry(Timestamp t, int metaFlags, SizeType size)
+        : _timestamp(t),
+          _metaFlags(metaFlags),
+          _size(size)
+    {}
+private:
+    Timestamp    _timestamp;
+    int          _metaFlags;
+    SizeType     _size;
 };
 
 std::ostream & operator << (std::ostream & os, const DocEntry & r);
diff --git a/persistence/src/vespa/persistence/spi/test.cpp b/persistence/src/vespa/persistence/spi/test.cpp
index c61f21a3893..0f10b839f98 100644
--- a/persistence/src/vespa/persistence/spi/test.cpp
+++ b/persistence/src/vespa/persistence/spi/test.cpp
@@ -21,15 +21,13 @@ std::unique_ptr<DocEntry>
 cloneDocEntry(const DocEntry & e) {
     std::unique_ptr<DocEntry> ret;
     if (e.getDocument()) {
-        ret = std::make_unique<DocEntry>(e.getTimestamp(), e.getFlags(),
+        ret = DocEntry::create(e.getTimestamp(), e.getFlags(),
                            std::make_unique<Document>(*e.getDocument()),
-                           e.getPersistedDocumentSize());
+                           e.getDocumentSize());
     } else if (e.getDocumentId()) {
-        ret = std::make_unique<DocEntry>(e.getTimestamp(), e.getFlags(), *e.getDocumentId());
-        ret->setPersistedDocumentSize(e.getPersistedDocumentSize());
+        ret = DocEntry::create(e.getTimestamp(), e.getFlags(), *e.getDocumentId());
     } else {
-        ret = std::make_unique<DocEntry>(e.getTimestamp(), e.getFlags());
-        ret->setPersistedDocumentSize(e.getPersistedDocumentSize());
+        ret = DocEntry::create(e.getTimestamp(), e.getFlags());
     }
     return ret;
 }
@@ -38,7 +36,7 @@ bool
 equal(const DocEntry & a, const DocEntry & b) {
     if (a.getTimestamp() != b.getTimestamp()) return false;
     if (a.getFlags() != b.getFlags()) return false;
-    if (a.getPersistedDocumentSize() != b.getPersistedDocumentSize()) return false;
+    if (a.getDocumentSize() != b.getDocumentSize()) return false;
 
     if (a.getDocument()) {
         if (!b.getDocument()) return false;