From 61857dbdbffcdc116ab9a365a95e1da131fe7a89 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 27 Jul 2017 09:42:10 +0200 Subject: Optimize multiple FieldPathUpdates by using a transaction concept. --- document/src/tests/documenttestcase.cpp | 6 +- document/src/vespa/document/base/field.h | 1 + .../document/fieldvalue/structuredfieldvalue.cpp | 129 +++++++++++++++++---- .../document/fieldvalue/structuredfieldvalue.h | 53 +++++++-- .../src/vespa/document/update/documentupdate.cpp | 1 + 5 files changed, 155 insertions(+), 35 deletions(-) (limited to 'document') 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 #include 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; + using Cache = vespalib::hash_map; +} + +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(*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(*this).remove(fpe.getFieldRef()); - return ModificationStatus::MODIFIED; + const_cast(*this).remove(field); + status = ModificationStatus::MODIFIED; } else if (status == ModificationStatus::MODIFIED) { LOG(spam, "field exists, status = MODIFIED"); - const_cast(*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(*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(*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 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(); +} +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 +#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 _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 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); -- cgit v1.2.3