Move MemStats struct to searchlib test library. Add class comment.

Stop wrapping EntryType in WrappedEntry.
Change Compare to reference value.  Add class comment.
Remove WrappedCompare.
Add freeze() method.

5 files changed, 69 insertions, 93 deletions

diff --git a/searchlib/src/tests/datastore/array_store/array_store_test.cpp b/searchlib/src/tests/datastore/array_store/array_store_test.cpp
index 5053e3605a5..aace9f6988a 100644
--- a/searchlib/src/tests/datastore/array_store/array_store_test.cpp
+++ b/searchlib/src/tests/datastore/array_store/array_store_test.cpp
@@ -6,37 +6,14 @@ LOG_SETUP("array_store_test");
 #include <vespa/vespalib/test/insertion_operators.h>
 #include <vespa/vespalib/util/traits.h>
 #include <vespa/searchlib/datastore/array_store.hpp>
+#include <vespa/searchlib/test/datastore/memstats.h>
 #include <vector>
 
 using namespace search::datastore;
 using search::MemoryUsage;
 using vespalib::ArrayRef;
 using generation_t = vespalib::GenerationHandler::generation_t;
-
-struct MemStats
-{
-    size_t _used;
-    size_t _hold;
-    size_t _dead;
-    MemStats() : _used(0), _hold(0), _dead(0) {}
-    MemStats(const MemoryUsage &usage)
-        : _used(usage.usedBytes()),
-          _hold(usage.allocatedBytesOnHold()),
-          _dead(usage.deadBytes()) {}
-    MemStats &used(size_t val) { _used += val; return *this; }
-    MemStats &hold(size_t val) { _hold += val; return *this; }
-    MemStats &dead(size_t val) { _dead += val; return *this; }
-    MemStats &holdToDead(size_t val) {
-        decHold(val);
-        _dead += val;
-        return *this;
-    }
-    MemStats &decHold(size_t val) {
-        ASSERT_TRUE(_hold >= val);
-        _hold -= val;
-        return *this;
-    }
-};
+using MemStats = search::datastore::test::MemStats;
 
 template <typename EntryT, typename RefT = EntryRefT<19> >
 struct Fixture
diff --git a/searchlib/src/tests/datastore/unique_store/unique_store_test.cpp b/searchlib/src/tests/datastore/unique_store/unique_store_test.cpp
index 8d811b2a957..aefbf9a9835 100644
--- a/searchlib/src/tests/datastore/unique_store/unique_store_test.cpp
+++ b/searchlib/src/tests/datastore/unique_store/unique_store_test.cpp
@@ -6,37 +6,14 @@ LOG_SETUP("unique_store_test");
 #include <vespa/vespalib/test/insertion_operators.h>
 #include <vespa/vespalib/util/traits.h>
 #include <vespa/searchlib/datastore/unique_store.hpp>
+#include <vespa/searchlib/test/datastore/memstats.h>
 #include <vector>
 
 using namespace search::datastore;
 using search::MemoryUsage;
 using vespalib::ArrayRef;
 using generation_t = vespalib::GenerationHandler::generation_t;
-
-struct MemStats
-{
-    size_t _used;
-    size_t _hold;
-    size_t _dead;
-    MemStats() : _used(0), _hold(0), _dead(0) {}
-    MemStats(const MemoryUsage &usage)
-        : _used(usage.usedBytes()),
-          _hold(usage.allocatedBytesOnHold()),
-          _dead(usage.deadBytes()) {}
-    MemStats &used(size_t val) { _used += val; return *this; }
-    MemStats &hold(size_t val) { _hold += val; return *this; }
-    MemStats &dead(size_t val) { _dead += val; return *this; }
-    MemStats &holdToDead(size_t val) {
-        decHold(val);
-        _dead += val;
-        return *this;
-    }
-    MemStats &decHold(size_t val) {
-        ASSERT_TRUE(_hold >= val);
-        _hold -= val;
-        return *this;
-    }
-};
+using MemStats = search::datastore::test::MemStats;
 
 template <typename EntryT, typename RefT = EntryRefT<22> >
 struct Fixture
@@ -110,6 +87,7 @@ struct Fixture
         return EntryRef();
     }
     void trimHoldLists() {
+        store.freeze();
         store.transferHoldLists(generation++);
         store.trimHoldLists(generation);
     }
