From 0be32803cb962fb19bc84336508a7e7029dfb3d3 Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Tue, 29 Mar 2022 12:18:22 +0200 Subject: Remove const type qualifier from member functions that should only be called from writer in RcuVector, MultiValueMappingBase and CondensedBitVector. --- .../attribute/multi_value_mapping_base.cpp | 2 +- .../searchlib/attribute/multi_value_mapping_base.h | 24 ++++++++++++++++++---- .../searchlib/attribute/reference_attribute.cpp | 2 +- .../searchlib/attribute/singleenumattribute.cpp | 2 +- .../vespa/searchlib/common/condensedbitvectors.cpp | 12 +++++++++-- .../vespa/searchlib/common/condensedbitvectors.h | 12 +++++++++-- .../src/vespa/searchlib/docstore/logdatastore.cpp | 6 ++++-- .../src/vespa/searchlib/predicate/simple_index.h | 4 ++-- .../vespa/searchlib/tensor/hnsw_index_saver.cpp | 4 ++-- .../vespa/searchlib/tensor/tensor_attribute.cpp | 2 +- 10 files changed, 52 insertions(+), 18 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp index eebef46284b..0bcff91007d 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp @@ -24,7 +24,7 @@ MultiValueMappingBase::~MultiValueMappingBase() = default; MultiValueMappingBase::RefCopyVector MultiValueMappingBase::getRefCopy(uint32_t size) const { - assert(size <= _indices.size()); + assert(size <= _indices.get_size()); // Called from writer only auto* indices = &_indices.get_elem_ref(0); // Called from writer only RefCopyVector result; result.reserve(size); diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h index 9e42e2763e5..8fe64bd62e7 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h @@ -51,15 +51,31 @@ public: size_t getTotalValueCnt() const { return _totalValues; } RefCopyVector getRefCopy(uint32_t size) const; - bool isFull() const { return _indices.isFull(); } + /* + * isFull() should be called from writer only. + * Const type qualifier removed to prevent call from reader. + */ + bool isFull() { return _indices.isFull(); } void addDoc(uint32_t &docId); void shrink(uint32_t docidLimit); void reserve(uint32_t lidLimit); void clearDocs(uint32_t lidLow, uint32_t lidLimit, std::function clearDoc); - uint32_t size() const { return _indices.size(); } + /* + * size() should be called from writer only. + * Const type qualifier removed to prevent call from reader. + */ + uint32_t size() { return _indices.size(); } - uint32_t getNumKeys() const { return _indices.size(); } - uint32_t getCapacityKeys() const { return _indices.capacity(); } + /* + * getNumKeys() should be called from writer only. + * Const type qualifier removed to prevent call from reader. + */ + uint32_t getNumKeys() { return _indices.size(); } + /* + * getCapacityKeys() should be called from writer only. + * Const type qualifier removed to prevent call from reader. + */ + uint32_t getCapacityKeys() { return _indices.capacity(); } virtual void compactWorst(CompactionSpec compaction_spec, const CompactionStrategy& compaction_strategy) = 0; bool considerCompact(const CompactionStrategy &compactionStrategy); }; diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp index 2d8d4c4858b..4464ab02c86 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp @@ -336,7 +336,7 @@ ReferenceAttribute::getUniqueValueCount() const ReferenceAttribute::IndicesCopyVector ReferenceAttribute::getIndicesCopy(uint32_t size) const { - assert(size <= _indices.size()); + assert(size <= _indices.get_size()); // Called from writer only auto* indices = &_indices.get_elem_ref(0); // Called from writer only IndicesCopyVector result; result.reserve(size); diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp index b671a23bbe7..38ef186b717 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp @@ -40,7 +40,7 @@ SingleValueEnumAttributeBase::addDoc(bool &incGeneration) SingleValueEnumAttributeBase::EnumIndexCopyVector SingleValueEnumAttributeBase::getIndicesCopy(uint32_t size) const { - assert(size <= _enumIndices.size()); + assert(size <= _enumIndices.get_size()); // Called from writer only auto* enum_indices = &_enumIndices.get_elem_ref(0); // Called from writer only EnumIndexCopyVector result; result.reserve(size); diff --git a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp index 39986c43ac5..569e1275080 100644 --- a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp +++ b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp @@ -75,8 +75,16 @@ private: } size_t getKeyCapacity() const override { return sizeof(T)*8; } - size_t getCapacity() const override { return _v.capacity(); } - size_t getSize() const override { return _v.size(); } + /* + * getCapacity() should be called from writer only. + * Const type qualifier removed to prevent call from readers. + */ + size_t getCapacity() override { return _v.capacity(); } + /* + * getSize() should be called from writer only. + * Const type qualifier removed to prevent call from readers. + */ + size_t getSize() override { return _v.size(); } void adjustDocIdLimit(uint32_t docId) override; vespalib::RcuVectorBase _v; }; diff --git a/searchlib/src/vespa/searchlib/common/condensedbitvectors.h b/searchlib/src/vespa/searchlib/common/condensedbitvectors.h index f56890e730f..696400eb7a1 100644 --- a/searchlib/src/vespa/searchlib/common/condensedbitvectors.h +++ b/searchlib/src/vespa/searchlib/common/condensedbitvectors.h @@ -24,8 +24,16 @@ public: virtual bool get(Key key, uint32_t index) const = 0; virtual void clearIndex(uint32_t index) = 0; virtual size_t getKeyCapacity() const = 0; - virtual size_t getCapacity() const = 0; - virtual size_t getSize() const = 0; + /* + * getCapacity() should be called from writer only. + * Const type qualifier removed to prevent call from readers. + */ + virtual size_t getCapacity() = 0; + /* + * getSize() should be called from writer only. + * Const type qualifier removed to prevent call from readers. + */ + virtual size_t getSize() = 0; virtual void adjustDocIdLimit(uint32_t docId) = 0; bool hasKey(Key key) const { return key < getKeyCapacity(); } void addKey(Key key) const; diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp index a026b8b8e69..b991773c50f 100644 --- a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp +++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp @@ -1211,7 +1211,8 @@ LogDataStore::canShrinkLidSpace() const bool LogDataStore::canShrinkLidSpace(const MonitorGuard &) const { - return getDocIdLimit() < _lidInfo.size() && + // Update lock is held, allowing call to _lidInfo.get_size() + return getDocIdLimit() < _lidInfo.get_size() && _compactLidSpaceGeneration < _genHandler.getFirstUsedGeneration(); } @@ -1222,7 +1223,8 @@ LogDataStore::getEstimatedShrinkLidSpaceGain() const if (!canShrinkLidSpace(guard)) { return 0; } - return (_lidInfo.size() - getDocIdLimit()) * sizeof(uint64_t); + // Update lock is held, allowing call to _lidInfo.get_size() + return (_lidInfo.get_size() - getDocIdLimit()) * sizeof(uint64_t); } void diff --git a/searchlib/src/vespa/searchlib/predicate/simple_index.h b/searchlib/src/vespa/searchlib/predicate/simple_index.h index df3d0686e82..78805820a30 100644 --- a/searchlib/src/vespa/searchlib/predicate/simple_index.h +++ b/searchlib/src/vespa/searchlib/predicate/simple_index.h @@ -92,7 +92,7 @@ public: : _vector(&vector.acquire_elem_ref(0)), _size(size) { - assert(_size <= vector.size()); + assert(_size <= vector.get_size()); // Data race: not writer linearSeek(1); } @@ -165,7 +165,7 @@ private: bool shouldCreateVectorPosting(size_t size, double ratio) const; bool shouldRemoveVectorPosting(size_t size, double ratio) const; size_t getVectorPostingSize(const PostingVector &vector) const { - return std::min(vector.size(), + return std::min(vector.get_size() /* Data race: not writer */, static_cast(_limit_provider.getCommittedDocIdLimit())); } diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp index cc2ecd584e1..8ac24d08dcf 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp @@ -12,7 +12,7 @@ namespace { size_t count_valid_link_arrays(const HnswGraph & graph) { size_t count(0); - size_t num_nodes = graph.node_refs.size(); + size_t num_nodes = graph.node_refs.get_size(); // Called from writer only for (size_t i = 0; i < num_nodes; ++i) { auto node_ref = graph.get_node_ref(i); if (node_ref.valid()) { @@ -39,7 +39,7 @@ HnswIndexSaver::HnswIndexSaver(const HnswGraph &graph) auto entry = graph.get_entry_node(); _meta_data.entry_docid = entry.docid; _meta_data.entry_level = entry.level; - size_t num_nodes = graph.node_refs.size(); + size_t num_nodes = graph.node_refs.get_size(); // Called from writer only assert (num_nodes <= (std::numeric_limits::max() - 1)); size_t link_array_count = count_valid_link_arrays(graph); assert (link_array_count <= std::numeric_limits::max()); diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp index 89f54eacc09..c1cafd547aa 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp @@ -268,7 +268,7 @@ TensorAttribute::RefCopyVector TensorAttribute::getRefCopy() const { uint32_t size = getCommittedDocIdLimit(); - assert(size <= _refVector.size()); + assert(size <= _refVector.get_size()); // Called from writer only auto* ref_vector = &_refVector.get_elem_ref(0); // Called from writer only RefCopyVector result; result.reserve(size); -- cgit v1.2.3