diff options
39 files changed, 423 insertions, 421 deletions
diff --git a/client/go/internal/vespa/document/dispatcher.go b/client/go/internal/vespa/document/dispatcher.go index 7a19d21f278..8ddb34c8c4d 100644 --- a/client/go/internal/vespa/document/dispatcher.go +++ b/client/go/internal/vespa/document/dispatcher.go @@ -137,6 +137,7 @@ func (d *Dispatcher) processResults() { if d.shouldRetry(op, op.result) { d.enqueue(op.resetResult(), true) } else { + op.document.Reset() d.inflightWg.Done() } d.dispatchNext(op.document.Id) diff --git a/client/go/internal/vespa/document/document.go b/client/go/internal/vespa/document/document.go index 8f884b223d7..616013dc59a 100644 --- a/client/go/internal/vespa/document/document.go +++ b/client/go/internal/vespa/document/document.go @@ -8,6 +8,7 @@ import ( "math/rand" "strconv" "strings" + "sync" "time" @@ -116,6 +117,24 @@ type Document struct { Body []byte Operation Operation Create bool + + resetFunc func() +} + +func (d Document) Equal(o Document) bool { + return d.Id.Equal(o.Id) && + d.Condition == o.Condition && + bytes.Equal(d.Body, o.Body) && + d.Operation == o.Operation && + d.Create == o.Create +} + +// Reset discards the body of this document. +func (d *Document) Reset() { + d.Body = nil + if d.resetFunc != nil { + d.resetFunc() + } } // Decoder decodes documents from a JSON structure which is either an array of objects, or objects separated by newline. @@ -127,6 +146,8 @@ type Decoder struct { jsonl bool fieldsEnd int64 + + documentBuffers sync.Pool } func (d Document) String() string { @@ -212,6 +233,12 @@ func (d *Decoder) Decode() (Document, error) { return doc, err } +func (d *Decoder) buffer() *bytes.Buffer { + buf := d.documentBuffers.Get().(*bytes.Buffer) + buf.Reset() + return buf +} + func (d *Decoder) readField(name string, offset int64, doc *Document) error { readId := false switch name { @@ -258,10 +285,14 @@ func (d *Decoder) readField(name string, offset int64, doc *Document) error { } d.fieldsEnd = d.dec.InputOffset() fields := d.buf.Next(int(d.fieldsEnd - fieldsStart)) - doc.Body = make([]byte, 0, len(fieldsPrefix)+len(fields)+len(fieldsSuffix)) - doc.Body = append(doc.Body, fieldsPrefix...) - doc.Body = append(doc.Body, fields...) - doc.Body = append(doc.Body, fieldsSuffix...) + // Try to re-use buffers holding the document body. The buffer is released by document.Reset() + bodyBuf := d.buffer() + bodyBuf.Grow(len(fieldsPrefix) + len(fields) + len(fieldsSuffix)) + bodyBuf.Write(fieldsPrefix) + bodyBuf.Write(fields) + bodyBuf.Write(fieldsSuffix) + doc.Body = bodyBuf.Bytes() + doc.resetFunc = func() { d.documentBuffers.Put(bodyBuf) } } if readId { s, err := d.readString() @@ -322,6 +353,7 @@ loop: func NewDecoder(r io.Reader) *Decoder { br := bufio.NewReaderSize(r, 1<<26) d := &Decoder{} + d.documentBuffers.New = func() any { return &bytes.Buffer{} } d.dec = json.NewDecoder(io.TeeReader(br, &d.buf)) return d } diff --git a/client/go/internal/vespa/document/document_test.go b/client/go/internal/vespa/document/document_test.go index fbaa076ab9d..f9bf321f1fb 100644 --- a/client/go/internal/vespa/document/document_test.go +++ b/client/go/internal/vespa/document/document_test.go @@ -3,7 +3,6 @@ package document import ( "fmt" "io" - "reflect" "strings" "testing" "time" @@ -147,14 +146,14 @@ func feedInput(jsonl bool) string { func testDocumentDecoder(t *testing.T, jsonLike string) { t.Helper() dec := NewDecoder(strings.NewReader(jsonLike)) - want := []Document{ + docs := []Document{ {Id: mustParseId("id:ns:type::doc1"), Operation: OperationPut, Body: []byte(`{"fields":{ "foo" : "123", "bar": {"a": [1, 2, 3]}}}`)}, {Id: mustParseId("id:ns:type::doc2"), Operation: OperationPut, Condition: "foo", Body: []byte(`{"fields":{"bar": "456"}}`)}, {Id: mustParseId("id:ns:type::doc3"), Operation: OperationRemove}, {Id: mustParseId("id:ns:type::doc4"), Operation: OperationPut, Create: true, Body: []byte(`{"fields":{"qux": "789"}}`)}, {Id: mustParseId("id:ns:type::doc5"), Operation: OperationRemove}, } - got := []Document{} + result := []Document{} for { doc, err := dec.Decode() if err == io.EOF { @@ -163,7 +162,7 @@ func testDocumentDecoder(t *testing.T, jsonLike string) { if err != nil { t.Fatal(err) } - got = append(got, doc) + result = append(result, doc) } wantBufLen := 0 if dec.array { @@ -172,8 +171,15 @@ func testDocumentDecoder(t *testing.T, jsonLike string) { if l := dec.buf.Len(); l != wantBufLen { t.Errorf("got dec.buf.Len() = %d, want %d", l, wantBufLen) } - if !reflect.DeepEqual(got, want) { - t.Errorf("got %+v, want %+v", got, want) + if len(docs) != len(result) { + t.Errorf("len(result) = %d, want %d", len(result), len(docs)) + } + for i := 0; i < len(docs); i++ { + got := result[i] + want := docs[i] + if !got.Equal(want) { + t.Errorf("got %+v, want %+v", got, want) + } } } diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java index 7464373df9e..bfd39fb66a5 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Bcp.java @@ -6,6 +6,7 @@ import java.time.Duration; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -87,6 +88,19 @@ public class Bcp { public static Bcp empty() { return empty; } @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Bcp bcp = (Bcp) o; + return defaultDeadline.equals(bcp.defaultDeadline) && groups.equals(bcp.groups); + } + + @Override + public int hashCode() { + return Objects.hash(defaultDeadline, groups); + } + + @Override public String toString() { if (isEmpty()) return "empty BCP"; return "BCP of " + @@ -117,6 +131,19 @@ public class Bcp { public Duration deadline() { return deadline; } @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Group group = (Group) o; + return members.equals(group.members) && memberRegions.equals(group.memberRegions) && deadline.equals(group.deadline); + } + + @Override + public int hashCode() { + return Objects.hash(members, memberRegions, deadline); + } + + @Override public String toString() { return "BCP group of " + members; } diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java index bd5056deec6..ac36e8e6c4d 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentInstanceSpec.java @@ -315,22 +315,27 @@ public class DeploymentInstanceSpec extends DeploymentSpec.Steps { steps().equals(other.steps()) && athenzService.equals(other.athenzService) && notifications.equals(other.notifications) && - endpoints.equals(other.endpoints); + endpoints.equals(other.endpoints) && + zoneEndpoints.equals(other.zoneEndpoints) && + bcp.equals(other.bcp) && + tags.equals(other.tags); } @Override public int hashCode() { - return Objects.hash(globalServiceId, upgradePolicy, revisionTarget, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints); + return Objects.hash(globalServiceId, upgradePolicy, revisionTarget, upgradeRollout, changeBlockers, steps(), athenzService, notifications, endpoints, zoneEndpoints, bcp, tags); } int deployableHashCode() { List<DeploymentSpec.DeclaredZone> zones = zones().stream().filter(zone -> zone.concerns(prod)).toList(); - Object[] toHash = new Object[zones.size() + 4]; + Object[] toHash = new Object[zones.size() + 6]; int i = 0; toHash[i++] = name; toHash[i++] = endpoints; + toHash[i++] = zoneEndpoints; toHash[i++] = globalServiceId; toHash[i++] = tags; + toHash[i++] = bcp; for (DeploymentSpec.DeclaredZone zone : zones) toHash[i++] = Objects.hash(zone, zone.athenzService()); diff --git a/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.cpp b/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.cpp index 75e0a82ff1d..4b8916273e7 100644 --- a/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.cpp +++ b/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.cpp @@ -8,15 +8,8 @@ namespace search::attribute { using BitVectorSP = BitVectorSearchCache::BitVectorSP; -BitVectorSearchCache::BitVectorSearchCache() - : _mutex(), - _cache() -{ -} - -BitVectorSearchCache::~BitVectorSearchCache() -{ -} +BitVectorSearchCache::BitVectorSearchCache() = default; +BitVectorSearchCache::~BitVectorSearchCache() = default; void BitVectorSearchCache::insert(const vespalib::string &term, Entry::SP entry) diff --git a/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp b/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp index 0f52b64ede0..5a5702f50e8 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp @@ -6,18 +6,6 @@ namespace search { template <typename EntryT> -EnumStoreComparator<EntryT>::EnumStoreComparator(const DataStoreType& data_store, const EntryT& fallback_value) - : ParentType(data_store, fallback_value) -{ -} - -template <typename EntryT> -EnumStoreComparator<EntryT>::EnumStoreComparator(const DataStoreType& data_store) - : ParentType(data_store) -{ -} - -template <typename EntryT> bool EnumStoreComparator<EntryT>::equal_helper(const EntryT& lhs, const EntryT& rhs) { diff --git a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h index 5df85cd1c57..546ac82e389 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h +++ b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h @@ -18,8 +18,12 @@ public: using ParentType = vespalib::datastore::UniqueStoreComparator<EntryT, IEnumStore::InternalIndex>; using DataStoreType = typename ParentType::DataStoreType; - EnumStoreComparator(const DataStoreType& data_store, const EntryT& fallback_value); - EnumStoreComparator(const DataStoreType& data_store); + EnumStoreComparator(const DataStoreType& data_store, const EntryT& fallback_value) + : ParentType(data_store, fallback_value) + {} + EnumStoreComparator(const DataStoreType& data_store) + : ParentType(data_store) + {} static bool equal_helper(const EntryT& lhs, const EntryT& rhs); }; diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp index 0c34ae6e330..12c887eb407 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.cpp @@ -10,28 +10,24 @@ namespace search::attribute { using vespalib::btree::BTreeNode; PostingListSearchContext:: -PostingListSearchContext(const IEnumStoreDictionary& dictionary, - uint32_t docIdLimit, - uint64_t numValues, - bool hasWeight, - bool useBitVector, - const ISearchContext &baseSearchCtx) +PostingListSearchContext(const IEnumStoreDictionary& dictionary, bool has_btree_dictionary, uint32_t docIdLimit, + uint64_t numValues, bool hasWeight, bool useBitVector, const ISearchContext &baseSearchCtx) : _dictionary(dictionary), - _frozenDictionary(_dictionary.get_has_btree_dictionary() ? _dictionary.get_posting_dictionary().getFrozenView() : FrozenDictionary()), - _lowerDictItr(_dictionary.get_has_btree_dictionary() ? DictionaryConstIterator(BTreeNode::Ref(), _frozenDictionary.getAllocator()) : DictionaryConstIterator()), - _upperDictItr(_dictionary.get_has_btree_dictionary() ? DictionaryConstIterator(BTreeNode::Ref(), _frozenDictionary.getAllocator()) : DictionaryConstIterator()), + _baseSearchCtx(baseSearchCtx), + _bv(nullptr), + _frozenDictionary(has_btree_dictionary ? _dictionary.get_posting_dictionary().getFrozenView() : FrozenDictionary()), + _lowerDictItr(has_btree_dictionary ? DictionaryConstIterator(BTreeNode::Ref(), _frozenDictionary.getAllocator()) : DictionaryConstIterator()), + _upperDictItr(has_btree_dictionary ? DictionaryConstIterator(BTreeNode::Ref(), _frozenDictionary.getAllocator()) : DictionaryConstIterator()), + _numValues(numValues), _uniqueValues(0u), _docIdLimit(docIdLimit), _dictSize(_frozenDictionary.size()), - _numValues(numValues), - _hasWeight(hasWeight), - _useBitVector(useBitVector), _pidx(), _frozenRoot(), _FSTC(0.0), _PLSTC(0.0), - _bv(nullptr), - _baseSearchCtx(baseSearchCtx) + _hasWeight(hasWeight), + _useBitVector(useBitVector) { } diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h index d0a8958f615..107abd24069 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.h @@ -32,26 +32,25 @@ protected: using FrozenDictionary = Dictionary::FrozenView; using EnumIndex = IEnumStore::Index; - const IEnumStoreDictionary& _dictionary; - const FrozenDictionary _frozenDictionary; + const IEnumStoreDictionary & _dictionary; + const ISearchContext &_baseSearchCtx; + const BitVector *_bv; // bitvector if _useBitVector has been set + const FrozenDictionary _frozenDictionary; DictionaryConstIterator _lowerDictItr; DictionaryConstIterator _upperDictItr; + uint64_t _numValues; // attr.getStatus().getNumValues(); uint32_t _uniqueValues; uint32_t _docIdLimit; uint32_t _dictSize; - uint64_t _numValues; // attr.getStatus().getNumValues(); - bool _hasWeight; - bool _useBitVector; vespalib::datastore::EntryRef _pidx; vespalib::datastore::EntryRef _frozenRoot; // Posting list in tree form float _FSTC; // Filtering Search Time Constant float _PLSTC; // Posting List Search Time Constant - const BitVector *_bv; // bitvector if _useBitVector has been set - const ISearchContext &_baseSearchCtx; - + bool _hasWeight; + bool _useBitVector; - PostingListSearchContext(const IEnumStoreDictionary& dictionary, uint32_t docIdLimit, uint64_t numValues, bool hasWeight, - bool useBitVector, const ISearchContext &baseSearchCtx); + PostingListSearchContext(const IEnumStoreDictionary& dictionary, bool has_btree_dictionary, uint32_t docIdLimit, + uint64_t numValues, bool hasWeight, bool useBitVector, const ISearchContext &baseSearchCtx); ~PostingListSearchContext() override; diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp index 98f89f9080f..d32d8cde7ea 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp @@ -22,9 +22,8 @@ namespace search::attribute { template <typename DataT> PostingListSearchContextT<DataT>:: PostingListSearchContextT(const IEnumStoreDictionary& dictionary, uint32_t docIdLimit, uint64_t numValues, bool hasWeight, - const PostingList &postingList, - bool useBitVector, const ISearchContext &searchContext) - : PostingListSearchContext(dictionary, docIdLimit, numValues, hasWeight, useBitVector, searchContext), + const PostingList &postingList, bool useBitVector, const ISearchContext &searchContext) + : PostingListSearchContext(dictionary, dictionary.get_has_btree_dictionary(), docIdLimit, numValues, hasWeight, useBitVector, searchContext), _postingList(postingList), _merger(docIdLimit) { diff --git a/searchlib/src/vespa/searchlib/attribute/search_context.cpp b/searchlib/src/vespa/searchlib/attribute/search_context.cpp index a0345ddce70..a0208ab787e 100644 --- a/searchlib/src/vespa/searchlib/attribute/search_context.cpp +++ b/searchlib/src/vespa/searchlib/attribute/search_context.cpp @@ -10,14 +10,6 @@ using search::queryeval::SearchIterator; namespace search::attribute { -SearchContext::SearchContext(const AttributeVector &attr) noexcept - : _attr(attr), - _plsc(nullptr) -{ -} - -SearchContext::~SearchContext() = default; - unsigned int SearchContext::approximateHits() const { diff --git a/searchlib/src/vespa/searchlib/attribute/search_context.h b/searchlib/src/vespa/searchlib/attribute/search_context.h index 025b0fdf113..cc55beee216 100644 --- a/searchlib/src/vespa/searchlib/attribute/search_context.h +++ b/searchlib/src/vespa/searchlib/attribute/search_context.h @@ -30,7 +30,7 @@ public: SearchContext(SearchContext&&) noexcept = default; SearchContext& operator=(const SearchContext&) = delete; SearchContext& operator=(SearchContext&&) noexcept = delete; - ~SearchContext() override; + ~SearchContext() override = default; unsigned int approximateHits() const override; std::unique_ptr<queryeval::SearchIterator> createIterator(fef::TermFieldMatchData* matchData, bool strict) override; @@ -47,7 +47,10 @@ public: const AttributeVector& attribute() const { return _attr; } protected: - SearchContext(const AttributeVector& attr) noexcept; + SearchContext(const AttributeVector& attr) noexcept + : _attr(attr), + _plsc(nullptr) + {} const AttributeVector& _attr; attribute::IPostingListSearchContext* _plsc; diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp index 093052608c5..199e9a4b8a0 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp @@ -97,7 +97,7 @@ FieldIndex<interleaved_features>::findFrozen(const vespalib::stringref word) con if (itr.valid()) { return _postingListStore.beginFrozen(itr.getData().load_acquire()); } - return typename PostingList::Iterator(); + return typename PostingList::ConstIterator(); } template <bool interleaved_features> diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp index 3e0cd71be8b..f2f7ad212de 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp @@ -176,8 +176,9 @@ template <HnswIndexType type> bool HnswIndex<type>::have_closer_distance(HnswTraversalCandidate candidate, const HnswTraversalCandidateVector& result) const { + auto df = _distance_ff->for_insertion_vector(get_vector(candidate.nodeid)); for (const auto & neighbor : result) { - double dist = calc_distance(candidate.nodeid, neighbor.nodeid); + double dist = calc_distance(*df, neighbor.nodeid); if (dist < candidate.distance) { return true; } @@ -253,8 +254,9 @@ HnswIndex<type>::shrink_if_needed(uint32_t nodeid, uint32_t level) if (old_links.size() > max_links) { HnswTraversalCandidateVector neighbors; neighbors.reserve(old_links.size()); + auto df = _distance_ff->for_insertion_vector(get_vector(nodeid)); for (uint32_t neighbor_nodeid : old_links) { - double dist = calc_distance(nodeid, neighbor_nodeid); + double dist = calc_distance(*df, neighbor_nodeid); neighbors.emplace_back(neighbor_nodeid, dist); } auto split = select_neighbors(neighbors, max_links); @@ -297,17 +299,6 @@ HnswIndex<type>::remove_link_to(uint32_t remove_from, uint32_t remove_id, uint32 _graph.set_link_array(remove_from, level, new_links); } - -template <HnswIndexType type> -double -HnswIndex<type>::calc_distance(uint32_t lhs_nodeid, uint32_t rhs_nodeid) const -{ - auto lhs = get_vector(lhs_nodeid); - auto df = _distance_ff->for_insertion_vector(lhs); - auto rhs = get_vector(rhs_nodeid); - return df->calc(rhs); -} - template <HnswIndexType type> double HnswIndex<type>::calc_distance(const BoundDistanceFunction &df, uint32_t rhs_nodeid) const @@ -639,10 +630,14 @@ HnswIndex<type>::mutual_reconnect(const LinkArrayRef &cluster, uint32_t level) for (uint32_t i = 0; i + 1 < cluster.size(); ++i) { uint32_t n_id_1 = cluster[i]; LinkArrayRef n_list_1 = _graph.get_link_array(n_id_1, level); + std::unique_ptr<BoundDistanceFunction> df; for (uint32_t j = i + 1; j < cluster.size(); ++j) { uint32_t n_id_2 = cluster[j]; if (has_link_to(n_list_1, n_id_2)) continue; - pairs.emplace_back(n_id_1, n_id_2, calc_distance(n_id_1, n_id_2)); + if (!df) { + df = _distance_ff->for_insertion_vector(get_vector(n_id_1)); + } + pairs.emplace_back(n_id_1, n_id_2, calc_distance(*df, n_id_2)); } } std::sort(pairs.begin(), pairs.end()); diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index.h index 7858cb65bf9..20ab0dbea92 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.h @@ -158,7 +158,6 @@ protected: return _vectors.get_vectors(docid); } - double calc_distance(uint32_t lhs_nodeid, uint32_t rhs_nodeid) const; double calc_distance(const BoundDistanceFunction &df, uint32_t rhs_nodeid) const; double calc_distance(const BoundDistanceFunction &df, uint32_t rhs_docid, uint32_t rhs_subspace) const; uint32_t estimate_visited_nodes(uint32_t level, uint32_t nodeid_limit, uint32_t neighbors_to_find, const GlobalFilter* filter) const; diff --git a/searchlib/src/vespa/searchlib/util/rawbuf.cpp b/searchlib/src/vespa/searchlib/util/rawbuf.cpp index 04d69544047..3af29d7eed5 100644 --- a/searchlib/src/vespa/searchlib/util/rawbuf.cpp +++ b/searchlib/src/vespa/searchlib/util/rawbuf.cpp @@ -1,32 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "rawbuf.h" -#include <vespa/vespalib/util/compress.h> #include <cassert> -#include <cstring> #include <cstdlib> namespace search { -RawBuf::RawBuf(size_t size) - : _bufStart(nullptr), - _bufEnd(nullptr), - _bufFillPos(nullptr), - _bufDrainPos(nullptr) -{ - if (size > 0) { - _bufStart = static_cast<char *>(malloc(size)); - } - _bufEnd = _bufStart + size; - _bufDrainPos = _bufFillPos = _bufStart; -} - -RawBuf::~RawBuf() -{ - free(_bufStart); -} - - /** * Allocate a new buffer at least as large as the parameter value, * move any content to the new and delete the old buffer. @@ -50,45 +29,6 @@ RawBuf::expandBuf(size_t needlen) _bufEnd = _bufStart + size; } - -/** - * Put 'data' of 'len'gth into the buffer. If insufficient room, - * make the buffer larger. - */ -void -RawBuf::append(const void *data, size_t len) -{ - if (__builtin_expect(len != 0, true)) { - ensureSize(len); - memcpy(_bufFillPos, data, len); - _bufFillPos += len; - } -} - -void -RawBuf::append(uint8_t byte) -{ - ensureSize(1); - *_bufFillPos++ = byte; -} - -void -RawBuf::appendCompressedPositiveNumber(uint64_t n) -{ - size_t len(vespalib::compress::Integer::compressedPositiveLength(n)); - ensureSize(len); - _bufFillPos += vespalib::compress::Integer::compressPositive(n, _bufFillPos); -} - -void -RawBuf::appendCompressedNumber(int64_t n) -{ - size_t len(vespalib::compress::Integer::compressedLength(n)); - ensureSize(len); - _bufFillPos += vespalib::compress::Integer::compress(n, _bufFillPos); -} - - /** * Compact any free space from the beginning of the buffer, by * copying the contents to the start of the buffer. diff --git a/searchlib/src/vespa/searchlib/util/rawbuf.h b/searchlib/src/vespa/searchlib/util/rawbuf.h index 30018cb45c2..9ecfbc23c24 100644 --- a/searchlib/src/vespa/searchlib/util/rawbuf.h +++ b/searchlib/src/vespa/searchlib/util/rawbuf.h @@ -2,8 +2,9 @@ #pragma once -#include <cstdint> -#include <cstddef> +#include <vespa/vespalib/util/compress.h> +#include <cstring> +#include <cstdlib> namespace search { /** @@ -45,13 +46,35 @@ private: public: RawBuf(const RawBuf &) = delete; RawBuf& operator=(const RawBuf &) = delete; - explicit RawBuf(size_t size); // malloc-s given size, assigns to _bufStart - ~RawBuf(); // Frees _bufStart, i.e. the char[]. + explicit RawBuf(size_t size) + : _bufStart(static_cast<char *>(malloc(size))), + _bufEnd(_bufStart + size), + _bufFillPos(_bufStart), + _bufDrainPos(_bufStart) + { } + ~RawBuf() { + free(_bufStart); + } // Frees _bufStart, i.e. the char[]. - void append(const void *data, size_t len); - void append(uint8_t byte); - void appendCompressedPositiveNumber(uint64_t n); - void appendCompressedNumber(int64_t n); + void append(const void *data, size_t len) { + if (__builtin_expect(len != 0, true)) { + ensureSize(len); + memcpy(_bufFillPos, data, len); + _bufFillPos += len; + } + } + void append(uint8_t byte) { + ensureSize(1); + *_bufFillPos++ = byte; + } + void appendCompressedPositiveNumber(uint64_t n) { + ensureSize(vespalib::compress::Integer::compressedPositiveLength(n)); + _bufFillPos += vespalib::compress::Integer::compressPositive(n, _bufFillPos); + } + void appendCompressedNumber(int64_t n) { + ensureSize(vespalib::compress::Integer::compressedLength(n)); + _bufFillPos += vespalib::compress::Integer::compress(n, _bufFillPos); + } size_t GetFreeLen() const { return _bufEnd - _bufFillPos; } const char *GetDrainPos() const { return _bufDrainPos; } char * GetWritableFillPos(size_t len) { preAlloc(len); return _bufFillPos; } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java index 2f8ef8cbcc0..e97409b40ef 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java @@ -241,7 +241,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen @Override public PrivateKey getPrivateKey() { - return autoReloadingX509KeyManager.getPrivateKey(AutoReloadingX509KeyManager.CERTIFICATE_ALIAS); + return autoReloadingX509KeyManager.getCurrentCertificateWithKey().privateKey(); } @Override @@ -251,7 +251,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen @Override public List<X509Certificate> getIdentityCertificate() { - return List.of(autoReloadingX509KeyManager.getCertificateChain(AutoReloadingX509KeyManager.CERTIFICATE_ALIAS)); + return List.of(autoReloadingX509KeyManager.getCurrentCertificateWithKey().certificate()); } @Override diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index eb13bf634ee..e6d5ea48e8f 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -1219,10 +1219,12 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { parameters.setPriority(DocumentProtocol.Priority.NORMAL_4); getProperty(request, FROM_TIMESTAMP, unsignedLongParser).ifPresent(parameters::setFromTimestamp); - getProperty(request, TO_TIMESTAMP, unsignedLongParser).ifPresent(parameters::setToTimestamp); - if (Long.compareUnsigned(parameters.getFromTimestamp(), parameters.getToTimestamp()) > 0) { - throw new IllegalArgumentException("toTimestamp must be greater than, or equal to, fromTimestamp"); - } + getProperty(request, TO_TIMESTAMP, unsignedLongParser).ifPresent(ts -> { + parameters.setToTimestamp(ts); + if (Long.compareUnsigned(parameters.getFromTimestamp(), parameters.getToTimestamp()) > 0) { + throw new IllegalArgumentException("toTimestamp must be greater than, or equal to, fromTimestamp"); + } + }); StorageCluster storageCluster = resolveCluster(cluster, clusters); parameters.setRoute(storageCluster.name()); diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java index c6cbd20651a..e8f42fbecfa 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java @@ -956,6 +956,39 @@ public class DocumentV1ApiTest { driver.close(); } + private void doTestVisitRequestWithParams(String httpReqParams, Consumer<VisitorParameters> paramChecker) { + try (var driver = new RequestHandlerTestDriver(handler)) { + access.expect(parameters -> { + paramChecker.accept(parameters); + parameters.getControlHandler().onDone(VisitorControlHandler.CompletionCode.SUCCESS, "great success"); + }); + var response = driver.sendRequest("http://localhost/document/v1/?cluster=content&%s".formatted(httpReqParams)); + assertSameJson(""" + { + "pathId": "/document/v1/", + "documents": [ ], + "documentCount": 0 + }""", + response.readAll()); + assertEquals(200, response.getStatus()); + } + } + + @Test + public void visit_timestamp_ranges_can_be_open_in_both_ends() { + // Only specifying fromTimestamp; visit up to current time + doTestVisitRequestWithParams("fromTimestamp=1234", (params) -> { + assertEquals(params.getFromTimestamp(), 1234); + assertEquals(params.getToTimestamp(), 0); // Means "current wall clock time" when it hits storage + }); + + // Only specifying toTimestamp; visit all docs up to this time point + doTestVisitRequestWithParams("toTimestamp=2345", (params) -> { + assertEquals(params.getFromTimestamp(), 0); // The dawn of time(tm) + assertEquals(params.getToTimestamp(), 2345); + }); + } + @Test public void testThroughput() throws InterruptedException { DocumentOperationExecutorConfig executorConfig = new DocumentOperationExecutorConfig.Builder().build(); diff --git a/vespalib/src/apps/vespa-stress-and-validate-memory/stress_and_validate_memory.cpp b/vespalib/src/apps/vespa-stress-and-validate-memory/stress_and_validate_memory.cpp index a7a227e45b3..e31f5e6413e 100644 --- a/vespalib/src/apps/vespa-stress-and-validate-memory/stress_and_validate_memory.cpp +++ b/vespalib/src/apps/vespa-stress-and-validate-memory/stress_and_validate_memory.cpp @@ -14,6 +14,7 @@ std::atomic<bool> stopped = false; std::mutex log_mutex; using namespace vespalib; +using vespalib::alloc::PtrAndSize; const char * description = "Runs stress test of memory by slowly growing a heap filled with 0.\n" @@ -121,7 +122,6 @@ public: size_t make_and_load_alloc_per_thread(); void random_write(unsigned int *seed); private: - using PtrAndSize = std::pair<void *, size_t>; const Config & _cfg; mutable std::mutex _mutex; alloc::MmapFileAllocator _allocator; @@ -153,7 +153,7 @@ FileBackedMemory::make_and_load_alloc_per_thread() { std::lock_guard guard(_mutex); alloc = _allocator.alloc(cfg().alloc_size()); } - memset(alloc.first, 0, cfg().alloc_size()); + memset(alloc.get(), 0, cfg().alloc_size()); std::lock_guard guard(_mutex); _allocations.push_back(std::move(alloc)); return 1; @@ -166,7 +166,7 @@ FileBackedMemory::random_write(unsigned int *seed) { std::lock_guard guard(_mutex); ptrAndSize = _allocations[rand_r(seed) % _allocations.size()]; } - memset(ptrAndSize.first, rand_r(seed)%256, ptrAndSize.second); + memset(ptrAndSize.get(), rand_r(seed)%256, ptrAndSize.size()); } void diff --git a/vespalib/src/tests/btree/btree_test.cpp b/vespalib/src/tests/btree/btree_test.cpp index 6305eca41ac..b2f9f01e517 100644 --- a/vespalib/src/tests/btree/btree_test.cpp +++ b/vespalib/src/tests/btree/btree_test.cpp @@ -297,9 +297,9 @@ BTreeTest::assertMemoryUsage(const vespalib::MemoryUsage & exp, const vespalib:: } TEST_F(BTreeTest, control_iterator_size) { - EXPECT_EQ(208u, sizeof(BTreeIteratorBase<uint32_t, uint32_t, NoAggregated>)); - EXPECT_EQ(208u, sizeof(BTreeIteratorBase<uint32_t, BTreeNoLeafData, NoAggregated>)); - EXPECT_EQ(544u, sizeof(MyTree::Iterator)); + EXPECT_EQ(120u, sizeof(BTreeIteratorBase<uint32_t, uint32_t, NoAggregated>)); + EXPECT_EQ(120u, sizeof(BTreeIteratorBase<uint32_t, BTreeNoLeafData, NoAggregated>)); + EXPECT_EQ(288u, sizeof(MyTree::Iterator)); } TEST_F(BTreeTest, require_that_node_insert_works) diff --git a/vespalib/src/tests/util/mmap_file_allocator/mmap_file_allocator_test.cpp b/vespalib/src/tests/util/mmap_file_allocator/mmap_file_allocator_test.cpp index 545733a1ebd..ef16998902e 100644 --- a/vespalib/src/tests/util/mmap_file_allocator/mmap_file_allocator_test.cpp +++ b/vespalib/src/tests/util/mmap_file_allocator/mmap_file_allocator_test.cpp @@ -6,6 +6,7 @@ using vespalib::alloc::MemoryAllocator; using vespalib::alloc::MmapFileAllocator; +using vespalib::alloc::PtrAndSize; namespace { @@ -18,10 +19,10 @@ struct MyAlloc void* data; size_t size; - MyAlloc(MemoryAllocator& allocator_in, MemoryAllocator::PtrAndSize buf) + MyAlloc(MemoryAllocator& allocator_in, PtrAndSize buf) : allocator(allocator_in), - data(buf.first), - size(buf.second) + data(buf.get()), + size(buf.size()) { } @@ -30,7 +31,7 @@ struct MyAlloc allocator.free(data, size); } - MemoryAllocator::PtrAndSize asPair() const noexcept { return std::make_pair(data, size); } + PtrAndSize asPair() const noexcept { return PtrAndSize(data, size); } }; } diff --git a/vespalib/src/vespa/vespalib/btree/btreeiterator.h b/vespalib/src/vespa/vespalib/btree/btreeiterator.h index 5e18eb0a5de..01f0ad3c2d3 100644 --- a/vespalib/src/vespa/vespalib/btree/btreeiterator.h +++ b/vespalib/src/vespa/vespalib/btree/btreeiterator.h @@ -35,64 +35,70 @@ class NodeElement using NodeType = NodeT; using KeyType = typename NodeType::KeyType; using DataType = typename NodeType::DataType; - const NodeType *_node; - uint32_t _idx; + uint64_t _nodeAndIdx; - NodeType * getWNode() const { return const_cast<NodeType *>(_node); } + NodeType * getWNode() const { return const_cast<NodeType *>(getNode()); } + static constexpr uint8_t NODE_BITS = 57; + static constexpr uint8_t IDX_BITS = 64 - NODE_BITS; + static constexpr uint64_t NODE_MASK = (1ul << NODE_BITS) - 1ul; + static constexpr uint64_t IDX_MASK = (1ul << IDX_BITS) - 1ul; + static constexpr uint8_t IDX_SHIFT = NODE_BITS; + static constexpr uint64_t IDX_ONE = 1ul << NODE_BITS; + + static_assert((NodeType::maxSlots() + 1) < (1ul << IDX_BITS), "IDX can be out of bounds above 127"); public: - NodeElement() : _node(nullptr), _idx(0u) { } - NodeElement(const NodeType *node, uint32_t idx) : _node(node), _idx(idx) { } - - void invalidate() { _node = nullptr; _idx = 0; } - void setNode(const NodeType *node) { _node = node; } - const NodeType * getNode() const { return _node; } - void setIdx(uint32_t idx) { _idx = idx; } - uint32_t getIdx() const { return _idx; } - void incIdx() { ++_idx; } - void decIdx() { --_idx; } - - void setNodeAndIdx(const NodeType *node, uint32_t idx) { - _node = node; - _idx = idx; + NodeElement() noexcept : _nodeAndIdx(0ul) { } + NodeElement(const NodeType *node, uint32_t idx) noexcept + : _nodeAndIdx(uint64_t(node) | uint64_t(idx) << IDX_SHIFT) + { } + + void invalidate() noexcept { _nodeAndIdx = 0; } + void setNode(const NodeType *node) noexcept { + _nodeAndIdx = (_nodeAndIdx & ~NODE_MASK) | uint64_t(node); + } + const NodeType * getNode() const noexcept { return reinterpret_cast<const NodeType *>(_nodeAndIdx & NODE_MASK); } + void setIdx(uint32_t idx) noexcept { + _nodeAndIdx = (_nodeAndIdx & NODE_MASK) | (uint64_t(idx) << IDX_SHIFT); + } + uint32_t getIdx() const noexcept { return _nodeAndIdx >> IDX_SHIFT; } + void incIdx() noexcept { _nodeAndIdx += IDX_ONE; } + void decIdx() noexcept { _nodeAndIdx -= IDX_ONE; } + + void setNodeAndIdx(const NodeType *node, uint32_t idx) noexcept { + _nodeAndIdx = uint64_t(node) | uint64_t(idx) << IDX_SHIFT; } - const KeyType & getKey() const { return _node->getKey(_idx); } - const DataType & getData() const { return _node->getData(_idx); } + const KeyType & getKey() const noexcept { return getNode()->getKey(getIdx()); } + const DataType & getData() const noexcept { return getNode()->getData(getIdx()); } // Only use during compaction when changing reference to moved value - DataType &getWData() { return getWNode()->getWData(_idx); } - bool valid() const { return _node != nullptr; } - void adjustLeftVictimKilled() { - assert(_idx > 0); - --_idx; + DataType &getWData() noexcept { return getWNode()->getWData(getIdx()); } + bool valid() const noexcept { return _nodeAndIdx != 0; } + void adjustLeftVictimKilled() noexcept { + assert(getIdx() > 0); + decIdx(); } - void adjustSteal(uint32_t stolen) { - assert(_idx + stolen < _node->validSlots()); - _idx += stolen; + void adjustSteal(uint32_t stolen) noexcept { + assert(getIdx() + stolen < getNode()->validSlots()); + setIdx(getIdx() + stolen); } - void adjustSplit(bool inRightSplit) { + void adjustSplit(bool inRightSplit) noexcept { if (inRightSplit) - ++_idx; + incIdx(); } - bool adjustSplit(bool inRightSplit, const NodeType *splitNode) { + bool adjustSplit(bool inRightSplit, const NodeType *splitNode) noexcept { adjustSplit(inRightSplit); - if (_idx >= _node->validSlots()) { - _idx -= _node->validSlots(); - _node = splitNode; + if (getIdx() >= getNode()->validSlots()) { + setNodeAndIdx(splitNode, getIdx() - getNode()->validSlots()); return true; } return false; } - void swap(NodeElement &rhs) { - std::swap(_node, rhs._node); - std::swap(_idx, rhs._idx); - } - - bool operator!=(const NodeElement &rhs) const { - return (_node != rhs._node) || (_idx != rhs._idx); + bool operator!=(const NodeElement &rhs) const noexcept { + return _nodeAndIdx != rhs._nodeAndIdx; } }; @@ -110,9 +116,7 @@ template <typename KeyT, class BTreeIteratorBase { protected: - using NodeAllocatorType = BTreeNodeAllocator<KeyT, DataT, AggrT, - INTERNAL_SLOTS, - LEAF_SLOTS>; + using NodeAllocatorType = BTreeNodeAllocator<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS>; using InternalNodeType = BTreeInternalNode<KeyT, AggrT, INTERNAL_SLOTS>; using LeafNodeType = BTreeLeafNode<KeyT, DataT, AggrT, LEAF_SLOTS> ; using InternalNodeTypeRefPair = typename InternalNodeType::RefPair; @@ -152,7 +156,6 @@ protected: // Temporary leaf node when iterating over short arrays std::unique_ptr<LeafNodeTempType> _compatLeafNode; - private: /* * Find the next leaf node, called by operator++() as needed. @@ -194,7 +197,7 @@ protected: /** * Default constructor. Iterator is not associated with a tree. */ - BTreeIteratorBase(); + BTreeIteratorBase() noexcept; /** * Step iterator forwards. If at end then leave it at end. @@ -523,7 +526,7 @@ public: /** * Default constructor. Iterator is not associated with a tree. */ - BTreeConstIterator() : ParentType() { } + BTreeConstIterator() noexcept : ParentType() { } /** * Step iterator forwards. If at end then leave it at end. diff --git a/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp b/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp index a94bac8a67e..5884bb2849b 100644 --- a/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp +++ b/vespalib/src/vespa/vespalib/btree/btreeiterator.hpp @@ -22,8 +22,8 @@ BTreeIteratorBase(const BTreeIteratorBase &other) for (size_t i = 0; i < _pathSize; ++i) { _path[i] = other._path[i]; } - if (other._compatLeafNode.get()) { - _compatLeafNode.reset( new LeafNodeTempType(*other._compatLeafNode)); + if (other._compatLeafNode) { + _compatLeafNode = std::make_unique<LeafNodeTempType>(*other._compatLeafNode); } if (other._leaf.getNode() == other._compatLeafNode.get()) { _leaf.setNode(_compatLeafNode.get()); @@ -435,8 +435,8 @@ BTreeIteratorBase(const KeyDataType *shortArray, _leafRoot(nullptr), _compatLeafNode() { - if(arraySize > 0) { - _compatLeafNode.reset(new LeafNodeTempType(shortArray, arraySize)); + if (arraySize > 0) { + _compatLeafNode = std::make_unique<LeafNodeTempType>(shortArray, arraySize); _leaf.setNode(_compatLeafNode.get()); _leafRoot = _leaf.getNode(); if constexpr (AggrCalcT::hasAggregated()) { @@ -450,7 +450,7 @@ BTreeIteratorBase(const KeyDataType *shortArray, template <typename KeyT, typename DataT, typename AggrT, uint32_t INTERNAL_SLOTS, uint32_t LEAF_SLOTS, uint32_t PATH_SIZE> BTreeIteratorBase<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS, PATH_SIZE>:: -BTreeIteratorBase() +BTreeIteratorBase() noexcept : _leaf(nullptr, 0u), _path(), _pathSize(0), diff --git a/vespalib/src/vespa/vespalib/btree/btreenode.h b/vespalib/src/vespa/vespalib/btree/btreenode.h index 1196b172d1f..0a77a0b4685 100644 --- a/vespalib/src/vespa/vespalib/btree/btreenode.h +++ b/vespalib/src/vespa/vespalib/btree/btreenode.h @@ -39,20 +39,20 @@ public: static constexpr uint8_t LEAF_LEVEL = 0; protected: uint16_t _validSlots; - BTreeNode(uint8_t level) + BTreeNode(uint8_t level) noexcept : _level(level), _isFrozen(false), _validSlots(0) {} - BTreeNode(const BTreeNode &rhs) + BTreeNode(const BTreeNode &rhs) noexcept : _level(rhs._level), _isFrozen(rhs._isFrozen), _validSlots(rhs._validSlots) {} BTreeNode & - operator=(const BTreeNode &rhs) + operator=(const BTreeNode &rhs) noexcept { assert(!_isFrozen); _level = rhs._level; @@ -89,8 +89,8 @@ class BTreeNodeDataWrap public: DataT _data[NumSlots]; - BTreeNodeDataWrap() : _data() {} - ~BTreeNodeDataWrap() { } + BTreeNodeDataWrap() noexcept : _data() {} + ~BTreeNodeDataWrap() = default; void copyData(const BTreeNodeDataWrap &rhs, uint32_t validSlots) { const DataT *rdata = rhs._data; @@ -100,11 +100,11 @@ public: *ldata = *rdata; } - const DataT &getData(uint32_t idx) const { return _data[idx]; } + const DataT &getData(uint32_t idx) const noexcept { return _data[idx]; } // Only use during compaction when changing reference to moved value - DataT &getWData(uint32_t idx) { return _data[idx]; } - void setData(uint32_t idx, const DataT &data) { _data[idx] = data; } - static bool hasData() { return true; } + DataT &getWData(uint32_t idx) noexcept { return _data[idx]; } + void setData(uint32_t idx, const DataT &data) noexcept { _data[idx] = data; } + static bool hasData() noexcept { return true; } }; @@ -112,7 +112,7 @@ template <uint32_t NumSlots> class BTreeNodeDataWrap<BTreeNoLeafData, NumSlots> { public: - BTreeNodeDataWrap() {} + BTreeNodeDataWrap() noexcept {} void copyData(const BTreeNodeDataWrap &rhs, uint32_t validSlots) { (void) rhs; @@ -145,7 +145,7 @@ class BTreeNodeAggregatedWrap static AggrT _instance; public: - BTreeNodeAggregatedWrap() + BTreeNodeAggregatedWrap() noexcept : _aggr() {} AggrT &getAggregated() { return _aggr; } @@ -161,7 +161,7 @@ class BTreeNodeAggregatedWrap<NoAggregated> static NoAggregated _instance; public: - BTreeNodeAggregatedWrap() {} + BTreeNodeAggregatedWrap() noexcept {} NoAggregated &getAggregated() { return _instance; } const NoAggregated &getAggregated() const { return _instance; } @@ -174,14 +174,14 @@ template <typename KeyT, uint32_t NumSlots> class BTreeNodeT : public BTreeNode { protected: KeyT _keys[NumSlots]; - BTreeNodeT(uint8_t level) + BTreeNodeT(uint8_t level) noexcept : BTreeNode(level), _keys() {} ~BTreeNodeT() = default; - BTreeNodeT(const BTreeNodeT &rhs) + BTreeNodeT(const BTreeNodeT &rhs) noexcept : BTreeNode(rhs) { const KeyT *rkeys = rhs._keys; @@ -192,7 +192,7 @@ protected: } BTreeNodeT & - operator=(const BTreeNodeT &rhs) + operator=(const BTreeNodeT &rhs) noexcept { BTreeNode::operator=(rhs); const KeyT *rkeys = rhs._keys; @@ -204,16 +204,16 @@ protected: } public: - const KeyT & getKey(uint32_t idx) const { return _keys[idx]; } - const KeyT & getLastKey() const { return _keys[validSlots() - 1]; } - void writeKey(uint32_t idx, const KeyT & key) { + const KeyT & getKey(uint32_t idx) const noexcept { return _keys[idx]; } + const KeyT & getLastKey() const noexcept { return _keys[validSlots() - 1]; } + void writeKey(uint32_t idx, const KeyT & key) noexcept { if constexpr (std::is_same_v<KeyT, vespalib::datastore::AtomicEntryRef>) { _keys[idx].store_release(key.load_relaxed()); } else { _keys[idx] = key; } } - void write_key_relaxed(uint32_t idx, const KeyT & key) { _keys[idx] = key; } + void write_key_relaxed(uint32_t idx, const KeyT & key) noexcept { _keys[idx] = key; } template <typename CompareT> uint32_t lower_bound(uint32_t sidx, const KeyT & key, CompareT comp) const; @@ -224,10 +224,10 @@ public: template <typename CompareT> uint32_t upper_bound(uint32_t sidx, const KeyT & key, CompareT comp) const; - bool isFull() const { return validSlots() == NumSlots; } - bool isAtLeastHalfFull() const { return validSlots() >= minSlots(); } - static uint32_t maxSlots() { return NumSlots; } - static uint32_t minSlots() { return NumSlots / 2; } + bool isFull() const noexcept { return validSlots() == NumSlots; } + bool isAtLeastHalfFull() const noexcept { return validSlots() >= minSlots(); } + static constexpr uint32_t maxSlots() noexcept { return NumSlots; } + static constexpr uint32_t minSlots() noexcept { return NumSlots / 2; } }; template <typename KeyT, typename DataT, typename AggrT, uint32_t NumSlots> @@ -247,14 +247,14 @@ public: using DataWrapType::setData; using DataWrapType::copyData; protected: - BTreeNodeTT(uint8_t level) + BTreeNodeTT(uint8_t level) noexcept : ParentType(level), DataWrapType() {} - ~BTreeNodeTT() {} + ~BTreeNodeTT() = default; - BTreeNodeTT(const BTreeNodeTT &rhs) + BTreeNodeTT(const BTreeNodeTT &rhs) noexcept : ParentType(rhs), DataWrapType(rhs), AggrWrapType(rhs) @@ -262,7 +262,7 @@ protected: copyData(rhs, _validSlots); } - BTreeNodeTT &operator=(const BTreeNodeTT &rhs) { + BTreeNodeTT &operator=(const BTreeNodeTT &rhs) noexcept { ParentType::operator=(rhs); AggrWrapType::operator=(rhs); copyData(rhs, _validSlots); @@ -325,19 +325,19 @@ public: private: uint32_t _validLeaves; protected: - BTreeInternalNode() + BTreeInternalNode() noexcept : ParentType(EMPTY_LEVEL), _validLeaves(0u) {} - BTreeInternalNode(const BTreeInternalNode &rhs) + BTreeInternalNode(const BTreeInternalNode &rhs) noexcept : ParentType(rhs), _validLeaves(rhs._validLeaves) {} - ~BTreeInternalNode() {} + ~BTreeInternalNode() = default; - BTreeInternalNode &operator=(const BTreeInternalNode &rhs) { + BTreeInternalNode &operator=(const BTreeInternalNode &rhs) noexcept { ParentType::operator=(rhs); _validLeaves = rhs._validLeaves; return *this; @@ -430,8 +430,7 @@ public: } }; -template <typename KeyT, typename DataT, typename AggrT, - uint32_t NumSlots = 16> +template <typename KeyT, typename DataT, typename AggrT, uint32_t NumSlots = 16> class BTreeLeafNode : public BTreeNodeTT<KeyT, DataT, AggrT, NumSlots> { public: @@ -460,17 +459,17 @@ public: using KeyType = KeyT; using DataType = DataT; protected: - BTreeLeafNode() : ParentType(LEAF_LEVEL) {} + BTreeLeafNode() noexcept : ParentType(LEAF_LEVEL) {} - BTreeLeafNode(const BTreeLeafNode &rhs) + BTreeLeafNode(const BTreeLeafNode &rhs) noexcept : ParentType(rhs) {} - BTreeLeafNode(const KeyDataType *smallArray, uint32_t arraySize); + BTreeLeafNode(const KeyDataType *smallArray, uint32_t arraySize) noexcept; ~BTreeLeafNode() = default; - BTreeLeafNode &operator=(const BTreeLeafNode &rhs) { + BTreeLeafNode &operator=(const BTreeLeafNode &rhs) noexcept { ParentType::operator=(rhs); return *this; } @@ -535,8 +534,7 @@ public: using ParentType = BTreeLeafNode<KeyT, DataT, AggrT, NumSlots>; using KeyDataType = typename ParentType::KeyDataType; - BTreeLeafNodeTemp(const KeyDataType *smallArray, - uint32_t arraySize) + BTreeLeafNodeTemp(const KeyDataType *smallArray, uint32_t arraySize) noexcept : ParentType(smallArray, arraySize) {} diff --git a/vespalib/src/vespa/vespalib/btree/btreenode.hpp b/vespalib/src/vespa/vespalib/btree/btreenode.hpp index c8bc4ec614c..90f4c9a9cc5 100644 --- a/vespalib/src/vespa/vespalib/btree/btreenode.hpp +++ b/vespalib/src/vespa/vespalib/btree/btreenode.hpp @@ -366,7 +366,7 @@ BTreeInternalNode<KeyT, AggrT, NumSlots>::cleanFrozen() template <typename KeyT, typename DataT, typename AggrT, uint32_t NumSlots> BTreeLeafNode<KeyT, DataT, AggrT, NumSlots>:: -BTreeLeafNode(const KeyDataType *smallArray, uint32_t arraySize) +BTreeLeafNode(const KeyDataType *smallArray, uint32_t arraySize) noexcept : ParentType(LEAF_LEVEL) { assert(arraySize <= BTreeLeafNode::maxSlots()); diff --git a/vespalib/src/vespa/vespalib/btree/btreeroot.h b/vespalib/src/vespa/vespalib/btree/btreeroot.h index c23cf900367..cd1d98725dc 100644 --- a/vespalib/src/vespa/vespalib/btree/btreeroot.h +++ b/vespalib/src/vespa/vespalib/btree/btreeroot.h @@ -13,10 +13,10 @@ namespace vespalib::btree { template <typename, typename, typename, size_t, size_t> class BTreeNodeAllocator; -template <typename, typename, typename, size_t, size_t, class> class -BTreeBuilder; -template <typename, typename, typename, size_t, size_t, class> class -BTreeAggregator; +template <typename, typename, typename, size_t, size_t, class> +class BTreeBuilder; +template <typename, typename, typename, size_t, size_t, class> +class BTreeAggregator; template <typename KeyT, typename DataT, @@ -61,15 +61,14 @@ public: const NodeAllocatorType *const _allocator; public: using Iterator = ConstIterator; - FrozenView(); - FrozenView(BTreeNode::Ref frozenRoot, - const NodeAllocatorType & allocator); - ConstIterator find(const KeyType& key, - CompareT comp = CompareT()) const; - ConstIterator lowerBound(const KeyType &key, - CompareT comp = CompareT()) const; - ConstIterator upperBound(const KeyType &key, - CompareT comp = CompareT()) const; + FrozenView() : _frozenRoot(BTreeNode::Ref()),_allocator(nullptr) {} + FrozenView(BTreeNode::Ref frozenRoot, const NodeAllocatorType & allocator) + : _frozenRoot(frozenRoot), + _allocator(&allocator) + {} + ConstIterator find(const KeyType& key, CompareT comp = CompareT()) const; + ConstIterator lowerBound(const KeyType &key, CompareT comp = CompareT()) const; + ConstIterator upperBound(const KeyType &key, CompareT comp = CompareT()) const; ConstIterator begin() const { return ConstIterator(_frozenRoot, *_allocator); } @@ -78,7 +77,12 @@ public: } BTreeNode::Ref getRoot() const { return _frozenRoot; } - size_t size() const; + size_t size() const { + if (NodeAllocatorType::isValidRef(_frozenRoot)) { + return _allocator->validLeaves(_frozenRoot); + } + return 0u; + } const NodeAllocatorType &getAllocator() const { return *_allocator; } const AggrT &getAggregated() const { diff --git a/vespalib/src/vespa/vespalib/btree/btreeroot.hpp b/vespalib/src/vespa/vespalib/btree/btreeroot.hpp index fdcc957009b..73e5fb32b92 100644 --- a/vespalib/src/vespa/vespalib/btree/btreeroot.hpp +++ b/vespalib/src/vespa/vespalib/btree/btreeroot.hpp @@ -14,8 +14,7 @@ namespace vespalib::btree { //----------------------- BTreeRoot ------------------------------------------// -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> vespalib::string BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: toString(BTreeNode::Ref node, @@ -123,8 +122,7 @@ isValid(BTreeNode::Ref node, return true; } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::Iterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: findHelper(BTreeNode::Ref root, const KeyType & key, @@ -138,20 +136,17 @@ findHelper(BTreeNode::Ref root, const KeyType & key, return itr; } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::Iterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -lowerBoundHelper(BTreeNode::Ref root, const KeyType & key, - const NodeAllocatorType & allocator, CompareT comp) +lowerBoundHelper(BTreeNode::Ref root, const KeyType & key, const NodeAllocatorType & allocator, CompareT comp) { Iterator itr(BTreeNode::Ref(), allocator); itr.lower_bound(root, key, comp); return itr; } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::Iterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: upperBoundHelper(BTreeNode::Ref root, const KeyType & key, @@ -167,31 +162,10 @@ upperBoundHelper(BTreeNode::Ref root, const KeyType & key, //----------------------- BTreeRoot::FrozenView ----------------------------------// -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> -BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -FrozenView::FrozenView() - : _frozenRoot(BTreeNode::Ref()), - _allocator(nullptr) -{ -} - -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> -BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -FrozenView::FrozenView(BTreeNode::Ref frozenRoot, - const NodeAllocatorType & allocator) - : _frozenRoot(frozenRoot), - _allocator(&allocator) -{ -} - -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::ConstIterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -FrozenView::find(const KeyType & key, - CompareT comp) const +FrozenView::find(const KeyType & key, CompareT comp) const { ConstIterator itr(BTreeNode::Ref(), *_allocator); itr.lower_bound(_frozenRoot, key, comp); @@ -201,24 +175,20 @@ FrozenView::find(const KeyType & key, return itr; } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::ConstIterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -FrozenView::lowerBound(const KeyType & key, - CompareT comp) const +FrozenView::lowerBound(const KeyType & key, CompareT comp) const { ConstIterator itr(BTreeNode::Ref(), *_allocator); itr.lower_bound(_frozenRoot, key, comp); return itr; } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::ConstIterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -FrozenView::upperBound(const KeyType & key, - CompareT comp) const +FrozenView::upperBound(const KeyType & key, CompareT comp) const { ConstIterator itr(_frozenRoot, *_allocator); if (itr.valid() && !comp(key, itr.getKey())) { @@ -227,30 +197,15 @@ FrozenView::upperBound(const KeyType & key, return itr; } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> -size_t -BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -FrozenView::size() const -{ - if (NodeAllocatorType::isValidRef(_frozenRoot)) { - return _allocator->validLeaves(_frozenRoot); - } - return 0u; -} - //----------------------- BTreeRoot ----------------------------------------------// -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::BTreeRootT() = default; -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::~BTreeRootT() = default; -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> void BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: clear(NodeAllocatorType &allocator) @@ -263,39 +218,32 @@ clear(NodeAllocatorType &allocator) } } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::Iterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -find(const KeyType & key, const NodeAllocatorType & allocator, - CompareT comp) const +find(const KeyType & key, const NodeAllocatorType & allocator, CompareT comp) const { return findHelper(_root, key, allocator, comp); } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::Iterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -lowerBound(const KeyType & key, const NodeAllocatorType & allocator, - CompareT comp) const +lowerBound(const KeyType & key, const NodeAllocatorType & allocator, CompareT comp) const { return lowerBoundHelper(_root, key, allocator, comp); } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> typename BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>::Iterator BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -upperBound(const KeyType & key, const NodeAllocatorType & allocator, - CompareT comp) const +upperBound(const KeyType & key, const NodeAllocatorType & allocator, CompareT comp) const { return upperBoundHelper(_root, key, allocator, comp); } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> size_t BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: size(const NodeAllocatorType &allocator) const @@ -307,8 +255,7 @@ size(const NodeAllocatorType &allocator) const } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> size_t BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: frozenSize(const NodeAllocatorType &allocator) const @@ -321,8 +268,7 @@ frozenSize(const NodeAllocatorType &allocator) const } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> vespalib::string BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: toString(const NodeAllocatorType &allocator) const @@ -353,8 +299,7 @@ template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT, class AggrCalcT> bool BTreeRoot<KeyT, DataT, AggrT, CompareT, TraitsT, AggrCalcT>:: -isValidFrozen(const NodeAllocatorType &allocator, - CompareT comp) const +isValidFrozen(const NodeAllocatorType &allocator, CompareT comp) const { BTreeNode::Ref frozenRoot = getFrozenRoot(); if (NodeAllocatorType::isValidRef(frozenRoot)) { @@ -382,8 +327,7 @@ template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> size_t BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: -bitSize(BTreeNode::Ref node, - const NodeAllocatorType &allocator) const +bitSize(BTreeNode::Ref node, const NodeAllocatorType &allocator) const { if (allocator.isLeafRef(node)) { return sizeof(LeafNodeType) * 8; @@ -399,8 +343,7 @@ bitSize(BTreeNode::Ref node, } -template <typename KeyT, typename DataT, typename AggrT, typename CompareT, - typename TraitsT> +template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT> void BTreeRootT<KeyT, DataT, AggrT, CompareT, TraitsT>:: thaw(Iterator &itr) @@ -455,8 +398,7 @@ insert(Iterator &itr, { using Inserter = BTreeInserter<KeyT, DataT, AggrT, CompareT, TraitsT, AggrCalcT>; bool oldFrozen = isFrozen(); - Inserter::insert(_root, itr, key, data, - aggrCalc); + Inserter::insert(_root, itr, key, data,aggrCalc); if (oldFrozen && !isFrozen()) itr.getAllocator().needFreeze(this); } @@ -483,8 +425,7 @@ template <typename KeyT, typename DataT, typename AggrT, typename CompareT, typename TraitsT, class AggrCalcT> void BTreeRoot<KeyT, DataT, AggrT, CompareT, TraitsT, AggrCalcT>:: -remove(Iterator &itr, - const AggrCalcT &aggrCalc) +remove(Iterator &itr, const AggrCalcT &aggrCalc) { using Remover = BTreeRemover<KeyT, DataT, AggrT, CompareT, TraitsT, AggrCalcT>; bool oldFrozen = isFrozen(); diff --git a/vespalib/src/vespa/vespalib/stllike/allocator.h b/vespalib/src/vespa/vespalib/stllike/allocator.h index b34533740a2..f35cfce8359 100644 --- a/vespalib/src/vespa/vespalib/stllike/allocator.h +++ b/vespalib/src/vespa/vespalib/stllike/allocator.h @@ -17,7 +17,7 @@ public: allocator_large() noexcept : _allocator(alloc::MemoryAllocator::select_allocator()) {} using value_type = T; constexpr T * allocate(std::size_t n) { - return static_cast<T *>(_allocator->alloc(n*sizeof(T)).first); + return static_cast<T *>(_allocator->alloc(n*sizeof(T)).get()); } void deallocate(T * p, std::size_t n) { _allocator->free(p, n*sizeof(T)); diff --git a/vespalib/src/vespa/vespalib/test/memory_allocator_observer.cpp b/vespalib/src/vespa/vespalib/test/memory_allocator_observer.cpp index ba23970f0ea..43817e63948 100644 --- a/vespalib/src/vespa/vespalib/test/memory_allocator_observer.cpp +++ b/vespalib/src/vespa/vespalib/test/memory_allocator_observer.cpp @@ -20,7 +20,7 @@ MemoryAllocatorObserver::MemoryAllocatorObserver(Stats &stats) } MemoryAllocatorObserver::~MemoryAllocatorObserver() = default; -MemoryAllocatorObserver::PtrAndSize +PtrAndSize MemoryAllocatorObserver::alloc(size_t sz) const { ++_stats.alloc_cnt; diff --git a/vespalib/src/vespa/vespalib/util/alloc.cpp b/vespalib/src/vespa/vespalib/util/alloc.cpp index 83c8a7de7e2..cb9a4b688b3 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.cpp +++ b/vespalib/src/vespa/vespalib/util/alloc.cpp @@ -284,12 +284,12 @@ AutoAllocator::getAllocator(size_t mmapLimit, size_t alignment) { return getAutoAllocator(availableAutoAllocators().first, mmapLimit, alignment); } -MemoryAllocator::PtrAndSize +PtrAndSize HeapAllocator::alloc(size_t sz) const { return salloc(sz); } -MemoryAllocator::PtrAndSize +PtrAndSize HeapAllocator::salloc(size_t sz) { return PtrAndSize((sz > 0) ? malloc(sz) : nullptr, sz); } @@ -299,10 +299,10 @@ void HeapAllocator::free(PtrAndSize alloc) const { } void HeapAllocator::sfree(PtrAndSize alloc) { - if (alloc.first) { ::free(alloc.first); } + if (alloc.get()) { ::free(alloc.get()); } } -MemoryAllocator::PtrAndSize +PtrAndSize AlignedHeapAllocator::alloc(size_t sz) const { if (!sz) { return PtrAndSize(nullptr, 0); } void* ptr; @@ -318,12 +318,12 @@ MMapAllocator::resize_inplace(PtrAndSize current, size_t newSize) const { return sresize_inplace(current, newSize); } -MemoryAllocator::PtrAndSize +PtrAndSize MMapAllocator::alloc(size_t sz) const { return salloc(sz, nullptr); } -MemoryAllocator::PtrAndSize +PtrAndSize MMapAllocator::salloc(size_t sz, void * wantedAddress) { void * buf(nullptr); @@ -382,23 +382,23 @@ MMapAllocator::salloc(size_t sz, void * wantedAddress) size_t MMapAllocator::sresize_inplace(PtrAndSize current, size_t newSize) { newSize = round_up_to_page_size(newSize); - if (newSize > current.second) { + if (newSize > current.size()) { return extend_inplace(current, newSize); - } else if (newSize < current.second) { + } else if (newSize < current.size()) { return shrink_inplace(current, newSize); } else { - return current.second; + return current.size(); } } size_t MMapAllocator::extend_inplace(PtrAndSize current, size_t newSize) { - if (current.second == 0u) { + if (current.size() == 0u) { return 0u; } - PtrAndSize got = MMapAllocator::salloc(newSize - current.second, static_cast<char *>(current.first)+current.second); - if ((static_cast<const char *>(current.first) + current.second) == static_cast<const char *>(got.first)) { - return current.second + got.second; + PtrAndSize got = MMapAllocator::salloc(newSize - current.size(), static_cast<char *>(current.get())+current.size()); + if ((static_cast<const char *>(current.get()) + current.size()) == static_cast<const char *>(got.get())) { + return current.size() + got.size(); } else { MMapAllocator::sfree(got); return 0; @@ -407,7 +407,7 @@ MMapAllocator::extend_inplace(PtrAndSize current, size_t newSize) { size_t MMapAllocator::shrink_inplace(PtrAndSize current, size_t newSize) { - PtrAndSize toUnmap(static_cast<char *>(current.first)+newSize, current.second - newSize); + PtrAndSize toUnmap(static_cast<char *>(current.get())+newSize, current.size() - newSize); sfree(toUnmap); return newSize; } @@ -418,27 +418,27 @@ void MMapAllocator::free(PtrAndSize alloc) const { void MMapAllocator::sfree(PtrAndSize alloc) { - if (alloc.first != nullptr) { - int madvise_retval = madvise(alloc.first, alloc.second, MADV_DONTNEED); + if (alloc.get() != nullptr) { + int madvise_retval = madvise(alloc.get(), alloc.size(), MADV_DONTNEED); if (madvise_retval != 0) { std::error_code ec(errno, std::system_category()); if (errno == EINVAL) { - LOG(debug, "madvise(%p, %lx)=%d, errno=%s", alloc.first, alloc.second, madvise_retval, ec.message().c_str()); + LOG(debug, "madvise(%p, %lx)=%d, errno=%s", alloc.get(), alloc.size(), madvise_retval, ec.message().c_str()); } else { - LOG(warning, "madvise(%p, %lx)=%d, errno=%s", alloc.first, alloc.second, madvise_retval, ec.message().c_str()); + LOG(warning, "madvise(%p, %lx)=%d, errno=%s", alloc.get(), alloc.size(), madvise_retval, ec.message().c_str()); } } - int munmap_retval = munmap(alloc.first, alloc.second); + int munmap_retval = munmap(alloc.get(), alloc.size()); if (munmap_retval != 0) { std::error_code ec(errno, std::system_category()); - LOG(warning, "munmap(%p, %lx)=%d, errno=%s", alloc.first, alloc.second, munmap_retval, ec.message().c_str()); + LOG(warning, "munmap(%p, %lx)=%d, errno=%s", alloc.get(), alloc.size(), munmap_retval, ec.message().c_str()); abort(); } - if (alloc.second >= _G_MMapLogLimit) { + if (alloc.size() >= _G_MMapLogLimit) { std::lock_guard guard(_G_lock); - MMapInfo info = _G_HugeMappings[alloc.first]; - assert(alloc.second == info._sz); - _G_HugeMappings.erase(alloc.first); + MMapInfo info = _G_HugeMappings[alloc.get()]; + assert(alloc.size() == info._sz); + _G_HugeMappings.erase(alloc.get()); LOG(info, "munmap %ld of size %ld", info._id, info._sz); LOG(info, "%ld mappings of accumulated size %ld", _G_HugeMappings.size(), sum(_G_HugeMappings)); } @@ -447,7 +447,7 @@ void MMapAllocator::sfree(PtrAndSize alloc) size_t AutoAllocator::resize_inplace(PtrAndSize current, size_t newSize) const { - if (useMMap(current.second) && useMMap(newSize)) { + if (useMMap(current.size()) && useMMap(newSize)) { newSize = roundUpToHugePages(newSize); return MMapAllocator::sresize_inplace(current, newSize); } else { @@ -455,7 +455,7 @@ AutoAllocator::resize_inplace(PtrAndSize current, size_t newSize) const { } } -MMapAllocator::PtrAndSize +PtrAndSize AutoAllocator::alloc(size_t sz) const { if ( ! useMMap(sz)) { if (_alignment == 0) { @@ -471,7 +471,7 @@ AutoAllocator::alloc(size_t sz) const { void AutoAllocator::free(PtrAndSize alloc) const { - if ( ! isMMapped(alloc.second)) { + if ( ! isMMapped(alloc.size())) { return HeapAllocator::sfree(alloc); } else { return MMapAllocator::sfree(alloc); @@ -513,7 +513,7 @@ Alloc::resize_inplace(size_t newSize) } size_t extendedSize = _allocator->resize_inplace(_alloc, newSize); if (extendedSize >= newSize) { - _alloc.second = extendedSize; + _alloc = PtrAndSize(_alloc.get(), extendedSize); return true; } return false; @@ -571,6 +571,14 @@ Alloc::alloc_with_allocator(const MemoryAllocator* allocator) noexcept return Alloc(allocator); } +PtrAndSize::PtrAndSize(void * ptr, size_t sz) noexcept + : _ptr(ptr), _sz(sz) +{ + constexpr uint8_t MAX_PTR_BITS = 57; + constexpr uint64_t MAX_PTR = 1ul << MAX_PTR_BITS; + assert((uint64_t(ptr) + sz) < MAX_PTR); +} + } } diff --git a/vespalib/src/vespa/vespalib/util/alloc.h b/vespalib/src/vespa/vespalib/util/alloc.h index 4066894b4e3..b78c10dd381 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.h +++ b/vespalib/src/vespa/vespalib/util/alloc.h @@ -15,14 +15,12 @@ namespace vespalib::alloc { **/ class Alloc { -private: - using PtrAndSize = std::pair<void *, size_t>; public: - size_t size() const { return _alloc.second; } - void * get() { return _alloc.first; } - const void * get() const { return _alloc.first; } - void * operator -> () { return _alloc.first; } - const void * operator -> () const { return _alloc.first; } + size_t size() const noexcept { return _alloc.size(); } + void * get() noexcept { return _alloc.get(); } + const void * get() const noexcept { return _alloc.get(); } + void * operator -> () noexcept { return get(); } + const void * operator -> () const noexcept { return get(); } /* * If possible the allocations will be resized. If it was possible it will return true * And you have an area that can be accessed up to the new size. @@ -42,7 +40,7 @@ public: } Alloc & operator=(Alloc && rhs) noexcept { if (this != & rhs) { - if (_alloc.first != nullptr) { + if (_alloc.get() != nullptr) { _allocator->free(_alloc); } _alloc = rhs._alloc; @@ -53,9 +51,9 @@ public: } Alloc() noexcept : _alloc(nullptr, 0), _allocator(nullptr) { } ~Alloc() { - if (_alloc.first != nullptr) { + if (_alloc.get() != nullptr) { _allocator->free(_alloc); - _alloc.first = nullptr; + _alloc = PtrAndSize(); } } void swap(Alloc & rhs) noexcept { @@ -63,10 +61,9 @@ public: std::swap(_allocator, rhs._allocator); } void reset() { - if (_alloc.first != nullptr) { + if (_alloc.get() != nullptr) { _allocator->free(_alloc); - _alloc.first = nullptr; - _alloc.second = 0u; + _alloc = PtrAndSize(); } } Alloc create(size_t sz) const noexcept { @@ -96,8 +93,7 @@ private: _allocator(allocator) { } void clear() { - _alloc.first = nullptr; - _alloc.second = 0; + _alloc = PtrAndSize(); _allocator = nullptr; } PtrAndSize _alloc; diff --git a/vespalib/src/vespa/vespalib/util/memory_allocator.h b/vespalib/src/vespa/vespalib/util/memory_allocator.h index 2bcd7e5889d..e9a494f3e6f 100644 --- a/vespalib/src/vespa/vespalib/util/memory_allocator.h +++ b/vespalib/src/vespa/vespalib/util/memory_allocator.h @@ -8,6 +8,17 @@ namespace vespalib::alloc { +class PtrAndSize { +public: + PtrAndSize() noexcept : _ptr(nullptr), _sz(0ul) {} + PtrAndSize(void * ptr, size_t sz) noexcept; + void * get() const noexcept { return _ptr; } + size_t size() const noexcept { return _sz; } +private: + void * _ptr; + size_t _sz; +}; + /* * Abstract base class for allocating memory at a low level. */ @@ -15,7 +26,6 @@ class MemoryAllocator { public: static constexpr size_t PAGE_SIZE = 4_Ki; static constexpr size_t HUGEPAGE_SIZE = 2_Mi; - using PtrAndSize = std::pair<void *, size_t>; MemoryAllocator(const MemoryAllocator &) = delete; MemoryAllocator & operator = (const MemoryAllocator &) = delete; MemoryAllocator() = default; diff --git a/vespalib/src/vespa/vespalib/util/mmap_file_allocator.cpp b/vespalib/src/vespa/vespalib/util/mmap_file_allocator.cpp index 8c89f6745e4..929b926c03e 100644 --- a/vespalib/src/vespa/vespalib/util/mmap_file_allocator.cpp +++ b/vespalib/src/vespa/vespalib/util/mmap_file_allocator.cpp @@ -42,7 +42,7 @@ MmapFileAllocator::alloc_area(size_t sz) const return offset; } -MmapFileAllocator::PtrAndSize +PtrAndSize MmapFileAllocator::alloc(size_t sz) const { if (sz == 0) { @@ -72,23 +72,23 @@ MmapFileAllocator::alloc(size_t sz) const void MmapFileAllocator::free(PtrAndSize alloc) const { - if (alloc.second == 0) { - assert(alloc.first == nullptr); + if (alloc.size() == 0) { + assert(alloc.get() == nullptr); return; // empty allocation } - assert(alloc.first != nullptr); + assert(alloc.get() != nullptr); // Check that matching allocation is registered - auto itr = _allocations.find(alloc.first); + auto itr = _allocations.find(alloc.get()); assert(itr != _allocations.end()); - assert(itr->first == alloc.first); - assert(itr->second.size == alloc.second); + assert(itr->first == alloc.get()); + assert(itr->second.size == alloc.size()); auto offset = itr->second.offset; _allocations.erase(itr); - int retval = madvise(alloc.first, alloc.second, MADV_DONTNEED); + int retval = madvise(alloc.get(), alloc.size(), MADV_DONTNEED); assert(retval == 0); - retval = munmap(alloc.first, alloc.second); + retval = munmap(alloc.get(), alloc.size()); assert(retval == 0); - _freelist.free(offset, alloc.second); + _freelist.free(offset, alloc.size()); } size_t diff --git a/vespamalloc/src/vespamalloc/malloc/common.h b/vespamalloc/src/vespamalloc/malloc/common.h index 58e05878f64..501b45cd067 100644 --- a/vespamalloc/src/vespamalloc/malloc/common.h +++ b/vespamalloc/src/vespamalloc/malloc/common.h @@ -59,6 +59,8 @@ using OSMemory = MmapMemory; using SizeClassT = int; constexpr size_t ALWAYS_REUSE_LIMIT = 0x100000ul; +constexpr uint8_t MAX_PTR_BITS = 57; // Maximum number of bits a pointer can use (Intel IceLake) +constexpr uint64_t MAX_PTR = 1ul << MAX_PTR_BITS; inline constexpr int msbIdx(uint64_t v) { diff --git a/vespamalloc/src/vespamalloc/malloc/threadproxy.cpp b/vespamalloc/src/vespamalloc/malloc/threadproxy.cpp index 02eb624ee64..4a02d599b63 100644 --- a/vespamalloc/src/vespamalloc/malloc/threadproxy.cpp +++ b/vespamalloc/src/vespamalloc/malloc/threadproxy.cpp @@ -58,6 +58,7 @@ void * mallocThreadProxy (void * arg) vespamalloc::Mutex::addThread(); vespamalloc::_G_myMemP->initThisThread(); void * result = nullptr; + ASSERT_STACKTRACE(uint64_t(&result) < vespamalloc::MAX_PTR); // Sanity check that stack is a legal PTR. DEBUG(fprintf(stderr, "arg(%p=%p), local(%p=%p)\n", &arg, arg, &ta, ta)); pthread_cleanup_push(cleanupThread, ta); diff --git a/vespamalloc/src/vespamalloc/util/osmem.cpp b/vespamalloc/src/vespamalloc/util/osmem.cpp index 0267e091bab..f1d4a527732 100644 --- a/vespamalloc/src/vespamalloc/util/osmem.cpp +++ b/vespamalloc/src/vespamalloc/util/osmem.cpp @@ -168,6 +168,7 @@ MmapMemory::get(size_t len) errno = prevErrno; // The temporary error should not impact if the end is good. memory = getNormalPages(len); } + ASSERT_STACKTRACE((uint64_t(&memory) + len) < vespamalloc::MAX_PTR); return memory; } |