diff --git a/searchlib/src/vespa/searchlib/datastore/unique_store.h b/searchlib/src/vespa/searchlib/datastore/unique_store.h
index b60aa30bea9..60381f38889 100644
--- a/searchlib/src/vespa/searchlib/datastore/unique_store.h
+++ b/searchlib/src/vespa/searchlib/datastore/unique_store.h
@@ -24,32 +24,24 @@ public:
     using DataStoreType = DataStoreT<RefT>;
     using EntryType = EntryT;
     using RefType = RefT;
-    class WrappedEntry {
-        EntryType _value;
-    public:
-        WrappedEntry() : _value() { }
-        WrappedEntry(const EntryType &value_) : _value(value_) { }
-        WrappedEntry(const WrappedEntry &rhs) : _value(rhs.value()) { }
-        const EntryType &value() const { return _value; }
-    };
+    /*
+     * Compare two values in data store based on reference.  Invalid
+     * reference is mapped to local value reference to support
+     * comparing with new value candidate outside data store.
+     */
     class Compare {
         const DataStoreType &_store;
-        const EntryType _value;
+        const EntryType &_value;
 public:
         Compare(const DataStoreType &store, const EntryType &value)
             : _store(store),
               _value(value)
         {
         }
-        Compare(const DataStoreType &store)
-            : _store(store),
-              _value()
-        {
-        }
         inline const EntryType &get(EntryRef ref) const {
             if (ref.valid()) {
                 RefType iRef(ref);
-                return _store.template getBufferEntry<WrappedEntry>(iRef.bufferId(), iRef.offset())->value();
+                return *_store.template getBufferEntry<EntryType>(iRef.bufferId(), iRef.offset());
             } else {
                 return _value;
             }
@@ -61,24 +53,11 @@ public:
             return lhsValue < rhsValue;
         }
     };
-    class WrappedCompare {
-        const Compare &_comp;
-    public:
-        WrappedCompare(const Compare &comp)
-            : _comp(comp)
-        {
-        }
-        inline bool operator()(EntryRef lhs, EntryRef rhs) const
-        {
-            return _comp(lhs, rhs);
-        }
-    };
-
-    using UniqueStoreBufferType = BufferType<WrappedEntry>;
+    using UniqueStoreBufferType = BufferType<EntryType>;
     using DictionaryTraits = btree::BTreeTraits<32, 32, 7, true>;
     using Dictionary = btree::BTree<EntryRef, uint32_t,
                                     btree::NoAggregated,
-                                    const WrappedCompare,
+                                    Compare,
                                     DictionaryTraits>;
 private:
     DataStoreType _store;
@@ -87,17 +66,16 @@ private:
     Dictionary _dict;
     using generation_t = vespalib::GenerationHandler::generation_t;
 
-    const WrappedEntry &getWrapped(EntryRef ref) const
-    {
-        RefType iRef(ref);
-        return *_store.template getBufferEntry<WrappedEntry>(iRef.bufferId(), iRef.offset());
-    }
 public:
     UniqueStore();
     ~UniqueStore();
     EntryRef move(EntryRef ref);
-    EntryRef add(const EntryType &array);
-    const EntryType &get(EntryRef ref) const { return getWrapped(ref).value(); }
+    EntryRef add(const EntryType &value);
+    const EntryType &get(EntryRef ref) const
+    {
+        RefType iRef(ref);
+        return *_store.template getBufferEntry<EntryType>(iRef.bufferId(), iRef.offset());
+    }
     void remove(EntryRef ref);
     ICompactionContext::UP compactWorst();
     MemoryUsage getMemoryUsage() const;
@@ -107,6 +85,7 @@ public:
     void trimHoldLists(generation_t firstUsed) { _dict.getAllocator().trimHoldLists(firstUsed); _store.trimHoldLists(firstUsed); }
     vespalib::GenerationHolder &getGenerationHolder(void) { return _store.getGenerationHolder(); }
     void setInitializing(bool initializing) { _store.setInitializing(initializing); }
+    void freeze() { _dict.getAllocator().freeze(); }
 
     // Should only be used for unit testing
     const BufferState &bufferState(EntryRef ref) const;
