- Use stash instead of the single use of VariableSizeVector.

12 files changed, 61 insertions, 217 deletions

diff --git a/vespalib/src/tests/memorydatastore/memorydatastore.cpp b/vespalib/src/tests/memorydatastore/memorydatastore.cpp
index 1d49b0af91b..649bd45a541 100644
--- a/vespalib/src/tests/memorydatastore/memorydatastore.cpp
+++ b/vespalib/src/tests/memorydatastore/memorydatastore.cpp
@@ -6,17 +6,7 @@
 
 using namespace vespalib;
 
-class MemoryDataStoreTest : public vespalib::TestApp
-{
-private:
-    void testMemoryDataStore();
-    void testVariableSizeVector();
-public:
-    int Main() override;
-};
-
-void
-MemoryDataStoreTest::testMemoryDataStore()
+TEST("testMemoryDataStore")
 {
     MemoryDataStore s(alloc::Alloc::alloc(256));
     std::vector<MemoryDataStore::Reference> v;
@@ -28,45 +18,9 @@ MemoryDataStoreTest::testMemoryDataStore()
     v.push_back(s.push_back("mumbo", 5));
     EXPECT_EQUAL(52ul, v.size());
     EXPECT_NOT_EQUAL(static_cast<const char *>(v[50].data()) + 5, v[51].data());
-    for (size_t i(0); i < v.size(); i++) {
-        EXPECT_EQUAL(0, memcmp("mumbo", v[i].data(), 5));
+    for (auto & i : v) {
+        EXPECT_EQUAL(0, memcmp("mumbo", i.data(), 5));
     }
 }
 
-void
-MemoryDataStoreTest::testVariableSizeVector()
-{
-    VariableSizeVector v(20000, 5*20000);
-    for (size_t i(0); i < 10000; i++) {
-        asciistream os;
-        os << i;
-        v.push_back(os.str().data(), os.str().size());
-    }
-    for (size_t i(0); i < v.size(); i++) {
-        asciistream os;
-        os << i;
-        EXPECT_EQUAL(os.str().size(), v[i].size());
-        EXPECT_EQUAL(0, memcmp(os.str().data(), v[i].data(), os.str().size()));
-    }
-    size_t i(0);
-    for (auto it(v.begin()), mt(v.end()); it != mt; it++, i++) {
-        asciistream os;
-        os << i;
-        EXPECT_EQUAL(os.str().size(), it->size());
-        EXPECT_EQUAL(0, memcmp(os.str().data(), (*it).data(), os.str().size()));
-    }
-    
-}
-
-int
-MemoryDataStoreTest::Main()
-{
-    TEST_INIT("data_test");
-    testMemoryDataStore();
-    testVariableSizeVector();
-
-    TEST_DONE();
-}
-
-TEST_APPHOOK(MemoryDataStoreTest);
-
+TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/tests/stllike/hash_test.cpp b/vespalib/src/tests/stllike/hash_test.cpp
index ae27d2dc58b..dc6e48100a9 100644
--- a/vespalib/src/tests/stllike/hash_test.cpp
+++ b/vespalib/src/tests/stllike/hash_test.cpp
@@ -494,6 +494,14 @@ TEST("test hash set initializer list - empty")
     EXPECT_EQUAL(0u, s.size());
 }
 
+TEST("empty hash_set can be looked up")
+{
+    IntHashSet s;
+    EXPECT_EQUAL(0u, s.size());
+    EXPECT_EQUAL(1u, s.capacity());
+    EXPECT_TRUE(s.find(1) == s.end());
+}
+
 TEST("test hash set initializer list - 1 element")
 {
     IntHashSet s = {1};
diff --git a/vespalib/src/vespa/vespalib/data/memorydatastore.cpp b/vespalib/src/vespa/vespalib/data/memorydatastore.cpp
index 354787690c2..6d483e6ff4e 100644
--- a/vespalib/src/vespa/vespalib/data/memorydatastore.cpp
+++ b/vespalib/src/vespa/vespalib/data/memorydatastore.cpp
@@ -41,21 +41,4 @@ MemoryDataStore::push_back(const void * data, const size_t sz)
     return ref;
 }
 
