From 1d9ad1fd69fdd015320fc2d6f4439528a2033548 Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@verizonmedia.com>
Date: Mon, 11 May 2020 20:32:46 +0200
Subject: Revert "Use a smart allocator for allocating memory for large 'long'
 lived"

---
 .../vespa/searchlib/attribute/enumattribute.hpp    |  4 ++-
 .../src/vespa/searchlib/attribute/enumstore.h      |  5 ++-
 .../vespa/searchlib/memoryindex/field_inverter.cpp |  4 +--
 .../vespa/searchlib/memoryindex/field_inverter.h   | 30 ++++++++++--------
 vespalib/src/tests/stllike/hash_test.cpp           | 14 ---------
 .../vespalib/datastore/i_unique_store_dictionary.h |  4 +--
 .../vespalib/datastore/unique_store_builder.h      |  5 ++-
 .../vespalib/datastore/unique_store_builder.hpp    |  4 ++-
 .../vespalib/datastore/unique_store_dictionary.h   |  4 +--
 .../vespalib/datastore/unique_store_dictionary.hpp |  8 ++---
 vespalib/src/vespa/vespalib/stllike/allocator.h    | 36 ----------------------
 vespalib/src/vespa/vespalib/util/alloc.cpp         | 26 +++++-----------
 vespalib/src/vespa/vespalib/util/alloc.h           |  5 ---
 vespalib/src/vespa/vespalib/util/arrayref.h        |  8 ++---
 14 files changed, 47 insertions(+), 110 deletions(-)
 delete mode 100644 vespalib/src/vespa/vespalib/stllike/allocator.h

diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp
index 58a220922fd..b527f89b224 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp
@@ -19,7 +19,9 @@ EnumAttribute(const vespalib::string &baseFileName,
 }
 
 template <typename B>
-EnumAttribute<B>::~EnumAttribute() = default;
+EnumAttribute<B>::~EnumAttribute()
+{
+}
 
 template <typename B>
 void EnumAttribute<B>::load_enum_store(LoadedVector& loaded)
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.h b/searchlib/src/vespa/searchlib/attribute/enumstore.h
index 9a0c9c378ed..42738b44461 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.h
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.h
@@ -17,7 +17,6 @@
 #include <vespa/vespalib/datastore/unique_store_string_allocator.h>
 #include <vespa/vespalib/util/buffer.h>
 #include <vespa/vespalib/util/array.h>
-#include <vespa//vespalib/stllike/allocator.h>
 #include <vespa/vespalib/util/stringfmt.h>
 #include <cmath>
 
@@ -115,8 +114,8 @@ public:
     private:
         AllocatorType& _allocator;
         vespalib::datastore::IUniqueStoreDictionary& _dict;
-        std::vector<EntryRef, vespalib::allocator_large<EntryRef>> _refs;
-        std::vector<uint32_t, vespalib::allocator_large<uint32_t>> _payloads;
+        std::vector<EntryRef> _refs;
+        std::vector<uint32_t> _payloads;
 
     public:
         NonEnumeratedLoader(AllocatorType& allocator, vespalib::datastore::IUniqueStoreDictionary& dict)
diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp
index f14cac54980..d68fae69a52 100644
--- a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp
+++ b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.cpp
@@ -197,8 +197,8 @@ FieldInverter::sortWords()
     // Populate word numbers in word buffer and mapping from
     // word numbers to word reference.
     // TODO: shrink word buffer to only contain unique words
-    auto w(_wordRefs.begin() + 1);
-    auto we(_wordRefs.end());
+    std::vector<uint32_t>::const_iterator w(_wordRefs.begin() + 1);
+    std::vector<uint32_t>::const_iterator we(_wordRefs.end());
     uint32_t wordNum = 1;   // First valid word number
     const char *lastWord = getWordFromRef(*w);
     updateWordNum(*w, wordNum);
diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h
index 78a1cf6c171..1c074b6ee77 100644
--- a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h
+++ b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h
@@ -9,9 +9,9 @@
 #include <vespa/searchlib/bitcompression/compression.h>
 #include <vespa/searchlib/bitcompression/posocccompression.h>
 #include <vespa/searchlib/index/docidandfeatures.h>
-#include <vespa/vespalib/stllike/allocator.h>
 #include <limits>
 #include <map>
