Merge pull request #6912 from vespa-engine/havardpe/smart-buffer-in-vespalib

slightly smarter buffer with test

6 files changed, 252 insertions, 0 deletions

diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt
index 4ae98be29b6..fb3b08b325f 100644
--- a/vespalib/CMakeLists.txt
+++ b/vespalib/CMakeLists.txt
@@ -33,6 +33,7 @@ vespa_define_module(
     src/tests/data/memory_input
     src/tests/data/output_writer
     src/tests/data/simple_buffer
+    src/tests/data/smart_buffer
     src/tests/delegatelist
     src/tests/dotproduct
     src/tests/dual_merge_director
diff --git a/vespalib/src/tests/data/smart_buffer/CMakeLists.txt b/vespalib/src/tests/data/smart_buffer/CMakeLists.txt
new file mode 100644
index 00000000000..e7468f4f508
--- /dev/null
+++ b/vespalib/src/tests/data/smart_buffer/CMakeLists.txt
@@ -0,0 +1,8 @@
+# Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+vespa_add_executable(vespalib_smart_buffer_test_app TEST
+    SOURCES
+    smart_buffer_test.cpp
+    DEPENDS
+    vespalib
+)
+vespa_add_test(NAME vespalib_smart_buffer_test_app COMMAND vespalib_smart_buffer_test_app)
diff --git a/vespalib/src/tests/data/smart_buffer/smart_buffer_test.cpp b/vespalib/src/tests/data/smart_buffer/smart_buffer_test.cpp
new file mode 100644
index 00000000000..360afba091a
--- /dev/null
+++ b/vespalib/src/tests/data/smart_buffer/smart_buffer_test.cpp
@@ -0,0 +1,133 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#include <vespa/vespalib/testkit/test_kit.h>
+#include <vespa/vespalib/data/smart_buffer.h>
+
+using namespace vespalib;
+
+void checkMemory(const vespalib::string &expect, const Memory &mem) {
+    EXPECT_EQUAL(expect, vespalib::string(mem.data, mem.size));
+}
+
+void checkBuffer(const vespalib::string &expect, SmartBuffer &buf) {
+    TEST_DO(checkMemory(expect, buf.obtain()));
+}
+
+void write_buf(const vespalib::string &str, SmartBuffer &buf) {
+    WritableMemory mem = buf.reserve(str.size());
+    for (size_t i = 0; i < str.size(); ++i) {
+        mem.data[i] = str.data()[i];
+    }
+    buf.commit(str.size());
+}
+
+TEST("require that basic read/write works") {
+    SmartBuffer buf(3);
+    TEST_DO(checkBuffer("", buf));
+    { // read from empty buffer
+        EXPECT_EQUAL(0u, buf.obtain().size);
+    }
+    { // write to buffer
+        WritableMemory mem = buf.reserve(10);
+        TEST_DO(checkBuffer("", buf));
+        EXPECT_LESS_EQUAL(10u, mem.size);
+        mem.data[0] = 'a';
+        mem.data[1] = 'b';
+        mem.data[2] = 'c';
+        EXPECT_EQUAL(&buf, &buf.commit(3));
+        mem = buf.reserve(0);
+        TEST_DO(checkBuffer("abc", buf));
+        EXPECT_LESS_EQUAL(0u, mem.size);
+    }
+    { // read without evicting last byte
+        Memory mem = buf.obtain();
+        TEST_DO(checkBuffer("abc", buf));
+        TEST_DO(checkMemory("abc", mem));
+        EXPECT_EQUAL(&buf, &buf.evict(2));
+        mem = buf.obtain();
+        TEST_DO(checkBuffer("c", buf));
+        TEST_DO(checkMemory("c", mem));
+        mem = buf.obtain();
+        TEST_DO(checkBuffer("c", buf));
+        TEST_DO(checkMemory("c", mem));
+    }
+    { // write more to buffer
+        WritableMemory mem = buf.reserve(10);
+        EXPECT_LESS_EQUAL(10u, mem.size);
+        TEST_DO(checkBuffer("c", buf));
+        mem.data[0] = 'd';
+        EXPECT_EQUAL(&buf, &buf.commit(1));
+        mem = buf.reserve(5);
+        TEST_DO(checkBuffer("cd", buf));
+        EXPECT_LESS_EQUAL(5u, mem.size);
+    }
+    { // read until end
+        Memory mem = buf.obtain();
+        TEST_DO(checkBuffer("cd", buf));
+        TEST_DO(checkMemory("cd", mem));
+        EXPECT_EQUAL(&buf, &buf.evict(1));
+        mem = buf.obtain();
+        TEST_DO(checkBuffer("d", buf));
+        TEST_DO(checkMemory("d", mem));
+        EXPECT_EQUAL(&buf, &buf.evict(1));
+        mem = buf.obtain();
+        TEST_DO(checkBuffer("", buf));
+        TEST_DO(checkMemory("", mem));
+    }
+}
+
+TEST("require that requested initial size is not adjusted") {
+    SmartBuffer buf(400);
+    EXPECT_EQUAL(buf.capacity(), 400u);
+}
+
+TEST("require that buffer auto-resets when empty") {
+    SmartBuffer buf(64);
+    EXPECT_EQUAL(buf.reserve(10).size, 64u);
+    write_buf("abc", buf);
+    EXPECT_EQUAL(buf.reserve(10).size, 61u);
+    buf.evict(3);
+    EXPECT_EQUAL(buf.reserve(10).size, 64u);
+}
+
+TEST("require that buffer can grow") {
+    SmartBuffer buf(64);
+    EXPECT_EQUAL(buf.capacity(), 64u);
+    write_buf("abc", buf);
+    write_buf("abc", buf);
+    buf.evict(3);
+    EXPECT_EQUAL(buf.reserve(70).size, size_t(128 - 3));
+    TEST_DO(checkBuffer("abc", buf));
+    EXPECT_EQUAL(buf.capacity(), 128u);
+}
+
+TEST("require that buffer can grow more than 2x") {
+    SmartBuffer buf(64);
+    EXPECT_EQUAL(buf.capacity(), 64u);
+    write_buf("abc", buf);
+    write_buf("abc", buf);
+    buf.evict(3);
+    EXPECT_EQUAL(buf.reserve(170).size, 170u);
+    TEST_DO(checkBuffer("abc", buf));
+    EXPECT_EQUAL(buf.capacity(), 173u);
+}
+
+TEST("require that buffer can be compacted") {
+    SmartBuffer buf(16);
+    EXPECT_EQUAL(buf.capacity(), 16u);
+    write_buf("abc", buf);
+    write_buf("abc", buf);
+    buf.evict(3);
+    write_buf("abc", buf);
+    buf.evict(3);
+    write_buf("abc", buf);
+    buf.evict(3);
+    write_buf("abc", buf);
+    buf.evict(3);
+    EXPECT_EQUAL(buf.reserve(0).size, 1u);
+    write_buf("abc", buf);
+    TEST_DO(checkBuffer("abcabc", buf));
+    EXPECT_EQUAL(buf.capacity(), 16u);
+    EXPECT_EQUAL(buf.reserve(0).size, 10u);
+}
+
+TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/vespa/vespalib/data/CMakeLists.txt b/vespalib/src/vespa/vespalib/data/CMakeLists.txt
index 3a94e00ae33..517d0cd198f 100644
--- a/vespalib/src/vespa/vespalib/data/CMakeLists.txt
+++ b/vespalib/src/vespa/vespalib/data/CMakeLists.txt
@@ -12,6 +12,7 @@ vespa_add_library(vespalib_vespalib_data OBJECT
     output.cpp
     output_writer.cpp
     simple_buffer.cpp
+    smart_buffer.cpp
     writable_memory.cpp
     DEPENDS
 )