-VariableSizeVector::VariableSizeVector(size_t initialCount, size_t initialBufferSize)
-    : _vector(),
-      _store(Alloc::alloc(initialBufferSize))
-{
-    _vector.reserve(initialCount);
-}
-
-VariableSizeVector::~VariableSizeVector() = default;
-
-VariableSizeVector::Reference
-VariableSizeVector::push_back(const void * data, const size_t sz)
-{
-    MemoryDataStore::Reference ptr(_store.push_back(data, sz));
-    _vector.push_back(Reference(ptr.data(), sz));
-    return _vector.back();
-}
-
 } // namespace vespalib
diff --git a/vespalib/src/vespa/vespalib/data/memorydatastore.h b/vespalib/src/vespa/vespalib/data/memorydatastore.h
index 7022eb88051..a0280454a91 100644
--- a/vespalib/src/vespa/vespalib/data/memorydatastore.h
+++ b/vespalib/src/vespa/vespalib/data/memorydatastore.h
@@ -44,83 +44,5 @@ private:
     std::mutex              * _lock;
 };
 
-class VariableSizeVector
-{
-public:
-    class Reference {
-    public:
-        Reference(void * data_, size_t sz) noexcept : _data(data_), _sz(sz) { }
-        void * data() noexcept { return _data; }
-        const char * c_str() const noexcept { return static_cast<const char *>(_data); }
-        size_t size() const noexcept { return _sz; }
-    private:
-        void   * _data;
-        size_t   _sz;
-    };
-    class iterator {
-    public:
-        iterator(vespalib::Array<Reference> & v, size_t index) noexcept : _vector(&v), _index(index) {}
-        Reference & operator  * () const noexcept { return (*_vector)[_index]; }
-        Reference * operator -> () const noexcept { return &(*_vector)[_index]; }
-        iterator & operator ++ () noexcept {
-            _index++;
-            return *this;
-        }
-        iterator operator ++ (int) noexcept {
-            iterator prev = *this;
-            ++(*this);
-            return prev;
-        }
-        bool operator==(const iterator& rhs) const noexcept { return (_index == rhs._index); }
-        bool operator!=(const iterator& rhs) const noexcept { return (_index != rhs._index); }
-    private:
-        vespalib::Array<Reference> * _vector;
-        size_t _index;
-    };
-    class const_iterator {
-    public:
-        const_iterator(const vespalib::Array<Reference> & v, size_t index) noexcept : _vector(&v), _index(index) {}
-        const Reference & operator  * () const noexcept { return (*_vector)[_index]; }
-        const Reference * operator -> () const noexcept { return &(*_vector)[_index]; }
-        const_iterator & operator ++ () noexcept {
-            _index++;
-            return *this;
-        }
-        const_iterator operator ++ (int) noexcept {
-            const_iterator prev = *this;
-            ++(*this);
-            return prev;
-        }
-        bool operator==(const const_iterator& rhs) const noexcept { return (_index == rhs._index); }
-        bool operator!=(const const_iterator& rhs) const noexcept { return (_index != rhs._index); }
-    private:
-        const vespalib::Array<Reference> * _vector;
-        size_t _index;
-    };
-    VariableSizeVector(const VariableSizeVector &) = delete;
-    VariableSizeVector & operator = (const VariableSizeVector &) = delete;
-    VariableSizeVector(size_t initialCount, size_t initialBufferSize);
-    ~VariableSizeVector();
-    iterator begin() noexcept { return iterator(_vector, 0); }
-    iterator end() noexcept { return iterator(_vector, size()); }
-    const_iterator begin() const noexcept { return const_iterator(_vector, 0); }
-    const_iterator end() const noexcept { return const_iterator(_vector, size()); }
-    Reference push_back(const void * data, const size_t sz);
-    Reference operator [] (uint32_t index) const noexcept { return _vector[index]; }
-    size_t size() const noexcept { return _vector.size(); }
-    bool empty() const noexcept { return _vector.empty(); }
-    void swap(VariableSizeVector & rhs) noexcept {
-        _vector.swap(rhs._vector);
-        _store.swap(rhs._store);
-    }
-    void clear() {
-        _vector.clear();
-        _store.clear();
-    }
-private:
-    vespalib::Array<Reference> _vector;
-    MemoryDataStore _store;
-};
-
 } // namespace vespalib
 
