From 167ea08c532c7e37315091b0f687aea05cc4f1ba Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Tue, 2 May 2023 18:29:19 +0200 Subject: [PATCH] Pass `StringNode*` to `VariantData` --- extras/tests/MemoryPool/StringCopier.cpp | 25 ++++++++----- extras/tests/MemoryPool/saveString.cpp | 36 ++++++++++++------- .../Collection/CollectionFunctions.hpp | 4 +-- src/ArduinoJson/Json/JsonDeserializer.hpp | 4 +-- src/ArduinoJson/Memory/MemoryPool.hpp | 6 ++-- .../MsgPack/MsgPackDeserializer.hpp | 4 +-- .../StringStorage/StringCopier.hpp | 4 +-- src/ArduinoJson/StringStorage/StringMover.hpp | 7 ++-- src/ArduinoJson/Variant/ConverterImpl.hpp | 4 +-- src/ArduinoJson/Variant/VariantData.hpp | 25 +++++++------ src/ArduinoJson/Variant/VariantFunctions.hpp | 12 +++---- src/ArduinoJson/Variant/VariantImpl.hpp | 6 ++++ src/ArduinoJson/Variant/VariantSlot.hpp | 13 +++---- 13 files changed, 90 insertions(+), 60 deletions(-) diff --git a/extras/tests/MemoryPool/StringCopier.cpp b/extras/tests/MemoryPool/StringCopier.cpp index 2988584c..29a78407 100644 --- 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 index 5ee88617..a78a2470 100644 --- a/extras/tests/MemoryPool/saveString.cpp +++ b/extras/tests/MemoryPool/saveString.cpp @@ -10,11 +10,11 @@ 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)); } @@ -22,35 +22,47 @@ 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 index 441fe27e..afa81a5a 100644 --- 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 index d90dc41c..ff21e2f2 100644 --- 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 index 0b0775a8..42df6c81 100644 --- a/src/ArduinoJson/Memory/MemoryPool.hpp +++ b/src/ArduinoJson/Memory/MemoryPool.hpp @@ -107,14 +107,14 @@ class MemoryPool { } template - 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 index eb4a6980..dde5256e 100644 --- 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 index 449ce253..c4f60d7a 100644 --- 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 index 584b2531..1b53c159 100644 --- 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 index 334a4cf1..3e4cc57d 100644 --- 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 index a6e85c60..fe5a8173 100644 --- 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 @@ -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 index dd2fe776..210a03fe 100644 --- a/src/ArduinoJson/Variant/VariantFunctions.hpp +++ b/src/ArduinoJson/Variant/VariantFunctions.hpp @@ -61,7 +61,7 @@ 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: { @@ -69,7 +69,7 @@ inline bool variantCopyFrom(VariantData* dst, const VariantData* src, 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 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 index 8a1c0de0..cb728330 100644 --- a/src/ArduinoJson/Variant/VariantImpl.hpp +++ b/src/ArduinoJson/Variant/VariantImpl.hpp @@ -137,4 +137,10 @@ inline void convertToJson(const VariantRefBase& src, dst.set(src.template as()); } +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 index 5bffba5d..90d5028e 100644 --- a/src/ArduinoJson/Variant/VariantSlot.hpp +++ b/src/ArduinoJson/Variant/VariantSlot.hpp @@ -11,6 +11,8 @@ ARDUINOJSON_BEGIN_PRIVATE_NAMESPACE +struct StringNode; + typedef int_t::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_; }