Refactored StringBuilder into StringStorage

extras/tests/JsonDeserializer/filter.cpp

@@ -525,7 +525,7 @@ TEST_CASE("Filtering") {
       10,
       DeserializationError::InvalidInput,
       "{}", 
-      JSON_OBJECT_SIZE(0) + 8
+      JSON_OBJECT_SIZE(0)
     },
     {
       // incomplete comment after key
@@ -534,7 +534,7 @@ TEST_CASE("Filtering") {
       10,
       DeserializationError::IncompleteInput,
       "{}", 
-      JSON_OBJECT_SIZE(0) + 8
+      JSON_OBJECT_SIZE(0)
     },
     {
       // invalid comment after colon
@@ -730,20 +730,3 @@ TEST_CASE("Overloads") {
   }
 #endif
 }
-
-TEST_CASE("StringMover::reclaim()") {
-  StaticJsonDocument<200> filter;
-  filter["a"] = true;
-  filter["c"] = true;
-  char input[] = "{\"a\":1,\"b\":2,\"c\":1}";
-
-  StaticJsonDocument<200> doc;
-  deserializeJson(doc, input, DeserializationOption::Filter(filter));
-
-  REQUIRE(doc.as<std::string>() == "{\"a\":1,\"c\":1}");
-
-  CHECK(input[0] == 'a');
-  CHECK(input[1] == 0);
-  CHECK(input[2] == 'c');
-  CHECK(input[3] == 0);
-}

extras/tests/MemoryPool/CMakeLists.txt

@@ -7,7 +7,7 @@ add_executable(MemoryPoolTests
 	allocString.cpp
 	clear.cpp
 	size.cpp
-	StringBuilder.cpp
+	StringCopier.cpp
 )
 
 add_test(MemoryPool MemoryPoolTests)

extras/tests/MemoryPool/StringCopier.cpp

@@ -2,40 +2,44 @@
 // Copyright Benoit Blanchon 2014-2020
 // MIT License
 
-#include <ArduinoJson/Memory/MemoryPool.hpp>
-#include <ArduinoJson/Memory/StringBuilder.hpp>
+#include <ArduinoJson/StringStorage/StringCopier.hpp>
 #include <catch.hpp>
 
 using namespace ARDUINOJSON_NAMESPACE;
 
-TEST_CASE("StringBuilder") {
+TEST_CASE("StringCopier") {
   char buffer[4096];
 
   SECTION("Works when buffer is big enough") {
     MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(6)));
+    StringCopier str;
 
-    StringBuilder str(&pool);
+    str.startString(&pool);
     str.append("hello");
+    str.append('\0');
 
-    REQUIRE(str.complete() == std::string("hello"));
+    REQUIRE(str.isValid() == true);
+    REQUIRE(str.c_str() == std::string("hello"));
   }
 
   SECTION("Returns null when too small") {
     MemoryPool pool(buffer, sizeof(void*));
+    StringCopier str;
 
-    StringBuilder str(&pool);
+    str.startString(&pool);
     str.append("hello world!");
 
-    REQUIRE(str.complete() == 0);
+    REQUIRE(str.isValid() == false);
   }
 
   SECTION("Increases size of memory pool") {
     MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(6)));
+    StringCopier str;
 
-    StringBuilder str(&pool);
+    str.startString(&pool);
     str.append('h');
-    str.complete();
+    str.commit(&pool);
 
-    REQUIRE(JSON_STRING_SIZE(1) == pool.size());
+    REQUIRE(1 == pool.size());
   }
 }

extras/tests/MemoryPool/size.cpp

@@ -22,11 +22,6 @@ TEST_CASE("MemoryPool::size()") {
     REQUIRE(0 == pool.size());
   }
 
