Remove temporary allocations from bolt::Decoder

Reviewers: teon.banek

Reviewed By: teon.banek

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D1374

src/communication/bolt/v1/decoder/decoder.hpp

@@ -249,16 +249,35 @@ class Decoder {
   }
 
   bool ReadString(const Marker &marker, DecodedValue *data) {
+    const int kMaxStackBuffer = 8192;
+    uint8_t buffer[kMaxStackBuffer];
     auto size = ReadTypeSize(marker, MarkerString);
     if (size == -1) {
       return false;
     }
-    std::unique_ptr<uint8_t[]> ret(new uint8_t[size]);
+    // Here we use a temporary buffer on the stack to prevent temporary
-    if (!buffer_.Read(ret.get(), size)) {
+    // allocations. Most of strings that are decoded are small so it makes no
-      return false;
+    // sense to allocate a temporary buffer every time we decode a string. This
+    // way we allocate a temporary buffer only when the string is large. This
+    // wouldn't be necessary if we had full C++17 support. In C++17 we could
+    // preallocate the `buffer[size]` in the destination string `*data =
+    // DecodedValue(std::string('\0', size))` and just call
+    // `buffer_.Read(data->ValueString().data())`.
+    if (size < kMaxStackBuffer) {
+      if (!buffer_.Read(buffer, size)) {
+        DLOG(WARNING) << "[ReadString] Missing data!";
+        return false;
+      }
+      *data = DecodedValue(std::string(reinterpret_cast<char *>(buffer), size));
+    } else {
+      std::unique_ptr<uint8_t[]> ret(new uint8_t[size]);
+      if (!buffer_.Read(ret.get(), size)) {
+        DLOG(WARNING) << "[ReadString] Missing data!";
+        return false;
+      }
+      *data =
+          DecodedValue(std::string(reinterpret_cast<char *>(ret.get()), size));
     }
-    *data =
-        DecodedValue(std::string(reinterpret_cast<char *>(ret.get()), size));
     return true;
   }
 

tests/unit/bolt_decoder.cpp

@@ -144,6 +144,29 @@ TEST(BoltDecoder, String) {
   }
 }
 
+TEST(BoltDecoder, StringLarge) {
+  TestDecoderBuffer buffer;
+  DecoderT decoder(buffer);
+  DecodedValue dv;
+
+  uint8_t header[6] = "\xD2\x00\x01\x86\xA0";
+
+  // test missing data
+  buffer.Clear();
+  buffer.Write(header, 5);
+  buffer.Write(data, 10);
+  ASSERT_EQ(decoder.ReadValue(&dv), false);
+
+  // test all ok
+  buffer.Clear();
+  buffer.Write(header, 5);
+  buffer.Write(data, 100000);
+  ASSERT_EQ(decoder.ReadValue(&dv), true);
+  ASSERT_EQ(dv.type(), DecodedValue::Type::String);
+  std::string &str = dv.ValueString();
+  for (int j = 0; j < 100000; ++j) EXPECT_EQ((uint8_t)str[j], data[j]);
+}
+
 TEST(BoltDecoder, List) {
   TestDecoderBuffer buffer;
   DecoderT decoder(buffer);