common/StackStringStream: don't reserve before every insert

Unlike ConcreteEntry, MutableEntry can be appended to.  Reserving the
exact number of elements before every append is harmful: vector will
likely reallocate each time and grow linearly instead of geometrically.
This results in quadratic behaviour when we spill past the preallocated
capacity and doesn't benefit the fast path in any way.

The new test case takes half a second with this patch and many hours
spinning in memmove without this patch.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

diff --git a/src/common/StackStringStream.h b/src/common/StackStringStream.h
index 6d080f362ec096121db552293cdac8a8b57c2c77..32ce245f8925bae16245488e4d56435649172542 100644 (file)
--- a/src/common/StackStringStream.h
+++ b/src/common/StackStringStream.h
@@ -39,7 +39,6 @@ public:
 
   void push(std::string_view sv)
   {
-    vec.reserve(vec.size() + sv.size());
     vec.insert(vec.end(), sv.begin(), sv.end());
   }
 

diff --git a/src/log/test.cc b/src/log/test.cc
index 2e650cb99371360af35641bb6b9084542bf4a7bb..8b66e9ddf316df76d32ea05bda3857086d7dc0c8 100644 (file)
--- a/src/log/test.cc
+++ b/src/log/test.cc
@@ -232,6 +232,28 @@ TEST(Log, LargeLog)
   log.stop();
 }
 
+TEST(Log, LargeFromSmallLog)
+{
+  SubsystemMap subs;
+  subs.set_log_level(1, 20);
+  subs.set_gather_level(1, 10);
+  Log log(&subs);
+  log.start();
+  log.set_log_file("/tmp/big");
+  log.reopen_log_file();
+  int l = 10;
+  {
+    MutableEntry e(l, 1);
+    for (int i = 0; i < 1000000; i++) {
+      std::string msg(10, 'a');
+      e.get_ostream() << msg;
+    }
+    log.submit_entry(std::move(e));
+  }
+  log.flush();
+  log.stop();
+}
+
 // Make sure nothing bad happens when we switch
 
 TEST(Log, TimeSwitch)