Optimize multiple FieldPathUpdates by using a transaction concept.

5 files changed, 155 insertions, 35 deletions

diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp
index 643dc03018c..b2da5e51b74 100644
--- a/document/src/tests/documenttestcase.cpp
+++ b/document/src/tests/documenttestcase.cpp
@@ -76,9 +76,9 @@ CPPUNIT_TEST_SUITE_REGISTRATION(DocumentTest);
 
 void DocumentTest::testSizeOf()
 {
-    CPPUNIT_ASSERT_EQUAL(120ul, sizeof(Document));
-    CPPUNIT_ASSERT_EQUAL(64ul, sizeof(StructFieldValue));
-    CPPUNIT_ASSERT_EQUAL(16ul, sizeof(StructuredFieldValue));
+    CPPUNIT_ASSERT_EQUAL(136ul, sizeof(Document));
+    CPPUNIT_ASSERT_EQUAL(72ul, sizeof(StructFieldValue));
+    CPPUNIT_ASSERT_EQUAL(24ul, sizeof(StructuredFieldValue));
     CPPUNIT_ASSERT_EQUAL(64ul, sizeof(SerializableArray));
 }
 
diff --git a/document/src/vespa/document/base/field.h b/document/src/vespa/document/base/field.h
index 47ee61c9fc0..cb16538c4f1 100644
--- a/document/src/vespa/document/base/field.h
+++ b/document/src/vespa/document/base/field.h
@@ -80,6 +80,7 @@ public:
     bool contains(const FieldSet& fields) const override;
     Type getType() const override { return FIELD; }
     bool valid() const { return _fieldId != 0; }
+    uint32_t hash() const { return getId(); }
 private:
     int calculateIdV7();
 
diff --git a/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp b/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp
index 70b853315f0..92d4e4788fb 100644
--- a/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp
+++ b/document/src/vespa/document/fieldvalue/structuredfieldvalue.cpp
@@ -4,6 +4,7 @@
 #include "iteratorhandler.h"
 #include "weightedsetfieldvalue.h"
 #include "arrayfieldvalue.h"
+#include <vespa/vespalib/stllike/hash_map.hpp>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".document.fieldvalue.structured");
@@ -42,6 +43,8 @@ StructuredFieldValue::StructuredFieldValue(const DataType &type)
 {
 }
 
+StructuredFieldValue::~StructuredFieldValue() {}
+
 void StructuredFieldValue::setType(const DataType& type)
 {
     _type = &type;
@@ -87,6 +90,78 @@ StructuredFieldValue::onGetNestedFieldValue(PathRange nested) const
     return fv;
 }
 
