diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-03-18 08:53:44 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2022-03-18 08:53:44 +0000 |
commit | 1bf26e6b6940296f046317e82c8667097ff21ca0 (patch) | |
tree | f55b08be1a804ae6b70b693a537917de66ddfda0 /document | |
parent | 2974fb58ddc245e94f325eb6b1407c16bd6887bc (diff) |
GC unused hasChanged concept. Only done on StructFieldValue as we never reference internal data.
Diffstat (limited to 'document')
29 files changed, 34 insertions, 245 deletions
diff --git a/document/src/tests/documenttestcase.cpp b/document/src/tests/documenttestcase.cpp index 6967be5b969..4c97e99459e 100644 --- a/document/src/tests/documenttestcase.cpp +++ b/document/src/tests/documenttestcase.cpp @@ -52,7 +52,7 @@ TEST(DocumentTest, testSizeOf) EXPECT_EQ(104ul, sizeof(DocumentId)); EXPECT_EQ(256ul, sizeof(Document)); EXPECT_EQ(80ul, sizeof(NumericDataType)); - EXPECT_EQ(32ul, sizeof(LongFieldValue)); + EXPECT_EQ(24ul, sizeof(LongFieldValue)); EXPECT_EQ(104ul, sizeof(StructFieldValue)); EXPECT_EQ(24ul, sizeof(StructuredFieldValue)); EXPECT_EQ(56ul, sizeof(SerializableArray)); @@ -931,48 +931,6 @@ TEST(DocumentTest, testCRC32) /// \todo TODO (was warning): Cannot test for in memory representation altered, as there is no syntax for getting internal refs to data from document. Add test when this is added. } -TEST(DocumentTest, testHasChanged) -{ - TestDocRepo test_repo; - Document doc(*test_repo.getDocumentType("testdoctype1"), - DocumentId("id:ns:testdoctype1::crawler:http://www.ntnu.no/")); - // Before deserialization we are changed. - EXPECT_TRUE(doc.hasChanged()); - - doc.setValue(doc.getField("hstringval"), StringFieldValue("bla bla bla bla bla")); - // Still changed after setting a value of course. - EXPECT_TRUE(doc.hasChanged()); - - nbostream buf; - doc.serialize(buf); - // Setting a value in doc tags us changed. - { - buf.rp(0); - Document doc2(test_repo.getTypeRepo(), buf); - EXPECT_TRUE(!doc2.hasChanged()); - - doc2.setValue("headerval", IntFieldValue::make(13)); - EXPECT_TRUE(doc2.hasChanged()); - } - // Overwriting a value in doc tags us changed. - { - buf.rp(0); - Document doc2(test_repo.getTypeRepo(), buf); - - doc2.setValue("hstringval", StringFieldValue::make("bla bla bla bla bla")); - EXPECT_TRUE(doc2.hasChanged()); - } - // Clearing value tags us changed. - { - buf.rp(0); - Document doc2(test_repo.getTypeRepo(), buf); - - doc2.clear(); - EXPECT_TRUE(doc2.hasChanged()); - } - // Add more tests here when we allow non-const refs to internals -} - TEST(DocumentTest, testSliceSerialize) { // Test that document doesn't need its own bytebuffer, such that we diff --git a/document/src/tests/fieldvalue/fieldvalue_test.cpp b/document/src/tests/fieldvalue/fieldvalue_test.cpp index c9b56bdc4b2..d27877f68c3 100644 --- a/document/src/tests/fieldvalue/fieldvalue_test.cpp +++ b/document/src/tests/fieldvalue/fieldvalue_test.cpp @@ -22,9 +22,9 @@ TEST("require that StringFieldValue can be assigned primitives") { TEST("require that FieldValues does not change their storage size.") { EXPECT_EQUAL(16u, sizeof(FieldValue)); - EXPECT_EQUAL(24u, sizeof(IntFieldValue)); - EXPECT_EQUAL(32u, sizeof(LongFieldValue)); - EXPECT_EQUAL(112u, sizeof(StringFieldValue)); + EXPECT_EQUAL(16u, sizeof(IntFieldValue)); + EXPECT_EQUAL(24u, sizeof(LongFieldValue)); + EXPECT_EQUAL(104u, sizeof(StringFieldValue)); } } // namespace diff --git a/document/src/tests/fieldvalue/referencefieldvalue_test.cpp b/document/src/tests/fieldvalue/referencefieldvalue_test.cpp index db380353a55..fccce30aaf8 100644 --- a/document/src/tests/fieldvalue/referencefieldvalue_test.cpp +++ b/document/src/tests/fieldvalue/referencefieldvalue_test.cpp @@ -7,7 +7,6 @@ #include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/util/exceptions.h> #include <ostream> -#include <sstream> using namespace document; @@ -44,14 +43,6 @@ TEST_F("Reference can be constructed with document ID", Fixture) { EXPECT_EQUAL(DocumentId("id:ns:foo::itsa-me"), fv.getDocumentId()); } -TEST_F("Newly constructed reference is marked as changed", Fixture) { - ReferenceFieldValue fv(f.refType); - EXPECT_TRUE(fv.hasChanged()); - - ReferenceFieldValue fv2(f.refType, DocumentId("id:ns:foo::itsa-me")); - EXPECT_TRUE(fv2.hasChanged()); -} - TEST_F("Exception is thrown if constructor doc ID type does not match referenced document type", Fixture) { EXPECT_EXCEPTION( ReferenceFieldValue(f.refType, DocumentId("id:ns:bar::wario-time")), @@ -78,12 +69,6 @@ TEST_F("Can explicitly assign new document ID to reference", Fixture) { EXPECT_EQUAL(f.refType, *fv.getDataType()); } -TEST_F("Assigning explicit document ID clears changed-flag", Fixture) { - ReferenceFieldValue fv(f.refType); - fv.setDeserializedDocumentId(DocumentId("id:ns:foo::yoshi-eggs")); - EXPECT_FALSE(fv.hasChanged()); -} - TEST_F("Exception is thrown if explicitly assigned doc ID does not have same type as reference target type", Fixture) { ReferenceFieldValue fv(f.refType); @@ -104,19 +89,6 @@ TEST_F("assign()ing another reference field value assigns doc ID and type", Fixt EXPECT_EQUAL(src.getDataType(), dest.getDataType()); } -// Different FieldValue subclasses actually disagree on whether this should be -// the case, e.g. LiteralFieldValue and TensorFieldValue. We go with the -// latter's approach, as that should be the most conservative one. -TEST_F("assign() marks assignee as changed", Fixture) { - ReferenceFieldValue src(f.refType, DocumentId("id:ns:foo::yoshi")); - ReferenceFieldValue dest(f.refType); - - dest.setDeserializedDocumentId(DocumentId("id:ns:foo::yoshi-eggs")); - EXPECT_FALSE(dest.hasChanged()); - - dest.assign(src); - EXPECT_TRUE(dest.hasChanged()); -} TEST_F("clone()ing creates new instance with same ID and type", Fixture) { ReferenceFieldValue src(f.refType, DocumentId("id:ns:foo::yoshi")); @@ -126,7 +98,6 @@ TEST_F("clone()ing creates new instance with same ID and type", Fixture) { ASSERT_TRUE(cloned->hasValidDocumentId()); EXPECT_EQUAL(src.getDocumentId(), cloned->getDocumentId()); EXPECT_EQUAL(src.getDataType(), cloned->getDataType()); - EXPECT_TRUE(cloned->hasChanged()); } TEST_F("Can clone() value without document ID", Fixture) { @@ -136,7 +107,6 @@ TEST_F("Can clone() value without document ID", Fixture) { ASSERT_TRUE(cloned); EXPECT_FALSE(cloned->hasValidDocumentId()); EXPECT_EQUAL(src.getDataType(), cloned->getDataType()); - EXPECT_TRUE(cloned->hasChanged()); } TEST_F("compare() orders first on type ID, then on document ID", Fixture) { diff --git a/document/src/tests/serialization/vespadocumentserializer_test.cpp b/document/src/tests/serialization/vespadocumentserializer_test.cpp index 33e8c521e09..3db001e1732 100644 --- a/document/src/tests/serialization/vespadocumentserializer_test.cpp +++ b/document/src/tests/serialization/vespadocumentserializer_test.cpp @@ -986,22 +986,6 @@ TEST_F("ReferenceFieldValue with ID can be roundtrip serialized", RefFixture) { serializeAndDeserialize(ref_with_id, stream, f.fixed_repo); } -TEST_F("Empty ReferenceFieldValue has changed-flag cleared after deserialization", RefFixture) { - ReferenceFieldValue src(f.ref_type()); - ReferenceFieldValue dest(f.ref_type()); - f.roundtrip_serialize(src, dest); - - EXPECT_FALSE(dest.hasChanged()); -} - -TEST_F("ReferenceFieldValue with ID has changed-flag cleared after deserialization", RefFixture) { - ReferenceFieldValue src(f.ref_type(), DocumentId("id:ns:" + doc_name + "::foo")); - ReferenceFieldValue dest(f.ref_type()); - f.roundtrip_serialize(src, dest); - - EXPECT_FALSE(dest.hasChanged()); -} - TEST_F("Empty ReferenceFieldValue serialization matches Java", RefFixture) { ReferenceFieldValue value(f.ref_type()); f.verify_cross_language_serialization("empty_reference", value); diff --git a/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.cpp b/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.cpp index d63212e402b..130e68a9f54 100644 --- a/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.cpp @@ -29,8 +29,4 @@ void AnnotationReferenceFieldValue::printXml(XmlOutputStream &out) const { out << _annotation_index; } -bool AnnotationReferenceFieldValue::hasChanged() const { - return false; -} - } // namespace document diff --git a/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.h b/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.h index 0651e429878..732be94c5c8 100644 --- a/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.h +++ b/document/src/vespa/document/fieldvalue/annotationreferencefieldvalue.h @@ -32,7 +32,6 @@ public: AnnotationReferenceFieldValue *clone() const override; const DataType *getDataType() const override { return _type; } void printXml(XmlOutputStream &out) const override; - bool hasChanged() const override; }; } // namespace document diff --git a/document/src/vespa/document/fieldvalue/arrayfieldvalue.cpp b/document/src/vespa/document/fieldvalue/arrayfieldvalue.cpp index 457b510e78a..51bc42d2cee 100644 --- a/document/src/vespa/document/fieldvalue/arrayfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/arrayfieldvalue.cpp @@ -168,15 +168,6 @@ ArrayFieldValue::print(std::ostream& out, bool verbose, out << "\n" << indent << ")"; } -bool -ArrayFieldValue::hasChanged() const -{ - for (uint32_t i=0, n=_array->size(); i<n; ++i) { - if (array()[i].hasChanged()) return true; - } - return false; -} - fieldvalue::ModificationStatus ArrayFieldValue::iterateSubset(int startPos, int endPos, vespalib::stringref variable, diff --git a/document/src/vespa/document/fieldvalue/arrayfieldvalue.h b/document/src/vespa/document/fieldvalue/arrayfieldvalue.h index af14c689be3..ae6aea47c2c 100644 --- a/document/src/vespa/document/fieldvalue/arrayfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/arrayfieldvalue.h @@ -68,7 +68,6 @@ public: int compare(const FieldValue&) const override; void printXml(XmlOutputStream& out) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; - bool hasChanged() const override; void swap(ArrayFieldValue & other) { _array.swap(other._array); } // Iterator functionality diff --git a/document/src/vespa/document/fieldvalue/boolfieldvalue.cpp b/document/src/vespa/document/fieldvalue/boolfieldvalue.cpp index a249f6e2a68..bbd40623267 100644 --- a/document/src/vespa/document/fieldvalue/boolfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/boolfieldvalue.cpp @@ -10,7 +10,7 @@ using namespace vespalib::xml; namespace document { BoolFieldValue::BoolFieldValue(bool value) - : FieldValue(Type::BOOL), _value(value), _altered(false) { + : FieldValue(Type::BOOL), _value(value) { } BoolFieldValue::~BoolFieldValue() = default; @@ -21,7 +21,6 @@ BoolFieldValue::assign(const FieldValue &rhs) { operator=(static_cast<const BoolFieldValue &>(rhs)); return *this; } else { - _altered = true; return FieldValue::assign(rhs); } } @@ -49,11 +48,6 @@ BoolFieldValue::getDataType() const { return DataType::BOOL; } -bool -BoolFieldValue::hasChanged() const { - return _altered; -} - FieldValue * BoolFieldValue::clone() const { return new BoolFieldValue(*this); diff --git a/document/src/vespa/document/fieldvalue/boolfieldvalue.h b/document/src/vespa/document/fieldvalue/boolfieldvalue.h index e4431b36a6d..0265205665d 100644 --- a/document/src/vespa/document/fieldvalue/boolfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/boolfieldvalue.h @@ -11,7 +11,6 @@ namespace document { **/ class BoolFieldValue final : public FieldValue { bool _value; - bool _altered; public: BoolFieldValue(bool value=false); @@ -27,7 +26,6 @@ public: void print(std::ostream &out, bool verbose, const std::string &indent) const override; const DataType *getDataType() const override; - bool hasChanged() const override; bool getValue() const { return _value; } void setValue(bool v) { _value = v; } diff --git a/document/src/vespa/document/fieldvalue/document.cpp b/document/src/vespa/document/fieldvalue/document.cpp index 04c328cad5c..0cd1faa5d62 100644 --- a/document/src/vespa/document/fieldvalue/document.cpp +++ b/document/src/vespa/document/fieldvalue/document.cpp @@ -171,12 +171,6 @@ Document::setFieldValue(const Field& field, FieldValue::UP data) _fields.setFieldValue(field, std::move(data)); } -bool -Document::hasChanged() const -{ - return _fields.hasChanged(); -} - FieldValue& Document::assign(const FieldValue& value) { diff --git a/document/src/vespa/document/fieldvalue/document.h b/document/src/vespa/document/fieldvalue/document.h index 30a168ca5b0..610b2fcbd1b 100644 --- a/document/src/vespa/document/fieldvalue/document.h +++ b/document/src/vespa/document/fieldvalue/document.h @@ -86,8 +86,6 @@ public: void clear() override; - bool hasChanged() const override; - // FieldValue implementation. FieldValue& assign(const FieldValue&) override; int compare(const FieldValue& other) const override; diff --git a/document/src/vespa/document/fieldvalue/fieldvalue.h b/document/src/vespa/document/fieldvalue/fieldvalue.h index 4f589913a4d..a45e42539ee 100644 --- a/document/src/vespa/document/fieldvalue/fieldvalue.h +++ b/document/src/vespa/document/fieldvalue/fieldvalue.h @@ -79,13 +79,6 @@ public: */ virtual int fastCompare(const FieldValue& other) const; - /** - * Returns true if this object have been altered since last - * serialization/deserialization. If hasChanged() is false, then cached - * information from last serialization effort is still valid. - */ - virtual bool hasChanged() const = 0; - /** Cloneable implementation */ virtual FieldValue* clone() const = 0; diff --git a/document/src/vespa/document/fieldvalue/literalfieldvalue.cpp b/document/src/vespa/document/fieldvalue/literalfieldvalue.cpp index 2ba382f5ef8..02ad759fb8c 100644 --- a/document/src/vespa/document/fieldvalue/literalfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/literalfieldvalue.cpp @@ -11,8 +11,7 @@ namespace document { LiteralFieldValueB::LiteralFieldValueB(Type type) : FieldValue(type), _value(), - _backing(), - _altered(true) + _backing() { _value = _backing; } @@ -22,8 +21,7 @@ LiteralFieldValueB::~LiteralFieldValueB() = default; LiteralFieldValueB::LiteralFieldValueB(const LiteralFieldValueB& other) : FieldValue(other), _value(), - _backing(other.getValueRef()), - _altered(other._altered) + _backing(other.getValueRef()) { _value = _backing; } @@ -31,8 +29,7 @@ LiteralFieldValueB::LiteralFieldValueB(const LiteralFieldValueB& other) LiteralFieldValueB::LiteralFieldValueB(Type type, const stringref & value) : FieldValue(type), _value(), - _backing(value), - _altered(true) + _backing(value) { _value = _backing; } @@ -43,7 +40,6 @@ LiteralFieldValueB::operator=(const LiteralFieldValueB& other) FieldValue::operator=(other); _backing = other.getValueRef(); _value = _backing; - _altered = other._altered; return *this; } diff --git a/document/src/vespa/document/fieldvalue/literalfieldvalue.h b/document/src/vespa/document/fieldvalue/literalfieldvalue.h index b8ce255771e..89f2f6afd74 100644 --- a/document/src/vespa/document/fieldvalue/literalfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/literalfieldvalue.h @@ -45,13 +45,11 @@ public: void setValueRef(stringref value) { _value = value; - _altered = true; } void setValue(stringref value) { _backing = value; _value = _backing; - _altered = true; } size_t hash() const override final { return vespalib::hashValue(_value.data(), _value.size()); } void setValue(const char* val, size_t size) { setValue(stringref(val, size)); } @@ -65,7 +63,6 @@ public: void printXml(XmlOutputStream& out) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; FieldValue& assign(const FieldValue&) override; - bool hasChanged() const override{ return _altered; } FieldValue& operator=(vespalib::stringref) override; protected: @@ -77,7 +74,6 @@ protected: } mutable stringref _value; mutable string _backing; // Lazily set when needed - mutable bool _altered; // Set if altered after deserialization }; template<typename SubClass, int dataType> diff --git a/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp b/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp index 3fc8a3129f0..33ce4357f03 100644 --- a/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp @@ -79,8 +79,7 @@ MapFieldValue::MapFieldValue(const DataType &mapType) _keys(static_cast<IArray *>(createArray(getMapType().getKeyType()).release())), _values(static_cast<IArray *>(createArray(getMapType().getValueType()).release())), _present(), - _lookupMap(), - _altered(true) + _lookupMap() { } @@ -93,8 +92,7 @@ MapFieldValue::MapFieldValue(const MapFieldValue & rhs) : _keys(rhs._keys ? rhs._keys->clone() : nullptr), _values(rhs._values ? rhs._values->clone() : nullptr), _present(rhs._present), - _lookupMap(), - _altered(rhs._altered) + _lookupMap() { } @@ -116,7 +114,6 @@ MapFieldValue::swap(MapFieldValue & rhs) { std::swap(_values, rhs._values); std::swap(_present, rhs._present); std::swap(_lookupMap, rhs._lookupMap); - std::swap(_altered, rhs._altered); } void MapFieldValue::verifyKey(const FieldValue & fv) const @@ -144,7 +141,6 @@ MapFieldValue::insertVerify(const FieldValue& key, const FieldValue& value) bool result(false); if (found != end()) { if (!(value == *found->second)) { - _altered = true; found->second->assign(value); } } else { @@ -164,8 +160,6 @@ MapFieldValue::push_back(const FieldValue& key, const FieldValue& value) if (_lookupMap) { _lookupMap->insert(_present.size() - 1); } - - _altered = true; } @@ -179,7 +173,6 @@ MapFieldValue::push_back(FieldValue::UP key, FieldValue::UP value) if (_lookupMap) { _lookupMap->insert(_present.size() - 1); } - _altered = true; } bool @@ -253,7 +246,6 @@ MapFieldValue::erase(const FieldValue& key) _count--; _present[found.offset()] = false; _lookupMap->erase(found.offset()); - _altered = true; } return result; } @@ -329,13 +321,6 @@ MapFieldValue::printXml(XmlOutputStream& xos) const } } -bool -MapFieldValue::hasChanged() const -{ - // Keys are not allowed to change in a map, so the keys should not be - // referred to externally, and should thus not need to be checked. - return _altered; -} const DataType * MapFieldValue::getDataType() const { return _type; diff --git a/document/src/vespa/document/fieldvalue/mapfieldvalue.h b/document/src/vespa/document/fieldvalue/mapfieldvalue.h index 5645bfbe176..551543684f0 100644 --- a/document/src/vespa/document/fieldvalue/mapfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/mapfieldvalue.h @@ -134,7 +134,6 @@ public: MapFieldValue* clone() const override { return new MapFieldValue(*this); } int compare(const FieldValue&) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; - bool hasChanged() const override; const DataType *getDataType() const override; void printXml(XmlOutputStream& out) const override; diff --git a/document/src/vespa/document/fieldvalue/numericfieldvalue.h b/document/src/vespa/document/fieldvalue/numericfieldvalue.h index 91c6826d174..a19456f0e4a 100644 --- a/document/src/vespa/document/fieldvalue/numericfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/numericfieldvalue.h @@ -26,10 +26,8 @@ protected: template<typename Number> class NumericFieldValue : public NumericFieldValueBase { protected: - explicit NumericFieldValue(Type type, Number value=0) : NumericFieldValueBase(type), _value(value), _altered(false) { } + explicit NumericFieldValue(Type type, Number value=0) : NumericFieldValueBase(type), _value(value) { } Number _value; - bool _altered; - public: typedef Number value_type; @@ -51,7 +49,6 @@ public: vespalib::string getAsString() const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; - bool hasChanged() const override final { return _altered; } }; extern template class NumericFieldValue<float>; diff --git a/document/src/vespa/document/fieldvalue/numericfieldvalue.hpp b/document/src/vespa/document/fieldvalue/numericfieldvalue.hpp index c9f25d67d59..0c214a97aec 100644 --- a/document/src/vespa/document/fieldvalue/numericfieldvalue.hpp +++ b/document/src/vespa/document/fieldvalue/numericfieldvalue.hpp @@ -17,22 +17,21 @@ template<typename Number> FieldValue& NumericFieldValue<Number>::assign(const FieldValue& value) { - if (value.type() == FieldValue::Type::BYTE) { + if (value.isA(FieldValue::Type::BYTE)) { _value = static_cast<Number>(value.getAsByte()); - } else if (value.type() == FieldValue::Type::SHORT) { + } else if (value.isA(FieldValue::Type::SHORT)) { _value = static_cast<Number>(value.getAsInt()); - } else if (value.type() == FieldValue::Type::INT) { + } else if (value.isA(FieldValue::Type::INT)) { _value = static_cast<Number>(value.getAsInt()); - } else if (value.type() == FieldValue::Type::LONG) { + } else if (value.isA(FieldValue::Type::LONG)) { _value = static_cast<Number>(value.getAsLong()); - } else if (value.type() == FieldValue::Type::FLOAT) { + } else if (value.isA(FieldValue::Type::FLOAT)) { _value = static_cast<Number>(value.getAsFloat()); - } else if (value.type() == FieldValue::Type::DOUBLE) { + } else if (value.isA(FieldValue::Type::DOUBLE)) { _value = static_cast<Number>(value.getAsDouble()); } else { return FieldValue::assign(value); } - _altered = true; return *this; } @@ -91,7 +90,6 @@ NumericFieldValue<Number>::operator=(vespalib::stringref value) // Allow numbers to be specified in range max signed to max // unsigned. These become negative numbers. _value = static_cast<Number>(val); - _altered = true; return *this; } } @@ -118,7 +116,6 @@ NumericFieldValue<Number>::operator=(vespalib::stringref value) } } } - _altered = true; return *this; } diff --git a/document/src/vespa/document/fieldvalue/predicatefieldvalue.cpp b/document/src/vespa/document/fieldvalue/predicatefieldvalue.cpp index 05a8abc606c..b474db2ab6e 100644 --- a/document/src/vespa/document/fieldvalue/predicatefieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/predicatefieldvalue.cpp @@ -16,20 +16,17 @@ namespace document { PredicateFieldValue::PredicateFieldValue() : FieldValue(Type::PREDICATE), - _slime(std::make_unique<Slime>()), - _altered(false) { -} + _slime(std::make_unique<Slime>()) +{ } PredicateFieldValue::PredicateFieldValue(vespalib::Slime::UP s) : FieldValue(Type::PREDICATE), - _slime(std::move(s)), - _altered(false) + _slime(std::move(s)) { } PredicateFieldValue::PredicateFieldValue(const PredicateFieldValue &rhs) : FieldValue(rhs), - _slime(new Slime), - _altered(rhs._altered) + _slime(new Slime) { inject(rhs._slime->get(), SlimeInserter(*_slime)); } @@ -42,7 +39,6 @@ PredicateFieldValue::assign(const FieldValue &rhs) { operator=(static_cast<const PredicateFieldValue &>(rhs)); } else { _slime.reset(); - _altered = true; } return *this; } @@ -52,7 +48,6 @@ PredicateFieldValue::operator=(const PredicateFieldValue &rhs) { _slime = std::make_unique<Slime>(); inject(rhs._slime->get(), SlimeInserter(*_slime)); - _altered = true; return *this; } @@ -79,11 +74,6 @@ PredicateFieldValue::getDataType() const { return DataType::PREDICATE; } -bool -PredicateFieldValue::hasChanged() const { - return _altered; -} - FieldValue * PredicateFieldValue::clone() const { return new PredicateFieldValue(*this); diff --git a/document/src/vespa/document/fieldvalue/predicatefieldvalue.h b/document/src/vespa/document/fieldvalue/predicatefieldvalue.h index 585722e669a..c99a82a59e2 100644 --- a/document/src/vespa/document/fieldvalue/predicatefieldvalue.h +++ b/document/src/vespa/document/fieldvalue/predicatefieldvalue.h @@ -11,7 +11,6 @@ namespace document { class PredicateFieldValue final : public FieldValue { std::unique_ptr<vespalib::Slime> _slime; - bool _altered; PredicateFieldValue & operator=(const PredicateFieldValue &rhs); public: @@ -33,7 +32,6 @@ public: void print(std::ostream &out, bool verbose, const std::string &indent) const override; const DataType *getDataType() const override; - bool hasChanged() const override; const vespalib::Slime &getSlime() const { return *_slime; } diff --git a/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp b/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp index d4e02d2da07..cade633fb00 100644 --- a/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/referencefieldvalue.cpp @@ -14,24 +14,21 @@ namespace document { ReferenceFieldValue::ReferenceFieldValue() : FieldValue(Type::REFERENCE), _dataType(nullptr), - _documentId(), - _altered(true) + _documentId() { } ReferenceFieldValue::ReferenceFieldValue(const ReferenceDataType& dataType) : FieldValue(Type::REFERENCE), _dataType(&dataType), - _documentId(), - _altered(true) + _documentId() { } ReferenceFieldValue::ReferenceFieldValue(const ReferenceDataType& dataType, const DocumentId& documentId) : FieldValue(Type::REFERENCE), _dataType(&dataType), - _documentId(documentId), - _altered(true) + _documentId(documentId) { requireIdOfMatchingType(_documentId, _dataType->getTargetType()); } @@ -66,7 +63,6 @@ FieldValue& ReferenceFieldValue::assign(const FieldValue& rhs) { } _documentId = refValueRhs->_documentId; _dataType = refValueRhs->_dataType; - _altered = true; return *this; } @@ -76,7 +72,6 @@ void ReferenceFieldValue::setDeserializedDocumentId(const DocumentId& id) { _documentId = id; // Pre-cache GID to ensure it's not attempted lazily initialized later in a racing manner. (void) _documentId.getGlobalId(); - _altered = false; } ReferenceFieldValue* ReferenceFieldValue::clone() const { @@ -108,10 +103,6 @@ void ReferenceFieldValue::print(std::ostream& os, bool verbose, const std::strin os << indent << "ReferenceFieldValue(" << *_dataType << ", DocumentId(" << _documentId << "))"; } -bool ReferenceFieldValue::hasChanged() const { - return _altered; -} - void ReferenceFieldValue::accept(FieldValueVisitor& visitor) { visitor.visit(*this); } diff --git a/document/src/vespa/document/fieldvalue/referencefieldvalue.h b/document/src/vespa/document/fieldvalue/referencefieldvalue.h index 41fb9a6665e..c8adb49102a 100644 --- a/document/src/vespa/document/fieldvalue/referencefieldvalue.h +++ b/document/src/vespa/document/fieldvalue/referencefieldvalue.h @@ -26,7 +26,6 @@ class ReferenceFieldValue final : public FieldValue { const ReferenceDataType* _dataType; // TODO wrap in std::optional once available. DocumentId _documentId; - bool _altered; public: // Empty constructor required for Identifiable. ReferenceFieldValue(); @@ -53,9 +52,6 @@ public: // Should only be called by deserializer code, as it will clear hasChanged. // `id` must be a valid document ID and cannot be empty. void setDeserializedDocumentId(const DocumentId& id); - void clearChanged() { - _altered = false; - } const DataType* getDataType() const override { return _dataType; } FieldValue& assign(const FieldValue&) override; @@ -63,7 +59,6 @@ public: int compare(const FieldValue&) const override; void printXml(XmlOutputStream&) const override { /* Not implemented */ } void print(std::ostream&, bool, const std::string&) const override; - bool hasChanged() const override; void accept(FieldValueVisitor&) override; void accept(ConstFieldValueVisitor&) const override; private: diff --git a/document/src/vespa/document/fieldvalue/structfieldvalue.h b/document/src/vespa/document/fieldvalue/structfieldvalue.h index c50d8248307..564c48273e6 100644 --- a/document/src/vespa/document/fieldvalue/structfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/structfieldvalue.h @@ -76,7 +76,12 @@ public: bool empty() const override; - bool hasChanged() const override { return _hasChanged; } + /** + * Returns true if this object have been altered since last + * serialization/deserialization. If hasChanged() is false, then cached + * information from last serialization effort is still valid. + */ + bool hasChanged() const { return _hasChanged; } uint32_t calculateChecksum() const; diff --git a/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp b/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp index 94f2ddc994f..d9a56f9fa56 100644 --- a/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/tensorfieldvalue.cpp @@ -38,16 +38,14 @@ TensorFieldValue::TensorFieldValue() TensorFieldValue::TensorFieldValue(const TensorDataType &dataType) : FieldValue(Type::TENSOR), _dataType(dataType), - _tensor(), - _altered(true) + _tensor() { } TensorFieldValue::TensorFieldValue(const TensorFieldValue &rhs) : FieldValue(Type::TENSOR), _dataType(rhs._dataType), - _tensor(), - _altered(true) + _tensor() { if (rhs._tensor) { _tensor = FastValueBuilderFactory::get().copy(*rhs._tensor); @@ -58,8 +56,7 @@ TensorFieldValue::TensorFieldValue(const TensorFieldValue &rhs) TensorFieldValue::TensorFieldValue(TensorFieldValue &&rhs) : FieldValue(Type::TENSOR), _dataType(rhs._dataType), - _tensor(), - _altered(true) + _tensor() { _tensor = std::move(rhs._tensor); } @@ -81,7 +78,6 @@ TensorFieldValue::operator=(const TensorFieldValue &rhs) } else { _tensor.reset(); } - _altered = true; } else { throw WrongTensorTypeException(makeWrongTensorTypeMsg(_dataType.getTensorType(), rhs._tensor->type()), VESPA_STRLOC); } @@ -95,7 +91,6 @@ TensorFieldValue::operator=(std::unique_ptr<vespalib::eval::Value> rhs) { if (!rhs || _dataType.isAssignableType(rhs->type())) { _tensor = std::move(rhs); - _altered = true; } else { throw WrongTensorTypeException(makeWrongTensorTypeMsg(_dataType.getTensorType(), rhs->type()), VESPA_STRLOC); } @@ -133,14 +128,6 @@ TensorFieldValue::getDataType() const return &_dataType; } - -bool -TensorFieldValue::hasChanged() const -{ - return _altered; -} - - TensorFieldValue* TensorFieldValue::clone() const { @@ -188,7 +175,6 @@ TensorFieldValue::assignDeserialized(std::unique_ptr<vespalib::eval::Value> rhs) { if (!rhs || _dataType.isAssignableType(rhs->type())) { _tensor = std::move(rhs); - _altered = false; // Serialized form already exists } else { throw WrongTensorTypeException(makeWrongTensorTypeMsg(_dataType.getTensorType(), rhs->type()), VESPA_STRLOC); } diff --git a/document/src/vespa/document/fieldvalue/tensorfieldvalue.h b/document/src/vespa/document/fieldvalue/tensorfieldvalue.h index 058e2b86297..52b27346ff8 100644 --- a/document/src/vespa/document/fieldvalue/tensorfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/tensorfieldvalue.h @@ -17,7 +17,6 @@ class TensorFieldValue final : public FieldValue { private: const TensorDataType &_dataType; std::unique_ptr<vespalib::eval::Value> _tensor; - bool _altered; public: TensorFieldValue(); explicit TensorFieldValue(const TensorDataType &dataType); @@ -33,7 +32,6 @@ public: void accept(FieldValueVisitor &visitor) override; void accept(ConstFieldValueVisitor &visitor) const override; const DataType *getDataType() const override; - bool hasChanged() const override; TensorFieldValue* clone() const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; void printXml(XmlOutputStream& out) const override; diff --git a/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.cpp b/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.cpp index d6d092a8a99..fd26deab15a 100644 --- a/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.cpp +++ b/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.cpp @@ -32,8 +32,7 @@ const DataType &getKeyType(const DataType &type) { WeightedSetFieldValue::WeightedSetFieldValue(const DataType &type) : CollectionFieldValue(Type::WSET, type), _map_type(std::make_shared<MapDataType>(getKeyType(type), *DataType::INT)), - _map(*_map_type), - _altered(true) + _map(*_map_type) { } WeightedSetFieldValue::WeightedSetFieldValue(const WeightedSetFieldValue &) = default; @@ -52,7 +51,6 @@ WeightedSetFieldValue::add(const FieldValue& key, int weight) { verifyKey(key); const WeightedSetDataType & wdt(static_cast<const WeightedSetDataType&>(*_type)); - _altered = true; if (wdt.removeIfZero() && (weight == 0)) { _map.erase(key); return false; @@ -64,14 +62,12 @@ bool WeightedSetFieldValue::addIgnoreZeroWeight(const FieldValue& key, int32_t weight) { verifyKey(key); - _altered = true; return _map.insert(FieldValue::UP(key.clone()), IntFieldValue::make(weight)); } void WeightedSetFieldValue::push_back(FieldValue::UP key, int weight) { - _altered = true; _map.push_back(std::move(key), IntFieldValue::make(weight)); } @@ -101,7 +97,6 @@ WeightedSetFieldValue::increment(const FieldValue& key, int val) _map.erase(key); } } - _altered = true; } int32_t @@ -123,7 +118,6 @@ bool WeightedSetFieldValue::removeValue(const FieldValue& key) { bool result = _map.erase(key); - _altered |= result; return result; } @@ -177,14 +171,6 @@ WeightedSetFieldValue::print(std::ostream& out, bool verbose, const std::string& out << ")"; } -bool -WeightedSetFieldValue::hasChanged() const -{ - // Keys are not allowed to change in a map, so the keys should not be - // referred to externally, and should thus not need to be checked. - return _altered; -} - WeightedSetFieldValue::const_iterator WeightedSetFieldValue::find(const FieldValue& key) const { diff --git a/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.h b/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.h index 186c29d71e1..b2183efef42 100644 --- a/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/weightedsetfieldvalue.h @@ -26,7 +26,6 @@ public: private: std::shared_ptr<const MapDataType> _map_type; WeightedFieldValueMap _map; - bool _altered; void verifyKey(const FieldValue & key); bool addValue(const FieldValue& fval) override { return add(fval, 1); } @@ -78,7 +77,6 @@ public: int compare(const FieldValue&) const override; void printXml(XmlOutputStream& out) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; - bool hasChanged() const override; // Implements iterating through internal content. typedef WeightedFieldValueMap::const_iterator const_iterator; diff --git a/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp b/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp index 747dfbbcee6..55ea4988fc4 100644 --- a/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp +++ b/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp @@ -447,8 +447,6 @@ VespaDocumentDeserializer::read(ReferenceFieldValue& value) { DocumentId id; read(id); value.setDeserializedDocumentId(id); - } else { - value.clearChanged(); } } |