diff --git a/vespalib/src/vespa/vespalib/data/smart_buffer.cpp b/vespalib/src/vespa/vespalib/data/smart_buffer.cpp
new file mode 100644
index 00000000000..401b6729601
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/data/smart_buffer.cpp
@@ -0,0 +1,68 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "smart_buffer.h"
+#include <cassert>
+
+namespace vespalib {
+
+void
+SmartBuffer::ensure_free(size_t bytes)
+{
+    if (write_len() >= bytes) {
+        return;
+    }
+    if ((unused() < bytes) || ((unused() * 3) < read_len())) {
+        size_t new_size = std::max(_data.size() * 2, read_len() + bytes);
+        alloc::Alloc new_buf(alloc::Alloc::alloc(new_size));
+        memcpy(new_buf.get(), read_ptr(), read_len());
+        _data.swap(new_buf);
+    } else {
+        memmove(_data.get(), read_ptr(), read_len());
+    }
+    _write_pos = read_len();
+    _read_pos = 0;
+}
+
+SmartBuffer::SmartBuffer(size_t initial_size)
+    : _data(alloc::Alloc::alloc(initial_size)),
+      _read_pos(0),
+      _write_pos(0)
+{
+}
+
+SmartBuffer::~SmartBuffer() = default;
+
+Memory
+SmartBuffer::obtain()
+{
+    return Memory(read_ptr(), read_len());
+}
+
+Input &
+SmartBuffer::evict(size_t bytes)
+{
+    assert(read_len() >= bytes);
+    _read_pos += bytes;
+    if (_read_pos == _write_pos) {
+        _read_pos = 0;
+        _write_pos = 0;
+    }
+    return *this;
+}
+
+WritableMemory
+SmartBuffer::reserve(size_t bytes)
+{
+    ensure_free(bytes);
+    return WritableMemory(write_ptr(), write_len());
+}
+
+Output &
+SmartBuffer::commit(size_t bytes)
+{
+    assert(write_len() >= bytes);
+    _write_pos += bytes;
+    return *this;
+}
+
+} // namespace vespalib
diff --git a/vespalib/src/vespa/vespalib/data/smart_buffer.h b/vespalib/src/vespa/vespalib/data/smart_buffer.h
new file mode 100644
index 00000000000..f7c4dd05c3e
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/data/smart_buffer.h
@@ -0,0 +1,41 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include "input.h"
+#include "output.h"
+#include <vespa/vespalib/util/alloc.h>
+
+namespace vespalib {
+
+/**
+ * A somewhat smarter buffer compared to SimpleBuffer. Keeps track of
+ * data in a continuous memory segment. Tries to limit copying of
+ * data.
+ **/
+class SmartBuffer : public Input,
+                    public Output
+{
+private:
+    alloc::Alloc _data;
+    size_t       _read_pos;
+    size_t       _write_pos;
+
+    const char *read_ptr() const { return (const char *)(_data.get()) + _read_pos; }
+    size_t read_len() const { return (_write_pos - _read_pos); } 
+    char *write_ptr() { return (char *)(_data.get()) + _write_pos; }
+    size_t write_len() const { return (_data.size() - _write_pos); }
+    size_t unused() const { return (_data.size() - read_len()); }
+    void ensure_free(size_t bytes);
+
+public:
+    SmartBuffer(size_t initial_size);
+    ~SmartBuffer();
+    size_t capacity() const { return _data.size(); }
+    Memory obtain() override;
+    Input &evict(size_t bytes) override;
+    WritableMemory reserve(size_t bytes) override;
+    Output &commit(size_t bytes) override;
+};
+
+} // namespace vespalib