+#include <set>
 
 namespace search::index { class FieldLengthCalculator; }
 
@@ -115,8 +115,8 @@ private:
         void set_field_length(uint32_t field_length) { _field_length = field_length; }
     };
 
-    using ElemInfoVec = std::vector<ElemInfo, vespalib::allocator_large<ElemInfo>>;
-    using PosInfoVec = std::vector<PosInfo, vespalib::allocator_large<PosInfo>>;
+    using ElemInfoVec = std::vector<ElemInfo>;
+    using PosInfoVec = std::vector<PosInfo>;
 
     class CompareWordRef {
         const char *const _wordBuffer;
@@ -161,7 +161,6 @@ private:
         uint32_t getLen() const   { return _len; }
     };
 
-    using UInt32Vector = std::vector<uint32_t, vespalib::allocator_large<uint32_t>>;
     // Current field state.
     uint32_t                       _fieldId;   // current field id
     uint32_t                       _elem;      // current element
@@ -175,8 +174,8 @@ private:
     ElemInfoVec                    _elems;
     PosInfoVec                     _positions;
     index::DocIdAndPosOccFeatures  _features;
-    UInt32Vector                   _elementWordRefs;
-    UInt32Vector                   _wordRefs;
+    std::vector<uint32_t>          _elementWordRefs;
+    std::vector<uint32_t>          _wordRefs;
 
     using SpanTerm = std::pair<document::Span, const document::FieldValue *>;
     using SpanTermVector = std::vector<SpanTerm>;
@@ -185,16 +184,18 @@ private:
     // Info about aborted and pending documents.
     std::vector<PositionRange>        _abortedDocs;
     std::map<uint32_t, PositionRange> _pendingDocs;
-    UInt32Vector                      _removeDocs;
+    std::vector<uint32_t>             _removeDocs;
 
     FieldIndexRemover                &_remover;
     IOrderedFieldIndexInserter       &_inserter;
     index::FieldLengthCalculator     &_calculator;
 
-    void invertNormalDocTextField(const document::FieldValue &val);
+    void
+    invertNormalDocTextField(const document::FieldValue &val);
 
 public:
     void startElement(int32_t weight);
+
     void endElement();
 
 private:
@@ -252,9 +253,14 @@ public:
     processAnnotations(const document::StringFieldValue &value);
 
 private:
-    void processNormalDocTextField(const document::StringFieldValue &field);
-    void processNormalDocArrayTextField(const document::ArrayFieldValue &field);
-    void processNormalDocWeightedSetTextField(const document::WeightedSetFieldValue &field);
+    void
+    processNormalDocTextField(const document::StringFieldValue &field);
+
+    void
+    processNormalDocArrayTextField(const document::ArrayFieldValue &field);
+
+    void
+    processNormalDocWeightedSetTextField(const document::WeightedSetFieldValue &field);
 
     const index::Schema &getSchema() const { return _schema; }
 
@@ -307,7 +313,7 @@ public:
     /**
      * Setup remove of word in old version of document.
      */
