From 751973b6448dc29d064e999791ae365e4659854a Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Tue, 18 Apr 2023 14:01:27 +0200 Subject: Use default value ref from enum store to handle CLEARDOC. --- .../src/vespa/searchlib/attribute/enumattribute.h | 1 - .../vespa/searchlib/attribute/enumattribute.hpp | 9 --------- .../searchlib/attribute/singleenumattribute.h | 4 ++-- .../searchlib/attribute/singleenumattribute.hpp | 22 ++++++++-------------- .../attribute/singlenumericenumattribute.h | 2 +- .../attribute/singlenumericenumattribute.hpp | 6 +++--- .../attribute/singlenumericpostattribute.hpp | 8 +------- .../attribute/singlestringpostattribute.hpp | 7 +------ .../src/vespa/searchlib/attribute/stringbase.h | 2 +- 9 files changed, 17 insertions(+), 44 deletions(-) diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.h b/searchlib/src/vespa/searchlib/attribute/enumattribute.h index 773e5451703..4753dbe65f9 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.h @@ -56,7 +56,6 @@ protected: virtual void considerAttributeChange(const Change & c, EnumStoreBatchUpdater & inserter) = 0; vespalib::MemoryUsage getEnumStoreValuesMemoryUsage() const override; void populate_address_space_usage(AddressSpaceUsage& usage) const override; - void cache_change_data_entry_ref(const Change& c) const; public: EnumAttribute(const vespalib::string & baseFileName, const AttributeVector::Config & cfg); ~EnumAttribute(); diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp index f0f518f64f7..66d555df3cb 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp @@ -86,13 +86,4 @@ EnumAttribute::populate_address_space_usage(AddressSpaceUsage& usage) const usage.set(AddressSpaceComponents::enum_store, _enumStore.get_values_address_space_usage()); } -template -void -EnumAttribute::cache_change_data_entry_ref(const Change& c) const -{ - EnumIndex new_idx; - _enumStore.find_index(c._data.raw(), new_idx); - c.set_entry_ref(new_idx.ref()); -} - } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h index aac9a7b5416..7f36238ec6a 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h @@ -67,14 +67,14 @@ protected: void considerAttributeChange(const Change & c, EnumStoreBatchUpdater & inserter) override; // implemented by single value numeric enum attribute. - virtual void considerUpdateAttributeChange(const Change & c) { (void) c; } + virtual void considerUpdateAttributeChange(DocId, const Change&) { } virtual void considerArithmeticAttributeChange(const Change & c, EnumStoreBatchUpdater & inserter) { (void) c; (void) inserter; } virtual void applyValueChanges(EnumStoreBatchUpdater& updater) ; virtual void applyArithmeticValueChange(const Change& c, EnumStoreBatchUpdater& updater) { (void) c; (void) updater; } - void updateEnumRefCounts(const Change& c, EnumIndex newIdx, EnumIndex oldIdx, EnumStoreBatchUpdater& updater); + void updateEnumRefCounts(DocId doc, EnumIndex newIdx, EnumIndex oldIdx, EnumStoreBatchUpdater& updater); virtual void freezeEnumDictionary() { this->getEnumStore().freeze_dictionary(); diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp index f4f2b777abd..95976609940 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp @@ -146,7 +146,7 @@ SingleValueEnumAttribute::considerUpdateAttributeChange(const Change & c, Enu } else { c.set_entry_ref(idx.ref()); } - considerUpdateAttributeChange(c); // for numeric + considerUpdateAttributeChange(c._doc, c); // for numeric } template @@ -158,9 +158,7 @@ SingleValueEnumAttribute::considerAttributeChange(const Change & c, EnumStore } else if (c._type >= ChangeBase::ADD && c._type <= ChangeBase::DIV) { considerArithmeticAttributeChange(c, inserter); // for numeric } else if (c._type == ChangeBase::CLEARDOC) { - Change clearDoc(this->_defaultValue); - clearDoc._doc = c._doc; - considerUpdateAttributeChange(clearDoc, inserter); + considerUpdateAttributeChange(c._doc, this->_defaultValue); } } @@ -175,7 +173,7 @@ SingleValueEnumAttribute::applyUpdateValueChange(const Change& c, EnumStoreBa } else { this->_enumStore.find_index(c._data.raw(), newIdx); } - updateEnumRefCounts(c, newIdx, oldIdx, updater); + updateEnumRefCounts(c._doc, newIdx, oldIdx, updater); } template @@ -183,30 +181,26 @@ void SingleValueEnumAttribute::applyValueChanges(EnumStoreBatchUpdater& updater) { ValueModifier valueGuard(this->getValueModifier()); - // This avoids searching for the defaultValue in the enum store for each CLEARDOC in the change vector. - this->cache_change_data_entry_ref(this->_defaultValue); for (const auto& change : this->_changes.getInsertOrder()) { if (change._type == ChangeBase::UPDATE) { applyUpdateValueChange(change, updater); } else if (change._type >= ChangeBase::ADD && change._type <= ChangeBase::DIV) { applyArithmeticValueChange(change, updater); } else if (change._type == ChangeBase::CLEARDOC) { - Change clearDoc(this->_defaultValue); - clearDoc._doc = change._doc; - applyUpdateValueChange(clearDoc, updater); + EnumIndex oldIdx = _enumIndices[change._doc].load_relaxed(); + EnumIndex newIdx = this->_enumStore.get_default_value_ref().load_relaxed(); + updateEnumRefCounts(change._doc, newIdx, oldIdx, updater); } } - // We must clear the cached entry ref as the defaultValue might be located in another data buffer on later invocations. - this->_defaultValue.clear_entry_ref(); } template void -SingleValueEnumAttribute::updateEnumRefCounts(const Change& c, EnumIndex newIdx, EnumIndex oldIdx, +SingleValueEnumAttribute::updateEnumRefCounts(DocId doc, EnumIndex newIdx, EnumIndex oldIdx, EnumStoreBatchUpdater& updater) { updater.inc_ref_count(newIdx); - _enumIndices[c._doc].store_release(newIdx); + _enumIndices[doc].store_release(newIdx); if (oldIdx.valid()) { updater.dec_ref_count(oldIdx); } diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h index 5b0e1c6131e..4eeb6ceda57 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h @@ -43,7 +43,7 @@ private: protected: // from SingleValueEnumAttribute - void considerUpdateAttributeChange(const Change & c) override; + void considerUpdateAttributeChange(DocId doc, const Change & c) override; void considerArithmeticAttributeChange(const Change & c, EnumStoreBatchUpdater & inserter) override; void applyArithmeticValueChange(const Change& c, EnumStoreBatchUpdater& updater) override; diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp index 14bf9cdc9f0..b840a0516b2 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp @@ -15,9 +15,9 @@ namespace search { template void -SingleValueNumericEnumAttribute::considerUpdateAttributeChange(const Change & c) +SingleValueNumericEnumAttribute::considerUpdateAttributeChange(DocId doc, const Change & c) { - _currDocValues[c._doc] = c._data.get(); + _currDocValues[doc] = c._data.get(); } template @@ -53,7 +53,7 @@ SingleValueNumericEnumAttribute::applyArithmeticValueChange(const Change& c, T newValue = this->template applyArithmetic(get(c._doc), c._data.getArithOperand(), c._type); this->_enumStore.find_index(newValue, newIdx); - this->updateEnumRefCounts(c, newIdx, oldIdx, updater); + this->updateEnumRefCounts(c._doc, newIdx, oldIdx, updater); } template diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp index de4a7157dae..e353d03a9e8 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp @@ -89,8 +89,6 @@ SingleValueNumericPostingAttribute::applyValueChanges(EnumStoreBatchUpdater& // used to make sure several arithmetic operations on the same document in a single commit works std::map currEnumIndices; - // This avoids searching for the defaultValue in the enum store for each CLEARDOC in the change vector. - this->cache_change_data_entry_ref(this->_defaultValue); for (const auto& change : this->_changes.getInsertOrder()) { auto enumIter = currEnumIndices.find(change._doc); EnumIndex oldIdx; @@ -111,13 +109,9 @@ SingleValueNumericPostingAttribute::applyValueChanges(EnumStoreBatchUpdater& currEnumIndices[change._doc] = newIdx; } } else if(change._type == ChangeBase::CLEARDOC) { - Change clearDoc(this->_defaultValue); - clearDoc._doc = change._doc; - applyUpdateValueChange(clearDoc, enumStore, currEnumIndices); + currEnumIndices[change._doc] = enumStore.get_default_value_ref().load_relaxed(); } } - // We must clear the cached entry ref as the defaultValue might be located in another data buffer on later invocations. - this->_defaultValue.clear_entry_ref(); makePostingChange(enumStore.get_comparator(), currEnumIndices, changePost); diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp index 1ec9b54a73b..5b5214f6d3e 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp @@ -98,8 +98,6 @@ SingleValueStringPostingAttributeT::applyValueChanges(EnumStoreBatchUpdater& // used to make sure several arithmetic operations on the same document in a single commit works std::map currEnumIndices; - // This avoids searching for the defaultValue in the enum store for each CLEARDOC in the change vector. - this->cache_change_data_entry_ref(this->_defaultValue); for (const auto& change : this->_changes.getInsertOrder()) { auto enumIter = currEnumIndices.find(change._doc); EnumIndex oldIdx; @@ -111,12 +109,9 @@ SingleValueStringPostingAttributeT::applyValueChanges(EnumStoreBatchUpdater& if (change._type == ChangeBase::UPDATE) { applyUpdateValueChange(change, enumStore, currEnumIndices); } else if (change._type == ChangeBase::CLEARDOC) { - this->_defaultValue._doc = change._doc; - applyUpdateValueChange(this->_defaultValue, enumStore, currEnumIndices); + currEnumIndices[change._doc] = enumStore.get_default_value_ref().load_relaxed(); } } - // We must clear the cached entry ref as the defaultValue might be located in another data buffer on later invocations. - this->_defaultValue.clear_entry_ref(); makePostingChange(enumStore.get_folded_comparator(), dictionary, currEnumIndices, changePost); diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h index 5c6bf3c6b6a..98a3316947b 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.h +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h @@ -62,7 +62,7 @@ protected: using ChangeVector = ChangeVectorT; using EnumEntryType = const char*; ChangeVector _changes; - Change _defaultValue; + const Change _defaultValue; bool onLoad(vespalib::Executor *executor) override; bool onLoadEnumerated(ReaderBase &attrReader); -- cgit v1.2.3