diff --git a/vespalib/src/vespa/vespalib/data/slime/named_symbol_lookup.h b/vespalib/src/vespa/vespalib/data/slime/named_symbol_lookup.h
index 44dbf05c9da..fb4bc3943ae 100644
--- a/vespalib/src/vespa/vespalib/data/slime/named_symbol_lookup.h
+++ b/vespalib/src/vespa/vespalib/data/slime/named_symbol_lookup.h
@@ -20,7 +20,7 @@ private:
     const Memory &_name;
 
 public:
-    NamedSymbolLookup(const SymbolTable &table, const Memory &name)
+    NamedSymbolLookup(const SymbolTable &table, const Memory &name) noexcept
         : _table(table), _name(name) {}
     Symbol lookup() const override;
 };
diff --git a/vespalib/src/vespa/vespalib/data/slime/slime.cpp b/vespalib/src/vespa/vespalib/data/slime/slime.cpp
index d6fac44b360..5a9ec54584d 100644
--- a/vespalib/src/vespa/vespalib/data/slime/slime.cpp
+++ b/vespalib/src/vespa/vespalib/data/slime/slime.cpp
@@ -6,16 +6,6 @@
 
 namespace vespalib {
 
-Slime::Params::Params() : Params(std::make_unique<SymbolTable>()) { }
-Slime::Params::Params(std::unique_ptr<SymbolTable> symbols) noexcept : _symbols(std::move(symbols)), _chunkSize(4096) { }
-Slime::Params::Params(Params &&) noexcept = default;
-Slime::Params::~Params() = default;
-
-std::unique_ptr<slime::SymbolTable>
-Slime::Params::detachSymbols() {
-    return std::move(_symbols);
-}
-
 Slime::Slime(Params params)
     : _names(params.detachSymbols()),
       _stash(std::make_unique<Stash>(params.getChunkSize())),
@@ -33,26 +23,6 @@ Slime::reclaimSymbols(Slime &&rhs) {
     return std::move(rhs._names);
 }
 
-size_t
-Slime::symbols() const noexcept {
-    return _names->symbols();
-}
-
-Memory
-Slime::inspect(Symbol symbol) const {
-    return _names->inspect(symbol);
-}
-
-slime::Symbol
-Slime::insert(Memory name) {
-    return _names->insert(name);
-}
-
-slime::Symbol
-Slime::lookup(Memory name) const {
-    return _names->lookup(name);
-}
-
 bool operator == (const Slime & a, const Slime & b) noexcept
 {
     return a.get() == b.get();
diff --git a/vespalib/src/vespa/vespalib/data/slime/slime.h b/vespalib/src/vespa/vespalib/data/slime/slime.h
index a426f906563..8cfd6d52218 100644
--- a/vespalib/src/vespa/vespalib/data/slime/slime.h
+++ b/vespalib/src/vespa/vespalib/data/slime/slime.h
@@ -21,6 +21,7 @@
 #include "symbol.h"
 #include "symbol_inserter.h"
 #include "symbol_lookup.h"
+#include "symbol_table.h"
 #include "type.h"
 #include "value.h"
 #include "value_factory.h"
@@ -51,32 +52,30 @@ private:
     using Cursor = slime::Cursor;
     using Inspector = slime::Inspector;
 
-    std::unique_ptr<SymbolTable>   _names;
-    std::unique_ptr<Stash>         _stash;
-    RootValue                      _root;
+    std::unique_ptr<SymbolTable> _names;
+    std::unique_ptr<Stash>       _stash;
+    RootValue                    _root;
 
 public:
     using UP = std::unique_ptr<Slime>;
     class Params {
     private:
-        std::unique_ptr<SymbolTable>  _symbols;
-        size_t                        _chunkSize;
+        std::unique_ptr<SymbolTable> _symbols;
+        size_t                       _chunkSize;
     public:
-        Params();
-        explicit Params(std::unique_ptr<SymbolTable> symbols) noexcept;
-        Params(Params &&) noexcept;
-        ~Params();
-        Params & setChunkSize(size_t chunkSize) {
-            _chunkSize = chunkSize;
-            return *this;
-        }
-        size_t getChunkSize() const { return _chunkSize; }
-        std::unique_ptr<SymbolTable> detachSymbols();
+        Params() noexcept : Params(4096) {}
+        explicit Params(size_t chunkSize) noexcept : _symbols(std::make_unique<SymbolTable>()), _chunkSize(chunkSize) {}
+        explicit Params(std::unique_ptr<SymbolTable> symbols) noexcept : _symbols(std::move(symbols)), _chunkSize(4096) {}
+        Params(Params &&) noexcept = default;
+        ~Params() = default;
+        size_t getChunkSize() const noexcept { return _chunkSize; }
+        std::unique_ptr<SymbolTable> detachSymbols() noexcept { return std::move(_symbols); }
     };
     /**
      * Construct an initially empty Slime object.
      **/
-    explicit Slime(Params params = Params());
+    explicit Slime() : Slime(Params()) {}
+    explicit Slime(Params params);
 
     ~Slime();
 
@@ -88,13 +87,13 @@ public:
 
     static std::unique_ptr<SymbolTable> reclaimSymbols(Slime &&rhs);
 
-    size_t symbols() const noexcept;
+    size_t symbols() const noexcept { return _names->symbols(); }
 
-    Memory inspect(Symbol symbol) const;
+    Memory inspect(Symbol symbol) const { return _names->inspect(symbol); }
 
-    Symbol insert(Memory name);
+    Symbol insert(Memory name) { return _names->insert(name); }
 
-    Symbol lookup(Memory name) const;
+    Symbol lookup(Memory name) const { return _names->lookup(name); }
 
     Cursor &get() noexcept { return _root.get(); }
 
diff --git a/vespalib/src/vespa/vespalib/data/slime/symbol.h b/vespalib/src/vespa/vespalib/data/slime/symbol.h
index 3bce727fad9..a60a49fda27 100644
--- a/vespalib/src/vespa/vespalib/data/slime/symbol.h
+++ b/vespalib/src/vespa/vespalib/data/slime/symbol.h
@@ -19,8 +19,8 @@ private:
 public:
     Symbol() noexcept : _value(UNDEFINED) {}
     Symbol(uint32_t v) noexcept : _value(v) {}
-    bool undefined() const { return (_value == UNDEFINED); }
-    uint32_t getValue() const { return _value; }
+    bool undefined() const noexcept { return (_value == UNDEFINED); }
+    uint32_t getValue() const noexcept { return _value; }
     bool operator<(const Symbol &rhs) const noexcept { return (_value < rhs._value); }
     bool operator==(const Symbol &rhs) const noexcept { return (_value == rhs._value); }
 };
diff --git a/vespalib/src/vespa/vespalib/data/slime/symbol_table.cpp b/vespalib/src/vespa/vespalib/data/slime/symbol_table.cpp
index a3313516c64..3fc0c965230 100644
--- a/vespalib/src/vespa/vespalib/data/slime/symbol_table.cpp
+++ b/vespalib/src/vespa/vespalib/data/slime/symbol_table.cpp
@@ -5,10 +5,13 @@
 
 namespace vespalib::slime {
 
-SymbolTable::SymbolTable(size_t expectedNumSymbols) :
-    _symbols(3*expectedNumSymbols),
-    _names(expectedNumSymbols, expectedNumSymbols*16)
-{ }
+SymbolTable::SymbolTable(size_t expectedNumSymbols)
+    : _symbols(3*expectedNumSymbols),
+      _names(),
+      _stash()
+{
+    _names.reserve(expectedNumSymbols);
+}
 
 SymbolTable::~SymbolTable() = default;
 
@@ -16,6 +19,7 @@ void
 SymbolTable::clear() {
     _names.clear();
     _symbols.clear();
+    _stash.clear();
 }
 
 Symbol
@@ -23,8 +27,11 @@ SymbolTable::insert(const Memory &name) {
     SymbolMap::const_iterator pos = _symbols.find(name);
     if (pos == _symbols.end()) {
         Symbol symbol(_names.size());
-        SymbolVector::Reference r(_names.push_back(name.data, name.size));
-        _symbols.insert(std::make_pair(Memory(r.c_str(), r.size()), symbol));
+        char *buf = _stash.alloc(name.size);
+        memcpy(buf, name.data, name.size);
+        Memory backed(buf, name.size);
+        _names.push_back(backed);
+        _symbols.insert(std::make_pair(backed, symbol));
         return symbol;
     }
     return pos->second;
@@ -33,7 +40,7 @@ Symbol
 SymbolTable::lookup(const Memory &name) const {
     SymbolMap::const_iterator pos = _symbols.find(name);
     if (pos == _symbols.end()) {
-        return Symbol();
+        return {};
     }
     return pos->second;
 }
diff --git a/vespalib/src/vespa/vespalib/data/slime/symbol_table.h b/vespalib/src/vespa/vespalib/data/slime/symbol_table.h
index c5f3cf12fd6..67b05bc293b 100644
--- a/vespalib/src/vespa/vespalib/data/slime/symbol_table.h
+++ b/vespalib/src/vespa/vespalib/data/slime/symbol_table.h
@@ -4,8 +4,8 @@
 
 #include "symbol.h"
 #include <vespa/vespalib/data/memory.h>
+#include <vespa/vespalib/util/stash.h>
 #include <vespa/vespalib/stllike/hash_map.h>
-#include <vespa/vespalib/data/memorydatastore.h>
 
 namespace vespalib::slime {
 
@@ -21,21 +21,23 @@ private:
         }
     };
     using SymbolMap = hash_map<Memory, Symbol, hasher>;
-    using SymbolVector = VariableSizeVector;
+    using SymbolVector = std::vector<Memory>;
     SymbolMap    _symbols;
     SymbolVector _names;
+    Stash        _stash;
 
 public:
     using UP = std::unique_ptr<SymbolTable>;
     SymbolTable(size_t expectedNumSymbols=16);
+    SymbolTable(SymbolTable &&) noexcept = default;
+    SymbolTable & operator=(SymbolTable &&) noexcept = default;
     ~SymbolTable();
     size_t symbols() const noexcept { return _names.size(); }
     Memory inspect(const Symbol &symbol) const {
         if (symbol.getValue() > _names.size()) {
             return Memory();
         }
-        SymbolVector::Reference r(_names[symbol.getValue()]);
-        return Memory(r.c_str(), r.size());
+        return _names[symbol.getValue()];
     }
     Symbol insert(const Memory &name);
     Symbol lookup(const Memory &name) const;
diff --git a/vespalib/src/vespa/vespalib/stllike/hashtable.h b/vespalib/src/vespa/vespalib/stllike/hashtable.h
index e290d2f626c..fa88bb038b4 100644
--- a/vespalib/src/vespa/vespalib/stllike/hashtable.h
+++ b/vespalib/src/vespa/vespalib/stllike/hashtable.h
@@ -62,7 +62,7 @@ public:
     class prime_modulator
     {
     public:
-        prime_modulator(next_t sizeOfHashTable) noexcept : _modulo(sizeOfHashTable) { }
+        explicit prime_modulator(next_t sizeOfHashTable) noexcept : _modulo(sizeOfHashTable) { }
         next_t modulo(next_t hash) const noexcept { return hash % _modulo; }
         next_t getTableSize() const noexcept { return _modulo; }
         static next_t selectHashTableSize(size_t sz) { return hashtable_base::getModuloStl(sz); }
@@ -76,7 +76,7 @@ public:
     class and_modulator
     {
     public:
-        and_modulator(next_t sizeOfHashTable) noexcept : _mask(sizeOfHashTable-1) { }
+        explicit and_modulator(next_t sizeOfHashTable) noexcept : _mask(sizeOfHashTable-1) { }
         next_t modulo(next_t hash) const noexcept { return hash & _mask; }
         next_t getTableSize() const noexcept { return _mask + 1; }
         static next_t selectHashTableSize(size_t sz) noexcept { return hashtable_base::getModuloSimple(sz); }
@@ -198,7 +198,7 @@ public:
         using pointer = Value*;
         using iterator_category = std::forward_iterator_tag;
 
-        constexpr iterator(hashtable * hash) noexcept : _current(0), _hashTable(hash) {
+        constexpr explicit iterator(hashtable * hash) noexcept : _current(0), _hashTable(hash) {
             if (! _hashTable->_nodes[_current].valid()) {
                 advanceToNextValidHash();
             }
@@ -242,7 +242,7 @@ public:
         using pointer = const Value*;
         using iterator_category = std::forward_iterator_tag;
 
-        constexpr const_iterator(const hashtable * hash) noexcept : _current(0), _hashTable(hash) {
+        constexpr explicit const_iterator(const hashtable * hash) noexcept : _current(0), _hashTable(hash) {
             if (! _hashTable->_nodes[_current].valid()) {
                 advanceToNextValidHash();
             }
@@ -282,7 +282,7 @@ public:
     hashtable & operator = (hashtable &&) noexcept = default;
     hashtable(const hashtable &);
     hashtable & operator = (const hashtable &);
-    hashtable(size_t reservedSpace);
+    explicit hashtable(size_t reservedSpace);
     hashtable(size_t reservedSpace, const Hash & hasher, const Equal & equal);
     virtual ~hashtable();
     constexpr iterator begin()             noexcept { return iterator(this); }
diff --git a/vespalib/src/vespa/vespalib/stllike/hashtable.hpp b/vespalib/src/vespa/vespalib/stllike/hashtable.hpp
index 6d2d397a887..040e421f68c 100644
--- a/vespalib/src/vespa/vespalib/stllike/hashtable.hpp
+++ b/vespalib/src/vespa/vespalib/stllike/hashtable.hpp
@@ -53,8 +53,7 @@ hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::hashtable(size_t rese
     _nodes(createStore<NodeStore>(reservedSpace, _modulator.getTableSize())),
     _hasher(hasher),
     _equal(equal)
-{
-}
+{ }
 
 template< typename Key, typename Value, typename Hash, typename Equal, typename KeyExtract, typename Modulator >
 hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::hashtable(const hashtable &) = default;
@@ -130,7 +129,7 @@ hashtable<Key, Value, Hash, Equal, KeyExtract, Modulator>::insert_internal(V &&
         _count++;
         return insert_result(iterator(this, h), true);
     }
-    return insert_internal_cold(std::move(node), h);
+    return insert_internal_cold(std::forward<V>(node), h);
 }
 
 template< typename Key, typename Value, typename Hash, typename Equal, typename KeyExtract, typename Modulator >