-    void remove(const vespalib::stringref word, uint32_t docId) override;
+    virtual void remove(const vespalib::stringref word, uint32_t docId) override;
 
     void removeDocument(uint32_t docId) {
         abortPendingDoc(docId);
diff --git a/vespalib/src/tests/stllike/hash_test.cpp b/vespalib/src/tests/stllike/hash_test.cpp
index e86a9ad020a..d23c2c6b68c 100644
--- a/vespalib/src/tests/stllike/hash_test.cpp
+++ b/vespalib/src/tests/stllike/hash_test.cpp
@@ -4,7 +4,6 @@
 #include <vespa/vespalib/stllike/hash_set.hpp>
 #include <vespa/vespalib/stllike/hash_map.hpp>
 #include <vespa/vespalib/stllike/hash_map_equal.hpp>
-#include <vespa/vespalib/stllike/allocator.h>
 #include <cstddef>
 #include <algorithm>
 
@@ -554,17 +553,4 @@ TEST("test that begin and end are identical with empty hashtables") {
     EXPECT_TRUE(empty_but_reserved.begin() == empty_but_reserved.end());
 }
 
-TEST ("test that large_allocator works fine with std::vector") {
-    using V = std::vector<uint64_t, allocator_large<uint64_t>>;
-    V a;
-    a.push_back(1);
-    a.reserve(14);
-    for (size_t i(0); i < 400000; i++) {
-        a.push_back(i);
-    }
-    V b = std::move(a);
-    V c = b;
-    ASSERT_EQUAL(b.size(), c.size());
-}
-
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h
index 2c495ca9a1e..6b9a4f7201e 100644
--- a/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h
+++ b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h
@@ -44,9 +44,9 @@ public:
     virtual void move_entries(ICompactable& compactable) = 0;
     virtual uint32_t get_num_uniques() const = 0;
     virtual vespalib::MemoryUsage get_memory_usage() const = 0;
-    virtual void build(vespalib::ConstArrayRef<EntryRef>, vespalib::ConstArrayRef<uint32_t> ref_counts, std::function<void(EntryRef)> hold) = 0;
+    virtual void build(const std::vector<EntryRef> &refs, const std::vector<uint32_t> &ref_counts, std::function<void(EntryRef)> hold) = 0;
     virtual void build(vespalib::ConstArrayRef<EntryRef> refs) = 0;
-    virtual void build_with_payload(vespalib::ConstArrayRef<EntryRef> refs, vespalib::ConstArrayRef<uint32_t> payloads) = 0;
+    virtual void build_with_payload(const std::vector<EntryRef>& refs, const std::vector<uint32_t>& payloads) = 0;
     virtual std::unique_ptr<ReadSnapshot> get_read_snapshot() const = 0;
 };
 
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h
index cbafdbafd4f..f2be4e1237b 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.h
@@ -3,7 +3,6 @@
 #pragma once
 
 #include "unique_store_allocator.h"