-  SECTION("size() == capacity() after allocExpandableString()") {
-    pool.allocExpandableString();
-    REQUIRE(pool.size() == pool.capacity());
-  }
-
   SECTION("Decreases after freezeString()") {
     StringSlot a = pool.allocExpandableString();
     pool.freezeString(a, 1);

extras/tests/Misc/Utf8.cpp

@@ -12,12 +12,14 @@ using namespace ARDUINOJSON_NAMESPACE;
 static void testCodepoint(uint32_t codepoint, std::string expected) {
   char buffer[4096];
   MemoryPool pool(buffer, 4096);
-  StringBuilder str(&pool);
+  StringCopier str;
+  str.startString(&pool);
 
   CAPTURE(codepoint);
   Utf8::encodeCodepoint(codepoint, str);
 
-  REQUIRE(str.complete() == expected);
+  str.append('\0');
+  REQUIRE(str.c_str() == expected);
 }
 
 TEST_CASE("Utf8::encodeCodepoint()") {

src/ArduinoJson/Deserialization/deserialize.hpp

@@ -32,9 +32,8 @@ deserialize(JsonDocument &doc, const TString &input, NestingLimit nestingLimit,
             TFilter filter) {
   Reader<TString> reader(input);
   doc.clear();
-  return makeDeserializer<TDeserializer>(
-             doc.memoryPool(), reader,
-             makeStringStorage(doc.memoryPool(), input))
+  return makeDeserializer<TDeserializer>(doc.memoryPool(), reader,
+                                         makeStringStorage(input))
       .parse(doc.data(), filter, nestingLimit);
 }
 //
@@ -48,9 +47,8 @@ DeserializationError deserialize(JsonDocument &doc, TChar *input,
                                  TFilter filter) {
   BoundedReader<TChar *> reader(input, inputSize);
   doc.clear();
-  return makeDeserializer<TDeserializer>(
-             doc.memoryPool(), reader,
-             makeStringStorage(doc.memoryPool(), input))
+  return makeDeserializer<TDeserializer>(doc.memoryPool(), reader,
+                                         makeStringStorage(input))
       .parse(doc.data(), filter, nestingLimit);
 }
 //
@@ -62,9 +60,8 @@ DeserializationError deserialize(JsonDocument &doc, TStream &input,
                                  NestingLimit nestingLimit, TFilter filter) {
   Reader<TStream> reader(input);
   doc.clear();
-  return makeDeserializer<TDeserializer>(
-             doc.memoryPool(), reader,
-             makeStringStorage(doc.memoryPool(), input))
+  return makeDeserializer<TDeserializer>(doc.memoryPool(), reader,
+                                         makeStringStorage(input))
       .parse(doc.data(), filter, nestingLimit);
 }
 

src/ArduinoJson/Json/JsonDeserializer.hpp

@@ -19,22 +19,10 @@ namespace ARDUINOJSON_NAMESPACE {
 
 template <typename TReader, typename TStringStorage>
 class JsonDeserializer {
-  typedef typename remove_reference<TStringStorage>::type::StringBuilder
-      StringBuilder;
-
-  struct StringOrError {
-    DeserializationError err;
-    const char *value;
-
-    StringOrError(DeserializationError e) : err(e) {}
-    StringOrError(DeserializationError::Code c) : err(c) {}
-    StringOrError(const char *s) : err(DeserializationError::Ok), value(s) {}
-  };
-
  public:
   JsonDeserializer(MemoryPool &pool, TReader reader,
                    TStringStorage stringStorage)
-      : _pool(&pool), _stringStorage(stringStorage), _latch(reader) {}
+      : _stringStorage(stringStorage), _latch(reader), _pool(&pool) {}
 
   template <typename TFilter>
   DeserializationError parse(VariantData &variant, TFilter filter,
@@ -224,9 +212,10 @@ class JsonDeserializer {
 
     // Read each key value pair
     for (;;) {
+      _stringStorage.startString(_pool);
+
       // Parse key
-      StringOrError key = parseKey();
-      err = key.err;  // <- this trick saves 62 bytes on AVR
+      err = parseKey();
       if (err)
         return err;
 
@@ -237,17 +226,21 @@ class JsonDeserializer {
       if (!eat(':'))
         return DeserializationError::InvalidInput;
 
-      TFilter memberFilter = filter[key.value];
+      const char *key = _stringStorage.c_str();
+
+      TFilter memberFilter = filter[key];
 
       if (memberFilter.allow()) {
-        VariantData *variant = object.getMember(adaptString(key.value));
+        VariantData *variant = object.getMember(adaptString(key));
         if (!variant) {
+          _stringStorage.commit(_pool);
+
           // Allocate slot in object
           VariantSlot *slot = object.addSlot(_pool);
           if (!slot)
             return DeserializationError::NoMemory;
 
-          slot->setOwnedKey(make_not_null(key.value));
+          slot->setOwnedKey(make_not_null(key));
 
           variant = slot->data();
         }
@@ -257,7 +250,6 @@ class JsonDeserializer {
         if (err)
           return err;
       } else {
-        _stringStorage.reclaim(key.value);
         err = skipVariant(nestingLimit.decrement());
         if (err)
           return err;
@@ -332,7 +324,7 @@ class JsonDeserializer {
     }
   }
 
-  StringOrError parseKey() {
+  DeserializationError parseKey() {
     if (isQuote(current())) {
       return parseQuotedString();
     } else {
@@ -341,15 +333,16 @@ class JsonDeserializer {
   }
 
   DeserializationError parseStringValue(VariantData &variant) {
-    StringOrError result = parseQuotedString();
-    if (result.err)
-      return result.err;
-    variant.setOwnedString(make_not_null(result.value));
+    _stringStorage.startString(_pool);
+    DeserializationError err = parseQuotedString();
+    if (err)
+      return err;
+    _stringStorage.commit(_pool);
+    variant.setOwnedString(make_not_null(_stringStorage.c_str()));
     return DeserializationError::Ok;
   }
 
-  StringOrError parseQuotedString() {
-    StringBuilder builder = _stringStorage.startString();
+  DeserializationError parseQuotedString() {
 #if ARDUINOJSON_DECODE_UNICODE
     Utf16::Codepoint codepoint;
 #endif
@@ -377,7 +370,7 @@ class JsonDeserializer {
           if (err)
             return err;
           if (codepoint.append(codeunit))
-            Utf8::encodeCodepoint(codepoint.value(), builder);
+            Utf8::encodeCodepoint(codepoint.value(), _stringStorage);
           continue;
 #else
           return DeserializationError::NotSupported;
@@ -390,35 +383,37 @@ class JsonDeserializer {
         move();
       }
 
-      builder.append(c);
+      _stringStorage.append(c);
     }
 
-    const char *result = builder.complete();
-    if (!result)
+    _stringStorage.append('\0');
+
+    if (!_stringStorage.isValid())
       return DeserializationError::NoMemory;
-    return result;
+
+    return DeserializationError::Ok;
   }
 
-  StringOrError parseNonQuotedString() {
-    StringBuilder builder = _stringStorage.startString();
-
+  DeserializationError parseNonQuotedString() {
     char c = current();
     ARDUINOJSON_ASSERT(c);
 
     if (canBeInNonQuotedString(c)) {  // no quotes
       do {
         move();
-        builder.append(c);
+        _stringStorage.append(c);
         c = current();
       } while (canBeInNonQuotedString(c));
     } else {
       return DeserializationError::InvalidInput;
     }
 
-    const char *result = builder.complete();
-    if (!result)
+    _stringStorage.append('\0');
+
+    if (!_stringStorage.isValid())
       return DeserializationError::NoMemory;
-    return result;
+
+    return DeserializationError::Ok;
   }
 
   DeserializationError skipString() {
@@ -597,9 +592,9 @@ class JsonDeserializer {
     }
   }
 
-  MemoryPool *_pool;
   TStringStorage _stringStorage;
   Latch<TReader> _latch;
+  MemoryPool *_pool;
 };
 
 // deserializeJson(JsonDocument&, const std::string&, ...)

src/ArduinoJson/Memory/MemoryPool.hpp

@@ -77,21 +77,16 @@ class MemoryPool {
     StringSlot s;
     s.value = _left;
     s.size = size_t(_right - _left);
-    _left = _right;
     checkInvariants();
     return s;
   }
 
   void freezeString(StringSlot& s, size_t newSize) {
-    _left -= (s.size - newSize);
+    _left = (s.value + newSize);
     s.size = newSize;
     checkInvariants();
   }
 
-  void reclaimLastString(const char* s) {
-    _left = const_cast<char*>(s);
-  }
-
   void clear() {
     _left = _begin;
     _right = _end;

src/ArduinoJson/Memory/StringBuilder.hpp

@@ -1,51 +0,0 @@
-// ArduinoJson - arduinojson.org
-// Copyright Benoit Blanchon 2014-2020
-// MIT License
-
-#pragma once
-
-#include <ArduinoJson/Memory/MemoryPool.hpp>
-
-namespace ARDUINOJSON_NAMESPACE {
-
-class StringBuilder {
- public:
-  explicit StringBuilder(MemoryPool* parent) : _parent(parent), _size(0) {
-    _slot = _parent->allocExpandableString();
-  }
-
-  void append(const char* s) {
-    while (*s) append(*s++);
-  }
-
-  void append(const char* s, size_t n) {
-    while (n-- > 0) append(*s++);
-  }
-
-  void append(char c) {
-    if (!_slot.value)
-      return;
-
-    if (_size >= _slot.size) {
-      _slot.value = 0;
-      return;
-    }
-
-    _slot.value[_size++] = c;
-  }
-
-  char* complete() {
-    append('\0');
-    if (_slot.value) {
-      _parent->freezeString(_slot, _size);
-    }
-    return _slot.value;
-  }
-
- private:
-  MemoryPool* _parent;
-  size_t _size;
-  StringSlot _slot;
-};
-
-}  // namespace ARDUINOJSON_NAMESPACE

src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp

@@ -15,9 +15,6 @@ namespace ARDUINOJSON_NAMESPACE {
 
 template <typename TReader, typename TStringStorage>
 class MsgPackDeserializer {
-  typedef typename remove_reference<TStringStorage>::type::StringBuilder
-      StringBuilder;
-
  public:
   MsgPackDeserializer(MemoryPool &pool, TReader reader,
                       TStringStorage stringStorage)
@@ -241,16 +238,18 @@ class MsgPackDeserializer {
   }
 
   DeserializationError readString(const char *&result, size_t n) {
-    StringBuilder builder = _stringStorage.startString();
+    _stringStorage.startString(_pool);
     for (; n; --n) {
       uint8_t c;
       if (!readBytes(c))
         return DeserializationError::IncompleteInput;
-      builder.append(static_cast<char>(c));
+      _stringStorage.append(static_cast<char>(c));
     }
-    result = builder.complete();
-    if (!result)
+    _stringStorage.append('\0');
+    if (!_stringStorage.isValid())
       return DeserializationError::NoMemory;
+    _stringStorage.commit(_pool);
+    result = _stringStorage.c_str();
     return DeserializationError::Ok;
   }
 

src/ArduinoJson/StringStorage/StringCopier.hpp

@@ -5,25 +5,51 @@
 #pragma once
 
 #include <ArduinoJson/Memory/MemoryPool.hpp>
-#include <ArduinoJson/Memory/StringBuilder.hpp>
 
 namespace ARDUINOJSON_NAMESPACE {
 
 class StringCopier {
  public:
-  typedef ARDUINOJSON_NAMESPACE::StringBuilder StringBuilder;
-
-  StringCopier(MemoryPool* pool) : _pool(pool) {}
-
-  StringBuilder startString() {
-    return StringBuilder(_pool);
+  void startString(MemoryPool* pool) {
+    _slot = pool->allocExpandableString();
+    _size = 0;
   }
 
-  void reclaim(const char* s) {
-    _pool->reclaimLastString(s);
+  void commit(MemoryPool* pool) {
+    ARDUINOJSON_ASSERT(_slot.value);
+    pool->freezeString(_slot, _size);
+  }
+
+  void append(const char* s) {
+    while (*s) append(*s++);
+  }
+
+  void append(const char* s, size_t n) {
+    while (n-- > 0) append(*s++);
+  }
+
+  void append(char c) {
+    if (!_slot.value)
+      return;
+
+    if (_size >= _slot.size) {
+      _slot.value = 0;
+      return;
+    }
+
+    _slot.value[_size++] = c;
+  }
+
+  bool isValid() {
+    return _slot.value != 0;
+  }
+
+  const char* c_str() {
+    return _slot.value;
   }
 
  private:
-  MemoryPool* _pool;
+  size_t _size;
+  StringSlot _slot;
 };
 }  // namespace ARDUINOJSON_NAMESPACE

src/ArduinoJson/StringStorage/StringMover.hpp

@@ -10,36 +10,28 @@ namespace ARDUINOJSON_NAMESPACE {
 
 class StringMover {
  public:
-  class StringBuilder {
-   public:
-    StringBuilder(char** ptr) : _writePtr(ptr), _startPtr(*ptr) {}
+  StringMover(char* ptr) : _writePtr(ptr) {}
 
-    void append(char c) {
-      *(*_writePtr)++ = char(c);
+  void startString(MemoryPool*) {
+    _startPtr = _writePtr;
   }
 
-    char* complete() const {
-      *(*_writePtr)++ = 0;
+  void commit(MemoryPool*) const {}
+
+  void append(char c) {
+    *_writePtr++ = c;
+  }
+
+  bool isValid() const {
+    return true;
+  }
+
+  const char* c_str() const {
     return _startPtr;
   }
 
  private:
-    char** _writePtr;
+  char* _writePtr;
   char* _startPtr;
-  };
-
-  StringMover(char* ptr) : _ptr(ptr) {}
-
-  StringBuilder startString() {
-    return StringBuilder(&_ptr);
-  }
-
-  // recover memory from last string
-  void reclaim(const char* str) {
-    _ptr = const_cast<char*>(str);
-  }
-
- private:
-  char* _ptr;
 };
 }  // namespace ARDUINOJSON_NAMESPACE

src/ArduinoJson/StringStorage/StringStorage.hpp

@@ -13,8 +13,8 @@ template <typename TInput, typename Enable = void>
 struct StringStorage {
   typedef StringCopier type;
 
-  static type create(MemoryPool& pool, TInput&) {
-    return type(&pool);
+  static type create(TInput&) {
+    return type();
   }
 };
 
@@ -23,20 +23,18 @@ struct StringStorage<TChar*,
                      typename enable_if<!is_const<TChar>::value>::type> {
   typedef StringMover type;
 
-  static type create(MemoryPool&, TChar* input) {
+  static type create(TChar* input) {
     return type(reinterpret_cast<char*>(input));
   }
 };
 
 template <typename TInput>
-typename StringStorage<TInput>::type makeStringStorage(MemoryPool& pool,
-                                                       TInput& input) {
-  return StringStorage<TInput>::create(pool, input);
+typename StringStorage<TInput>::type makeStringStorage(TInput& input) {
+  return StringStorage<TInput>::create(input);
 }
 
 template <typename TChar>
-typename StringStorage<TChar*>::type makeStringStorage(MemoryPool& pool,
-                                                       TChar* input) {
-  return StringStorage<TChar*>::create(pool, input);
+typename StringStorage<TChar*>::type makeStringStorage(TChar* input) {
+  return StringStorage<TChar*>::create(input);
 }
 }  // namespace ARDUINOJSON_NAMESPACE