Pass StringNode* to VariantData

bblanchon · May 2, 2023 · 9f6afe0 · 9f6afe0
1 parent 5c03389
commit 9f6afe0
Show file tree

Hide file tree

Showing 13 changed files with 90 additions and 60 deletions.
diff --git a/extras/tests/MemoryPool/StringCopier.cpp b/extras/tests/MemoryPool/StringCopier.cpp
@@ -93,39 +93,46 @@ TEST_CASE("StringCopier") {
   }
 }
 
-static const char* addStringToPool(MemoryPool& pool, const char* s) {
+static StringNode* addStringToPool(MemoryPool& pool, const char* s) {
   StringCopier str(&pool);
   str.startString();
   str.append(s);
-  return str.save().c_str();
+  return str.save();
 }
 
 TEST_CASE("StringCopier::save() deduplicates strings") {
   MemoryPool pool(4096);
 
   SECTION("Basic") {
-    const char* s1 = addStringToPool(pool, "hello");
-    const char* s2 = addStringToPool(pool, "world");
-    const char* s3 = addStringToPool(pool, "hello");
+    auto s1 = addStringToPool(pool, "hello");
+    auto s2 = addStringToPool(pool, "world");
+    auto s3 = addStringToPool(pool, "hello");
 
     REQUIRE(s1 == s3);
     REQUIRE(s2 != s3);
+    REQUIRE(s1->references == 2);
+    REQUIRE(s2->references == 1);
+    REQUIRE(s3->references == 2);
     REQUIRE(pool.size() == 2 * sizeofString(5));
   }
 
   SECTION("Requires terminator") {
-    const char* s1 = addStringToPool(pool, "hello world");
-    const char* s2 = addStringToPool(pool, "hello");
+    auto s1 = addStringToPool(pool, "hello world");
+    auto s2 = addStringToPool(pool, "hello");
 
     REQUIRE(s2 != s1);
+    REQUIRE(s1->references == 1);
+    REQUIRE(s2->references == 1);
     REQUIRE(pool.size() == sizeofString(11) + sizeofString(5));
   }
 
   SECTION("Don't overrun") {
-    const char* s1 = addStringToPool(pool, "hello world");
-    const char* s2 = addStringToPool(pool, "wor");
+    auto s1 = addStringToPool(pool, "hello world");
+    auto s2 = addStringToPool(pool, "wor");
 
     REQUIRE(s2 != s1);
+    REQUIRE(s1->references == 1);
+    REQUIRE(s2->references == 1);
     REQUIRE(pool.size() == sizeofString(11) + sizeofString(3));
   }
 }
diff --git a/extras/tests/MemoryPool/saveString.cpp b/extras/tests/MemoryPool/saveString.cpp
@@ -10,47 +10,59 @@
 
 using namespace ArduinoJson::detail;
 
-static const char* saveString(MemoryPool& pool, const char* s) {
+static StringNode* saveString(MemoryPool& pool, const char* s) {
   return pool.saveString(adaptString(s));
 }
 
-static const char* saveString(MemoryPool& pool, const char* s, size_t n) {
+static StringNode* saveString(MemoryPool& pool, const char* s, size_t n) {
   return pool.saveString(adaptString(s, n));
 }
 
 TEST_CASE("MemoryPool::saveString()") {
   MemoryPool pool(32);
 
   SECTION("Duplicates different strings") {
-    const char* a = saveString(pool, "hello");
-    const char* b = saveString(pool, "world");
-    REQUIRE(a != b);
+    auto a = saveString(pool, "hello");
+    auto b = saveString(pool, "world");
+    REQUIRE(a->data != b->data);
+    REQUIRE(a->length == 5);
+    REQUIRE(b->length == 5);
+    REQUIRE(a->references == 1);
+    REQUIRE(b->references == 1);
     REQUIRE(pool.size() == 2 * sizeofString(5));
   }
 
   SECTION("Deduplicates identical strings") {
-    const char* a = saveString(pool, "hello");
-    const char* b = saveString(pool, "hello");
+    auto a = saveString(pool, "hello");
+    auto b = saveString(pool, "hello");
     REQUIRE(a == b);
+    REQUIRE(a->length == 5);
+    REQUIRE(a->references == 2);
     REQUIRE(pool.size() == sizeofString(5));
   }
 
   SECTION("Deduplicates identical strings that contain NUL") {
-    const char* a = saveString(pool, "hello\0world", 11);
-    const char* b = saveString(pool, "hello\0world", 11);
+    auto a = saveString(pool, "hello\0world", 11);
+    auto b = saveString(pool, "hello\0world", 11);
     REQUIRE(a == b);
+    REQUIRE(a->length == 11);
+    REQUIRE(a->references == 2);
     REQUIRE(pool.size() == sizeofString(11));
   }
 
   SECTION("Don't stop on first NUL") {
-    const char* a = saveString(pool, "hello");
-    const char* b = saveString(pool, "hello\0world", 11);
+    auto a = saveString(pool, "hello");
+    auto b = saveString(pool, "hello\0world", 11);
     REQUIRE(a != b);
+    REQUIRE(a->length == 5);
+    REQUIRE(b->length == 11);
+    REQUIRE(a->references == 1);
+    REQUIRE(b->references == 1);
     REQUIRE(pool.size() == sizeofString(5) + sizeofString(11));
   }
 
   SECTION("Returns NULL when allocation fails") {
     MemoryPool pool2(32, FailingAllocator::instance());
-    REQUIRE(0 == saveString(pool2, "a"));
+    REQUIRE(saveString(pool2, "a") == nullptr);
   }
 }
diff --git a/src/ArduinoJson/Collection/CollectionFunctions.hpp b/src/ArduinoJson/Collection/CollectionFunctions.hpp
@@ -28,12 +28,12 @@ inline VariantData* collectionAddMember(CollectionData* obj, TAdaptedString key,
   if (!slot)
     return nullptr;
   if (key.isLinked())
-    slot->setKey(JsonString(key.data(), key.size(), JsonString::Linked));
+    slot->setKey(key.data());
   else {
     auto storedKey = pool->saveString(key);
     if (!storedKey)
       return nullptr;
-    slot->setKey(JsonString(storedKey, key.size(), JsonString::Copied));
+    slot->setKey(storedKey);
   }
   obj->add(slot);
   return slot->data();

diff --git a/src/ArduinoJson/Json/JsonDeserializer.hpp b/src/ArduinoJson/Json/JsonDeserializer.hpp
@@ -277,14 +277,14 @@ class JsonDeserializer {
         VariantSlot* slot = object.get(adaptString(key.c_str()));
         if (!slot) {
           // Save key in memory pool.
-          key = stringStorage_.save();
+          auto savedKey = stringStorage_.save();
 
           // Allocate slot in object
           slot = pool_->allocVariant();
           if (!slot)
             return DeserializationError::NoMemory;
 
-          slot->setKey(key);
+          slot->setKey(savedKey);
           object.add(slot);
         }
 

diff --git a/src/ArduinoJson/Memory/MemoryPool.hpp b/src/ArduinoJson/Memory/MemoryPool.hpp
@@ -107,14 +107,14 @@ class MemoryPool {
   }
 
   template <typename TAdaptedString>
-  const char* saveString(TAdaptedString str) {
+  StringNode* saveString(TAdaptedString str) {
     if (str.isNull())
       return 0;
 
     auto node = findString(str);
     if (node) {
       node->references++;
-      return node->data;
+      return node;
     }
 
     size_t n = str.size();
@@ -126,7 +126,7 @@ class MemoryPool {
     stringGetChars(str, node->data, n);
     node->data[n] = 0;  // force NUL terminator
     addStringToList(node);
-    return node->data;
+    return node;
   }
 
   void addStringToList(StringNode* node) {

diff --git a/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp b/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp
@@ -494,13 +494,13 @@ class MsgPackDeserializer {
         ARDUINOJSON_ASSERT(object != 0);
 
         // Save key in memory pool.
-        key = stringStorage_.save();
+        auto savedKey = stringStorage_.save();
 
         VariantSlot* slot = pool_->allocVariant();
         if (!slot)
           return DeserializationError::NoMemory;
 
-        slot->setKey(key);
+        slot->setKey(savedKey);
         object->add(slot);
 
         member = slot->data();

diff --git a/src/ArduinoJson/StringStorage/StringCopier.hpp b/src/ArduinoJson/StringStorage/StringCopier.hpp
@@ -25,7 +25,7 @@ class StringCopier {
       node_ = pool_->allocString(initialCapacity);
   }
 
-  JsonString save() {
+  StringNode* save() {
     ARDUINOJSON_ASSERT(node_ != nullptr);
     node_->data[size_] = 0;
     StringNode* node = pool_->findString(adaptString(node_->data, size_));
@@ -36,7 +36,7 @@ class StringCopier {
     } else {
       node->references++;
     }
-    return JsonString(node->data, node->length, JsonString::Copied);
+    return node;
   }
 
   void append(const char* s) {

diff --git a/src/ArduinoJson/StringStorage/StringMover.hpp b/src/ArduinoJson/StringStorage/StringMover.hpp
@@ -17,10 +17,9 @@ class StringMover {
     startPtr_ = writePtr_;
   }
 
-  FORCE_INLINE JsonString save() {
-    JsonString s = str();
-    writePtr_++;
-    return s;
+  FORCE_INLINE const char* save() {
+    *writePtr_++ = 0;  // terminator
+    return startPtr_;
   }
 
   void append(char c) {

diff --git a/src/ArduinoJson/Variant/ConverterImpl.hpp b/src/ArduinoJson/Variant/ConverterImpl.hpp
@@ -188,7 +188,7 @@ class MemoryPoolPrint : public Print {
     copier_.startString();
   }
 
-  JsonString str() {
+  StringNode* save() {
     ARDUINOJSON_ASSERT(!overflowed());
     return copier_.save();
   }
@@ -227,7 +227,7 @@ inline void convertToJson(const ::Printable& src, JsonVariant dst) {
     data->setNull();
     return;
   }
-  data->setString(print.str());
+  data->setString(print.save());
 }
 
 #endif

diff --git a/src/ArduinoJson/Variant/VariantData.hpp b/src/ArduinoJson/Variant/VariantData.hpp
@@ -157,10 +157,11 @@ class VariantData {
     content_.asFloat = value;
   }
 
-  void setRawString(const char* data, size_t n) {
+  void setRawString(StringNode* s) {
+    ARDUINOJSON_ASSERT(s);
     setType(VALUE_IS_RAW_STRING);
-    content_.asString.data = data;
-    content_.asString.size = n;
+    content_.asString.data = s->data;
+    content_.asString.size = s->length;
   }
 
   template <typename T>
@@ -179,14 +180,18 @@ class VariantData {
     setType(VALUE_IS_NULL);
   }
 
-  void setString(JsonString s) {
+  void setString(StringNode* s) {
     ARDUINOJSON_ASSERT(s);
-    if (s.isLinked())
-      setType(VALUE_IS_LINKED_STRING);
-    else
-      setType(VALUE_IS_OWNED_STRING);
-    content_.asString.data = s.c_str();
-    content_.asString.size = s.size();
+    setType(VALUE_IS_OWNED_STRING);
+    content_.asString.data = s->data;
+    content_.asString.size = s->length;
+  }
+
+  void setString(const char* s) {
+    ARDUINOJSON_ASSERT(s);
+    setType(VALUE_IS_LINKED_STRING);
+    content_.asString.data = s;
+    content_.asString.size = strlen(s);
   }
 
   CollectionData& toArray() {

diff --git a/src/ArduinoJson/Variant/VariantFunctions.hpp b/src/ArduinoJson/Variant/VariantFunctions.hpp
@@ -61,15 +61,15 @@ inline bool variantCopyFrom(VariantData* dst, const VariantData* src,
       auto dup = pool->saveString(str);
       if (!dup)
         return false;
-      dst->setString(JsonString(dup, str.size(), JsonString::Copied));
+      dst->setString(dup);
       return true;
     }
     case VALUE_IS_RAW_STRING: {
       auto str = adaptString(src->asRawString());
       auto dup = pool->saveString(str);
       if (!dup)
         return false;
-      dst->setRawString(dup, str.size());
+      dst->setRawString(dup);
       return true;
     }
     default:
@@ -121,13 +121,13 @@ inline void variantSetString(VariantData* var, TAdaptedString value,
   }
 
   if (value.isLinked()) {
-    var->setString(JsonString(value.data(), value.size(), JsonString::Linked));
+    var->setString(value.data());
     return;
   }
 
   auto dup = pool->saveString(value);
   if (dup)
-    var->setString(JsonString(dup, value.size(), JsonString::Copied));
+    var->setString(dup);
   else
     var->setNull();
 }
@@ -138,9 +138,9 @@ inline void variantSetRawString(VariantData* var, SerializedValue<T> value,
   if (!var)
     return;
   variantRelease(var, pool);
-  const char* dup = pool->saveString(adaptString(value.data(), value.size()));
+  auto dup = pool->saveString(adaptString(value.data(), value.size()));
   if (dup)
-    var->setRawString(dup, value.size());
+    var->setRawString(dup);
   else
     var->setNull();
 }

diff --git a/src/ArduinoJson/Variant/VariantImpl.hpp b/src/ArduinoJson/Variant/VariantImpl.hpp
@@ -137,4 +137,10 @@ inline void convertToJson(const VariantRefBase<TDerived>& src,
   dst.set(src.template as<JsonVariantConst>());
 }
 
+inline void VariantSlot::setKey(StringNode* k) {
+  ARDUINOJSON_ASSERT(k);
+  flags_ |= OWNED_KEY_BIT;
+  key_ = k->data;
+}
+
 ARDUINOJSON_END_PRIVATE_NAMESPACE
diff --git a/src/ArduinoJson/Variant/VariantSlot.hpp b/src/ArduinoJson/Variant/VariantSlot.hpp
@@ -11,6 +11,8 @@
 
 ARDUINOJSON_BEGIN_PRIVATE_NAMESPACE
 
+class StringNode;
+
 typedef int_t<ARDUINOJSON_SLOT_OFFSET_SIZE * 8>::type VariantSlotDiff;
 
 class VariantSlot {
@@ -76,15 +78,14 @@ class VariantSlot {
     next_ = VariantSlotDiff(slot - this);
   }
 
-  void setKey(JsonString k) {
+  void setKey(const char* k) {
     ARDUINOJSON_ASSERT(k);
-    if (k.isLinked())
-      flags_ &= VALUE_MASK;
-    else
-      flags_ |= OWNED_KEY_BIT;
-    key_ = k.c_str();
+    flags_ &= VALUE_MASK;
+    key_ = k;
   }
 
+  void setKey(StringNode* k);
+
   const char* key() const {
     return key_;
   }