-#include <vespa/vespalib/stllike/allocator.h>
 
 namespace vespalib::datastore {
 
@@ -22,8 +21,8 @@ class UniqueStoreBuilder {
 
     Allocator& _allocator;
     IUniqueStoreDictionary& _dict;
-    std::vector<EntryRef, allocator_large<EntryRef>> _refs;
-    std::vector<uint32_t, allocator_large<uint32_t>> _refCounts;
+    std::vector<EntryRef> _refs;
+    std::vector<uint32_t> _refCounts;
 
 public:
     UniqueStoreBuilder(Allocator& allocator, IUniqueStoreDictionary& dict, uint32_t uniqueValuesHint);
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp
index d54b86558be..0925702ddcb 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_builder.hpp
@@ -20,7 +20,9 @@ UniqueStoreBuilder<Allocator>::UniqueStoreBuilder(Allocator& allocator, IUniqueS
 }
 
 template <typename Allocator>
-UniqueStoreBuilder<Allocator>::~UniqueStoreBuilder() = default;
+UniqueStoreBuilder<Allocator>::~UniqueStoreBuilder()
+{
+}
 
 template <typename Allocator>
 void
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h
index 8f7a344a99f..4ec98dc2100 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h
@@ -46,9 +46,9 @@ public:
     void move_entries(ICompactable& compactable) override;
     uint32_t get_num_uniques() const override;
     vespalib::MemoryUsage get_memory_usage() const override;
-    void build(vespalib::ConstArrayRef<EntryRef>, vespalib::ConstArrayRef<uint32_t> ref_counts, std::function<void(EntryRef)> hold) override;
+    void build(const std::vector<EntryRef> &refs, const std::vector<uint32_t> &ref_counts, std::function<void(EntryRef)> hold) override;
     void build(vespalib::ConstArrayRef<EntryRef> refs) override;
-    void build_with_payload(vespalib::ConstArrayRef<EntryRef>, vespalib::ConstArrayRef<uint32_t> payloads) override;
+    void build_with_payload(const std::vector<EntryRef>& refs, const std::vector<uint32_t>& payloads) override;
     std::unique_ptr<ReadSnapshot> get_read_snapshot() const override;
 };
 
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp
index 8ecf71d08c7..bce11591970 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp
@@ -158,8 +158,8 @@ UniqueStoreDictionary<DictionaryT, ParentT>::get_memory_usage() const
 
 template <typename DictionaryT, typename ParentT>
 void
-UniqueStoreDictionary<DictionaryT, ParentT>::build(vespalib::ConstArrayRef<EntryRef> refs,
-                                                   vespalib::ConstArrayRef<uint32_t> ref_counts,
+UniqueStoreDictionary<DictionaryT, ParentT>::build(const std::vector<EntryRef> &refs,
+                                                   const std::vector<uint32_t> &ref_counts,
                                                    std::function<void(EntryRef)> hold)
 {
     assert(refs.size() == ref_counts.size());
@@ -188,8 +188,8 @@ UniqueStoreDictionary<DictionaryT, ParentT>::build(vespalib::ConstArrayRef<Entry
 
 template <typename DictionaryT, typename ParentT>
 void
-UniqueStoreDictionary<DictionaryT, ParentT>::build_with_payload(vespalib::ConstArrayRef<EntryRef> refs,
-                                                                vespalib::ConstArrayRef<uint32_t> payloads)
+UniqueStoreDictionary<DictionaryT, ParentT>::build_with_payload(const std::vector<EntryRef>& refs,
+                                                                const std::vector<uint32_t>& payloads)
 {
     assert(refs.size() == payloads.size());
     typename DictionaryType::Builder builder(_dict.getAllocator());
diff --git a/vespalib/src/vespa/vespalib/stllike/allocator.h b/vespalib/src/vespa/vespalib/stllike/allocator.h
deleted file mode 100644
index 3d8c2f03154..00000000000
--- a/vespalib/src/vespa/vespalib/stllike/allocator.h
+++ /dev/null
@@ -1,36 +0,0 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <vespa/vespalib/util/alloc.h>
-
-namespace vespalib {
-
-/**
- * std compliant allocator that will use a smart allocator
- * that uses mmap prefering huge pages for large allocations.
- * This is a good fit for use with std::vector and std::deque.
- */
-template <typename T>
-class allocator_large {
-    using PtrAndSize = alloc::MemoryAllocator::PtrAndSize;
-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);
-    }
-    void deallocate(T * p, std::size_t n) {
-        _allocator->free(p, n*sizeof(T));
-    }
-    alloc::MemoryAllocator * allocator() const { return _allocator; }
-private:
-    const alloc::MemoryAllocator * _allocator;
-};
-
-template< class T1, class T2 >
-constexpr bool operator==( const allocator_large<T1>& lhs, const allocator_large<T2>& rhs ) noexcept {
-    return lhs.allocator() == rhs.allocator();
-}
-
-}
diff --git a/vespalib/src/vespa/vespalib/util/alloc.cpp b/vespalib/src/vespa/vespalib/util/alloc.cpp
index c550964a19b..4c0675198bb 100644
--- a/vespalib/src/vespa/vespalib/util/alloc.cpp
+++ b/vespalib/src/vespa/vespalib/util/alloc.cpp
@@ -162,7 +162,6 @@ public:
     AutoAllocator(size_t mmapLimit, size_t alignment) : _mmapLimit(mmapLimit), _alignment(alignment) { }
     PtrAndSize alloc(size_t sz) const override;
     void free(PtrAndSize alloc) const override;
-    void free(void * ptr, size_t sz) const override;
     size_t resize_inplace(PtrAndSize current, size_t newSize) const override;
     static MemoryAllocator & getDefault();
     static MemoryAllocator & getAllocator(size_t mmapLimit, size_t alignment);
@@ -172,11 +171,13 @@ private:
             ? MMapAllocator::roundUpToHugePages(sz)
             : sz;
     }
+    bool isMMapped(size_t sz) const { return (sz >= _mmapLimit); }
     bool useMMap(size_t sz) const {
-        return (sz + (HUGEPAGE_SIZE >> 1) - 1) >= _mmapLimit;
-    }
-    bool isMMapped(size_t sz) const {
-        return sz >= _mmapLimit;
+        if (_mmapLimit >= HUGEPAGE_SIZE) {
+            return (sz + (HUGEPAGE_SIZE >> 1) - 1) >= _mmapLimit;
+        } else {
+            return (sz >= _mmapLimit);
+        }
     }
     size_t _mmapLimit;
     size_t _alignment;
@@ -423,7 +424,7 @@ void MMapAllocator::sfree(PtrAndSize alloc)
 
 size_t
 AutoAllocator::resize_inplace(PtrAndSize current, size_t newSize) const {
-    if (useMMap(current.second) && useMMap(newSize)) {
+    if (isMMapped(current.second) && useMMap(newSize)) {
         newSize = roundUpToHugePages(newSize);
         return MMapAllocator::sresize_inplace(current, newSize);
     } else {
@@ -454,21 +455,8 @@ AutoAllocator::free(PtrAndSize alloc) const {
     }
 }
 
-void
-AutoAllocator::free(void * ptr, size_t sz) const {
-    if (useMMap(sz)) {
-        return MMapAllocator::sfree(PtrAndSize(ptr, roundUpToHugePages(sz)));
-    } else {
-        return HeapAllocator::sfree(PtrAndSize(ptr, sz));
-    }
-}
-
 }
 
-const MemoryAllocator *
-MemoryAllocator::select_allocator(size_t mmapLimit, size_t alignment) {
-    return & AutoAllocator::getAllocator(mmapLimit, alignment);
-}
 
 Alloc
 Alloc::allocHeap(size_t sz)
diff --git a/vespalib/src/vespa/vespalib/util/alloc.h b/vespalib/src/vespa/vespalib/util/alloc.h
index 449cdde5fc7..03ebc2807f9 100644
--- a/vespalib/src/vespa/vespalib/util/alloc.h
+++ b/vespalib/src/vespa/vespalib/util/alloc.h
@@ -17,10 +17,6 @@ public:
     virtual ~MemoryAllocator() { }
     virtual PtrAndSize alloc(size_t sz) const = 0;
     virtual void free(PtrAndSize alloc) const = 0;
-    // Allow for freeing memory there size is the size requested, and not the size allocated.
-    virtual void free(void * ptr, size_t sz) const {
-        free(PtrAndSize(ptr, sz));
-    }
     /*
      * If possible the allocations will be resized. If it was possible it will return the real size,
      * if not it shall return 0.
@@ -34,7 +30,6 @@ public:
     static size_t roundUpToHugePages(size_t sz) {
         return (sz+(HUGEPAGE_SIZE-1)) & ~(HUGEPAGE_SIZE-1);
     }
-    static const MemoryAllocator * select_allocator(size_t mmapLimit = MemoryAllocator::HUGEPAGE_SIZE, size_t alignment=0);
 };
 
 /**
diff --git a/vespalib/src/vespa/vespalib/util/arrayref.h b/vespalib/src/vespa/vespalib/util/arrayref.h
index 749395ff574..186316c10d3 100644
--- a/vespalib/src/vespa/vespalib/util/arrayref.h
+++ b/vespalib/src/vespa/vespalib/util/arrayref.h
@@ -15,13 +15,11 @@ class ArrayRef {
 public:
     ArrayRef() : _v(nullptr), _sz(0) { }
     ArrayRef(T * v, size_t sz) : _v(v), _sz(sz) { }
-    template<typename A=std::allocator<T>>
-    ArrayRef(std::vector<T, A> & v) : _v(&v[0]), _sz(v.size()) { }
+    ArrayRef(std::vector<T> & v) : _v(&v[0]), _sz(v.size()) { }
     ArrayRef(Array<T> &v) : _v(&v[0]), _sz(v.size()) { }
     T & operator [] (size_t i) { return _v[i]; }
     const T & operator [] (size_t i) const { return _v[i]; }
     size_t size() const { return _sz; }
-    bool empty() const { return _sz == 0; }
     T *begin() { return _v; }
     T *end() { return _v + _sz; }
 private:
@@ -33,14 +31,12 @@ template <typename T>
 class ConstArrayRef {
 public:
     ConstArrayRef(const T *v, size_t sz) : _v(v), _sz(sz) { }
-    template<typename A=std::allocator<T>>
-    ConstArrayRef(const std::vector<T, A> & v) : _v(&v[0]), _sz(v.size()) { }
+    ConstArrayRef(const std::vector<T> & v) : _v(&v[0]), _sz(v.size()) { }
     ConstArrayRef(const ArrayRef<T> & v) : _v(&v[0]), _sz(v.size()) { }
     ConstArrayRef(const Array<T> &v) : _v(&v[0]), _sz(v.size()) { }
     ConstArrayRef() : _v(nullptr), _sz(0) {}
     const T & operator [] (size_t i) const { return _v[i]; }
     size_t size() const { return _sz; }
-    bool empty() const { return _sz == 0; }
     const T *cbegin() const { return _v; }
     const T *cend() const { return _v + _sz; }
     const T *begin() const { return _v; }
-- 
cgit v1.2.3