+namespace {
+    using ValuePair = std::pair<fieldvalue::ModificationStatus, FieldValue::UP>;
+    using Cache = vespalib::hash_map<Field, ValuePair>;
+}
+
+class StructuredCache {
+public:
+    void remove(const Field & field) {
+        ValuePair & entry = _cache[field];
+        entry.first = ModificationStatus::REMOVED;
+        entry.second.reset();
+    }
+    Cache::iterator find(const Field & field) {
+        return _cache.find(field);
+    }
+    void set(const Field & field, FieldValue::UP value, ModificationStatus status) {
+        ValuePair & entry = _cache[field];
+        entry.first = status;
+        entry.second = std::move(value);
+    }
+    Cache::iterator begin() { return _cache.begin(); }
+    Cache::iterator end() { return _cache.end(); }
+private:
+    Cache _cache;
+};
+
+
+void
+StructuredFieldValue::remove(const Field& field) {
+    if (_cache) {
+        _cache->remove(field);
+    } else {
+        removeFieldValue(field);
+    }
+}
+
+void
+StructuredFieldValue::updateValue(const Field & field, FieldValue::UP value) const {
+    if (_cache) {
+        _cache->set(field, std::move(value), ModificationStatus::MODIFIED);
+    } else {
+        const_cast<StructuredFieldValue&>(*this).setFieldValue(field, std::move(value));
+    }
+}
+
+void
+StructuredFieldValue::returnValue(const Field & field, FieldValue::UP value) const {
+    if (_cache) {
+        _cache->set(field, std::move(value), ModificationStatus::NOT_MODIFIED);
+    }
+}
+
+FieldValue::UP
+StructuredFieldValue::getValue(const Field& field, FieldValue::UP container) const {
+    if (_cache) {
+        auto found = _cache->find(field);
+        if (found == _cache->end()) {
+            container = getFieldValue(field);
+            _cache->set(field, FieldValue::UP(), ModificationStatus::NOT_MODIFIED);
+        } else {
+            container = std::move(found->second.second);
+        }
+    } else {
+        if (container) {
+            getFieldValue(field, *container);
+        } else {
+            container = getFieldValue(field);
+        }
+    }
+    return container;
+}
+
 ModificationStatus
 StructuredFieldValue::onIterateNested(PathRange nested, IteratorHandler & handler) const
 {
@@ -95,33 +170,34 @@ StructuredFieldValue::onIterateNested(PathRange nested, IteratorHandler & handle
     if ( ! nested.atEnd()) {
         const FieldPathEntry & fpe = nested.cur();
         if (fpe.getType() == FieldPathEntry::STRUCT_FIELD) {
-            bool exists = getValue(fpe.getFieldRef(), fpe.getFieldValueToSet());
-            LOG(spam, "fieldRef = %s", fpe.getFieldRef().toString().c_str());
-            LOG(spam, "fieldValueToSet = %s", fpe.getFieldValueToSet().toString().c_str());
-            if (exists) {
-                ModificationStatus status = fpe.getFieldValueToSet().iterateNested(nested.next(), handler);
+            const Field & field = fpe.getFieldRef();
+            FieldValue::UP value = getValue(field, FieldValue::UP());
+            LOG(spam, "fieldRef = %s", field.toString().c_str());
+            LOG(spam, "fieldValueToSet = %s", value->toString().c_str());
+            ModificationStatus status = ModificationStatus::NOT_MODIFIED;
+            if (value) {
+                status = value->iterateNested(nested.next(), handler);
                 if (status == ModificationStatus::REMOVED) {
                     LOG(spam, "field exists, status = REMOVED");
-                    const_cast<StructuredFieldValue&>(*this).remove(fpe.getFieldRef());
-                    return ModificationStatus::MODIFIED;
+                    const_cast<StructuredFieldValue&>(*this).remove(field);
+                    status = ModificationStatus::MODIFIED;
                 } else if (status == ModificationStatus::MODIFIED) {
                     LOG(spam, "field exists, status = MODIFIED");
-                    const_cast<StructuredFieldValue&>(*this).setFieldValue(fpe.getFieldRef(), fpe.stealFieldValueToSet());
-                    return ModificationStatus::MODIFIED;
+                    updateValue(field, std::move(value));
                 } else {
-                    return status;
+                    returnValue(field, std::move(value));
                 }
             } else if (handler.createMissingPath()) {
                 LOG(spam, "createMissingPath is true");
-                ModificationStatus status = fpe.getFieldValueToSet().iterateNested(nested.next(), handler);
+                status = fpe.getFieldValueToSet().iterateNested(nested.next(), handler);
                 if (status == ModificationStatus::MODIFIED) {
                     LOG(spam, "field did not exist, status = MODIFIED");
-                    const_cast<StructuredFieldValue&>(*this).setFieldValue(fpe.getFieldRef(), fpe.stealFieldValueToSet());
-                    return status;
+                    updateValue(field, fpe.stealFieldValueToSet());
                 }
+            } else {
+                LOG(spam, "field did not exist, returning NOT_MODIFIED");
             }
-            LOG(spam, "field did not exist, returning NOT_MODIFIED");
-            return ModificationStatus::NOT_MODIFIED;
+            return status;
         } else {
             throw IllegalArgumentException("Illegal field path for struct value");
         }
@@ -129,10 +205,7 @@ StructuredFieldValue::onIterateNested(PathRange nested, IteratorHandler & handle
         ModificationStatus status = handler.modify(const_cast<StructuredFieldValue&>(*this));
         if (status == ModificationStatus::REMOVED) {
             LOG(spam, "field REMOVED");
-            return status;
-        }
-
-        if (handler.handleComplex(*this)) {
+        } else if (handler.handleComplex(*this)) {
             LOG(spam, "handleComplex");
             std::vector<const Field*> fieldsToRemove;
             for (const_iterator it(begin()), mt(end()); it != mt; ++it) {
@@ -141,7 +214,7 @@ StructuredFieldValue::onIterateNested(PathRange nested, IteratorHandler & handle
                     fieldsToRemove.push_back(&it.field());
                     status = ModificationStatus::MODIFIED;
                 } else if (currStatus == ModificationStatus::MODIFIED) {
-                    status = currStatus;
+                    status = ModificationStatus::MODIFIED;
                 }
             }
 
@@ -154,6 +227,22 @@ StructuredFieldValue::onIterateNested(PathRange nested, IteratorHandler & handle
     }
 }
 
+void
+StructuredFieldValue::beginTransaction() {
+    _cache = std::make_unique<StructuredCache>();
+}
+void
+StructuredFieldValue::commitTransaction() {
+    for (auto & e : *_cache) {
+        if (e.second.first == ModificationStatus::REMOVED) {
+            removeFieldValue(e.first);
+        } else if (e.second.first == ModificationStatus ::MODIFIED) {
+            setFieldValue(e.first, std::move(e.second.second));
+        }
+    }
+    _cache.reset();
+}
+
 using ConstCharP = const char *;
 template void StructuredFieldValue::set(const vespalib::stringref & field, int32_t value);
 template void StructuredFieldValue::set(const vespalib::stringref & field, int64_t value);
diff --git a/document/src/vespa/document/fieldvalue/structuredfieldvalue.h b/document/src/vespa/document/fieldvalue/structuredfieldvalue.h
index 7e38a512da3..d93bee9b093 100644
--- a/document/src/vespa/document/fieldvalue/structuredfieldvalue.h
+++ b/document/src/vespa/document/fieldvalue/structuredfieldvalue.h
@@ -13,6 +13,8 @@
 #include "fieldvalue.h"
 #include <vespa/document/base/field.h>
 
+#define VESPA_DLL_LOCAL  __attribute__ ((visibility("hidden")))
+
 namespace document {
 
 class ArrayFieldValue;
@@ -26,12 +28,20 @@ class StringFieldValue;
 class StructFieldValue;
 class MapFieldValue;
 class WeightedSetFieldValue;
+class StructuredCache;
 
 class StructuredFieldValue : public FieldValue
 {
     const DataType *_type;
+    std::unique_ptr<StructuredCache> _cache;
 
     UP onGetNestedFieldValue(PathRange nested) const override;
+    /** @return Retrieve value of given field. Null pointer if not set.
+     * Will use container as inplace is present.
+     */
+    VESPA_DLL_LOCAL FieldValue::UP getValue(const Field& field, FieldValue::UP container) const;
+    VESPA_DLL_LOCAL void updateValue(const Field & field, FieldValue::UP value) const;
+    VESPA_DLL_LOCAL void returnValue(const Field & field, FieldValue::UP value) const;
 
 protected:
     StructuredFieldValue(const DataType &type);
@@ -94,10 +104,9 @@ protected:
 
     fieldvalue::ModificationStatus
     onIterateNested(PathRange nested, fieldvalue::IteratorHandler & handler) const override;
-
 public:
     DECLARE_IDENTIFIABLE_ABSTRACT(StructuredFieldValue);
-
+    ~StructuredFieldValue();
 
     virtual StructuredFieldValue* clone() const override = 0;
     const DataType *getDataType() const override { return _type; }
@@ -110,23 +119,30 @@ public:
      */
     virtual const Field& getField(const vespalib::stringref & name) const = 0;
 
+    void beginTransaction();
+    void commitTransaction();
+
     /**
      * Retrieve value of given field and assign it to given field.
      *
      * @return True if field is set and stored in value, false if unset.
      * @throws vespalib::IllegalArgumentException If value given has wrong type
      */
-    inline bool getValue(const Field& field, FieldValue& value) const
-        { return getFieldValue(field, value); }
+    bool getValue(const Field& field, FieldValue& value) const {
+        return getFieldValue(field, value);
+    }
     /** @return Retrieve value of given field. Null pointer if not set. */
-    inline FieldValue::UP getValue(const Field& field) const
-        { return getFieldValue(field); }
+    FieldValue::UP getValue(const Field& field) const {
+        return getFieldValue(field);
+    }
     /** @return Retrieve value of given field. Null pointer if not set. */
-    inline FieldValue::UP getValue(const vespalib::stringref & name) const
-        { return getFieldValue(getField(name)); }
+    FieldValue::UP getValue(const vespalib::stringref & name) const {
+        return getFieldValue(getField(name));
+    }
     /** @return True if value is set. */
-    inline bool hasValue(const Field& field) const
-        { return hasFieldValue(field); }
+    bool hasValue(const Field& field) const {
+        return hasFieldValue(field);
+    }
 
     /**
      * Set the given field to contain given value.
@@ -136,8 +152,9 @@ public:
     inline void setValue(const Field& field, const FieldValue& value)
         { setFieldValue(field, value); }
     /** Remove the value of given field if it is set. */
-    inline void remove(const Field& field)
-        { removeFieldValue(field); }
+
+    //These are affected by the begin/commitTanasaction
+    void remove(const Field& field);
 
     virtual void clear() = 0;
 
@@ -175,5 +192,17 @@ public:
     std::unique_ptr<T> getAs(const Field &field) const;
 };
 
+class TransactionGuard {
+public:
+    TransactionGuard(StructuredFieldValue & value)
+        : _value(value)
+    {
+        _value.beginTransaction();
+    }
+    ~TransactionGuard() { _value.commitTransaction(); }
+private:
+    StructuredFieldValue & _value;
+};
+
 } // document
 
diff --git a/document/src/vespa/document/update/documentupdate.cpp b/document/src/vespa/document/update/documentupdate.cpp
index cedbd32107b..717df16e62c 100644
--- a/document/src/vespa/document/update/documentupdate.cpp
+++ b/document/src/vespa/document/update/documentupdate.cpp
@@ -174,6 +174,7 @@ DocumentUpdate::applyTo(Document& doc) const
     for(const auto & update : _updates) {
         update.applyTo(doc);
     }
+    TransactionGuard guard(doc);
     // Apply fieldpath updates
     for (const auto & update : _fieldPathUpdates) {
         update->applyTo(doc);