diff --git a/searchlib/src/vespa/searchlib/datastore/unique_store.hpp b/searchlib/src/vespa/searchlib/datastore/unique_store.hpp
index b50c8e6ceaa..85ff503625f 100644
--- a/searchlib/src/vespa/searchlib/datastore/unique_store.hpp
+++ b/searchlib/src/vespa/searchlib/datastore/unique_store.hpp
@@ -14,10 +14,12 @@
 namespace search {
 namespace datastore {
 
+constexpr size_t NUMCLUSTERS_FOR_NEW_UNIQUESTORE_BUFFER = 1024u;
+
 template <typename EntryT, typename RefT>
 UniqueStore<EntryT, RefT>::UniqueStore()
     : _store(),
-      _typeHandler(1, 2u, RefT::offsetSize(), 1024u),
+      _typeHandler(1, 2u, RefT::offsetSize(), NUMCLUSTERS_FOR_NEW_UNIQUESTORE_BUFFER),
       _typeId(0),
       _dict()
 {
@@ -38,14 +40,14 @@ EntryRef
 UniqueStore<EntryT, RefT>::add(const EntryType &value)
 {
     Compare comp(_store, value);
-    auto itr = _dict.lowerBound(RefType(), WrappedCompare(comp));
+    auto itr = _dict.lowerBound(RefType(), comp);
     if (itr.valid() && !comp(EntryRef(), itr.getKey())) {
         uint32_t refCount = itr.getData();
         assert(refCount != std::numeric_limits<uint32_t>::max());
         itr.writeData(refCount + 1);
         return itr.getKey();
     } else {
-        EntryRef newRef = _store.template allocator<WrappedEntry>(_typeId).alloc(value).ref;
+        EntryRef newRef = _store.template allocator<EntryType>(_typeId).alloc(value).ref;
         _dict.insert(itr, newRef, 1u);
         return newRef;
     }
@@ -55,7 +57,7 @@ template <typename EntryT, typename RefT>
 EntryRef
 UniqueStore<EntryT, RefT>::move(EntryRef ref)
 {
-    return _store.template allocator<WrappedEntry>(_typeId).alloc(getWrapped(ref)).ref;
+    return _store.template allocator<EntryType>(_typeId).alloc(get(ref)).ref;
 }
 
 template <typename EntryT, typename RefT>
@@ -63,8 +65,9 @@ void
 UniqueStore<EntryT, RefT>::remove(EntryRef ref)
 {
     assert(ref.valid());
-    Compare comp(_store);
-    auto itr = _dict.lowerBound(ref, WrappedCompare(comp));
+    EntryType unused;
+    Compare comp(_store, unused);
+    auto itr = _dict.lowerBound(ref, comp);
     if (itr.valid() && itr.getKey() == ref) {
         uint32_t refCount = itr.getData();
         if (refCount > 1) {
diff --git a/searchlib/src/vespa/searchlib/test/datastore/memstats.h b/searchlib/src/vespa/searchlib/test/datastore/memstats.h
new file mode 100644
index 00000000000..e373fd96b65
--- /dev/null
+++ b/searchlib/src/vespa/searchlib/test/datastore/memstats.h
@@ -0,0 +1,39 @@
+// Copyright 2017 Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+namespace search {
+namespace datastore {
+namespace test {
+
+/*
+ * Class representing expected memory stats in unit tests.
+ */
+struct MemStats
+{
+    size_t _used;
+    size_t _hold;
+    size_t _dead;
+    MemStats() : _used(0), _hold(0), _dead(0) {}
+    MemStats(const MemoryUsage &usage)
+        : _used(usage.usedBytes()),
+          _hold(usage.allocatedBytesOnHold()),
+          _dead(usage.deadBytes()) {}
+    MemStats &used(size_t val) { _used += val; return *this; }
+    MemStats &hold(size_t val) { _hold += val; return *this; }
+    MemStats &dead(size_t val) { _dead += val; return *this; }
+    MemStats &holdToDead(size_t val) {
+        decHold(val);
+        _dead += val;
+        return *this;
+    }
+    MemStats &decHold(size_t val) {
+        assert(_hold >= val);
+        _hold -= val;
+        return *this;
+    }
+};
+
+}
+}
+}