diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ba30c69..d8e10dd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,3 +12,4 @@ HEAD * Remove `ARDUINOJSON_ENABLE_STRING_DEDUPLICATION` (string deduplication cannot be enabled anymore) * Remove `JsonDocument::capacity()` * Store the strings in the heap +* Reference-count shared strings diff --git a/extras/tests/JsonDocument/garbageCollect.cpp b/extras/tests/JsonDocument/garbageCollect.cpp index 5bf759b6..a32734fb 100644 --- a/extras/tests/JsonDocument/garbageCollect.cpp +++ b/extras/tests/JsonDocument/garbageCollect.cpp @@ -32,7 +32,6 @@ TEST_CASE("JsonDocument::garbageCollect()") { AllocatorLog() << AllocatorLog::Allocate(4096) << AllocatorLog::Allocate(sizeofString(7)) << AllocatorLog::Deallocate(sizeofString(7)) - << AllocatorLog::Deallocate(sizeofString(7)) << AllocatorLog::Deallocate(4096)); } @@ -46,7 +45,7 @@ TEST_CASE("JsonDocument::garbageCollect()") { bool result = doc.garbageCollect(); REQUIRE(result == false); - REQUIRE(doc.memoryUsage() == sizeofObject(2) + 2 * sizeofString(7)); + REQUIRE(doc.memoryUsage() == sizeofObject(2) + sizeofString(7)); REQUIRE(doc.as() == "{\"dancing\":2}"); REQUIRE(spyingAllocator.log() == AllocatorLog() diff --git a/extras/tests/JsonVariant/clear.cpp b/extras/tests/JsonVariant/clear.cpp index 4be139f9..02320a81 100644 --- a/extras/tests/JsonVariant/clear.cpp +++ b/extras/tests/JsonVariant/clear.cpp @@ -6,6 +6,9 @@ #include #include +using ArduinoJson::detail::sizeofArray; +using ArduinoJson::detail::sizeofString; + TEST_CASE("JsonVariant::clear()") { JsonDocument doc(4096); JsonVariant var = doc.to(); @@ -23,4 +26,12 @@ TEST_CASE("JsonVariant::clear()") { REQUIRE(var.isNull() == true); } + + SECTION("releases owned string") { + var.set(std::string("hello")); + REQUIRE(doc.memoryUsage() == sizeofString(5)); + + var.clear(); + REQUIRE(doc.memoryUsage() == 0); + } } diff --git a/extras/tests/JsonVariant/remove.cpp b/extras/tests/JsonVariant/remove.cpp index 493e69e2..785d3213 100644 --- a/extras/tests/JsonVariant/remove.cpp +++ b/extras/tests/JsonVariant/remove.cpp @@ -6,35 +6,67 @@ #include #include -TEST_CASE("JsonVariant::remove()") { +using ArduinoJson::detail::sizeofArray; +using ArduinoJson::detail::sizeofString; + +TEST_CASE("JsonVariant::remove(int)") { + JsonDocument doc(4096); + + SECTION("release top level strings") { + doc.add(std::string("hello")); + doc.add(std::string("hello")); + doc.add(std::string("world")); + + JsonVariant var = doc.as(); + REQUIRE(var.as() == "[\"hello\",\"hello\",\"world\"]"); + REQUIRE(doc.memoryUsage() == sizeofArray(3) + 2 * sizeofString(5)); + + var.remove(1); + REQUIRE(var.as() == "[\"hello\",\"world\"]"); + REQUIRE(doc.memoryUsage() == sizeofArray(3) + 2 * sizeofString(5)); + + var.remove(1); + REQUIRE(var.as() == "[\"hello\"]"); + REQUIRE(doc.memoryUsage() == sizeofArray(3) + 1 * sizeofString(5)); + + var.remove(0); + REQUIRE(var.as() == "[]"); + REQUIRE(doc.memoryUsage() == sizeofArray(3)); + } + + SECTION("release strings in nested array") { + doc[0][0] = std::string("hello"); + + JsonVariant var = doc.as(); + REQUIRE(var.as() == "[[\"hello\"]]"); + REQUIRE(doc.memoryUsage() == 2 * sizeofArray(1) + sizeofString(5)); + + var.remove(0); + REQUIRE(var.as() == "[]"); + REQUIRE(doc.memoryUsage() == 2 * sizeofArray(1)); + } +} + +TEST_CASE("JsonVariant::remove(const char *)") { JsonDocument doc(4096); JsonVariant var = doc.to(); - SECTION("remove(int)") { - var.add(1); - var.add(2); - var.add(3); + var["a"] = 1; + var["b"] = 2; - var.remove(1); + var.remove("a"); - REQUIRE(var.as() == "[1,3]"); - } - - SECTION("remove(const char *)") { - var["a"] = 1; - var["b"] = 2; - - var.remove("a"); - - REQUIRE(var.as() == "{\"b\":2}"); - } - - SECTION("remove(std::string)") { - var["a"] = 1; - var["b"] = 2; - - var.remove(std::string("b")); - - REQUIRE(var.as() == "{\"a\":1}"); - } + REQUIRE(var.as() == "{\"b\":2}"); +} + +TEST_CASE("JsonVariant::remove(std::string)") { + JsonDocument doc(4096); + JsonVariant var = doc.to(); + + var["a"] = 1; + var["b"] = 2; + + var.remove(std::string("b")); + + REQUIRE(var.as() == "{\"a\":1}"); } diff --git a/extras/tests/JsonVariant/set.cpp b/extras/tests/JsonVariant/set.cpp index adc49b00..17bed43e 100644 --- a/extras/tests/JsonVariant/set.cpp +++ b/extras/tests/JsonVariant/set.cpp @@ -7,6 +7,9 @@ #include "Allocators.hpp" +using ArduinoJson::detail::sizeofObject; +using ArduinoJson::detail::sizeofString; + enum ErrorCode { ERROR_01 = 1, ERROR_10 = 10 }; TEST_CASE("JsonVariant::set() when there is enough memory") { @@ -172,3 +175,36 @@ TEST_CASE("JsonVariant::set(JsonDocument)") { serializeJson(doc2, json); REQUIRE(json == "{\"hello\":\"world\"}"); } + +TEST_CASE("JsonVariant::set() releases the previous value") { + JsonDocument doc(1024); + doc["hello"] = std::string("world"); + REQUIRE(doc.memoryUsage() == sizeofObject(1) + sizeofString(5)); + + JsonVariant v = doc["hello"]; + + SECTION("int") { + v.set(42); + REQUIRE(doc.memoryUsage() == sizeofObject(1)); + } + + SECTION("bool") { + v.set(false); + REQUIRE(doc.memoryUsage() == sizeofObject(1)); + } + + SECTION("const char*") { + v.set("hello"); + REQUIRE(doc.memoryUsage() == sizeofObject(1)); + } + + SECTION("float") { + v.set(1.2); + REQUIRE(doc.memoryUsage() == sizeofObject(1)); + } + + SECTION("Serialized") { + v.set(serialized("[]")); + REQUIRE(doc.memoryUsage() == sizeofObject(1)); + } +} diff --git a/src/ArduinoJson/Memory/MemoryPool.hpp b/src/ArduinoJson/Memory/MemoryPool.hpp index 2a294f97..ee2f7402 100644 --- a/src/ArduinoJson/Memory/MemoryPool.hpp +++ b/src/ArduinoJson/Memory/MemoryPool.hpp @@ -28,6 +28,7 @@ constexpr size_t sizeofObject(size_t n) { struct StringNode { struct StringNode* next; uint16_t length; + uint16_t references; char data[1]; }; @@ -112,6 +113,7 @@ class MemoryPool { auto node = findString(str); if (node) { + node->references++; return node->data; } @@ -147,6 +149,7 @@ class MemoryPool { _allocator->allocate(sizeofString(length))); if (node) { node->length = uint16_t(length); + node->references = 1; } else { _overflowed = true; } @@ -170,6 +173,23 @@ class MemoryPool { _allocator->deallocate(node); } + void dereferenceString(const char* s) { + StringNode* prev = nullptr; + for (auto node = _strings; node; node = node->next) { + if (node->data == s) { + if (--node->references == 0) { + if (prev) + prev->next = node->next; + else + _strings = node->next; + _allocator->deallocate(node); + } + return; + } + prev = node; + } + } + void clear() { _right = _end; _overflowed = false; diff --git a/src/ArduinoJson/StringStorage/StringCopier.hpp b/src/ArduinoJson/StringStorage/StringCopier.hpp index 79c2bd39..4e451d39 100644 --- a/src/ArduinoJson/StringStorage/StringCopier.hpp +++ b/src/ArduinoJson/StringStorage/StringCopier.hpp @@ -33,6 +33,8 @@ class StringCopier { node = _pool->reallocString(_node, _size); _pool->addStringToList(node); _node = nullptr; // next time we need a new string + } else { + node->references++; } return JsonString(node->data, node->length, JsonString::Copied); } diff --git a/src/ArduinoJson/Variant/SlotFunctions.hpp b/src/ArduinoJson/Variant/SlotFunctions.hpp index efa3f76e..992dd4f5 100644 --- a/src/ArduinoJson/Variant/SlotFunctions.hpp +++ b/src/ArduinoJson/Variant/SlotFunctions.hpp @@ -24,6 +24,8 @@ inline VariantData* slotData(VariantSlot* slot) { inline void slotRelease(const VariantSlot* slot, MemoryPool* pool) { ARDUINOJSON_ASSERT(slot != nullptr); + if (slot->ownsKey()) + pool->dereferenceString(slot->key()); variantRelease(slot->data(), pool); } diff --git a/src/ArduinoJson/Variant/VariantFunctions.hpp b/src/ArduinoJson/Variant/VariantFunctions.hpp index 6ccfad1f..eb8e780e 100644 --- a/src/ArduinoJson/Variant/VariantFunctions.hpp +++ b/src/ArduinoJson/Variant/VariantFunctions.hpp @@ -33,6 +33,10 @@ inline typename TVisitor::result_type variantAccept(const VariantData* var, inline void variantRelease(const VariantData* var, MemoryPool* pool) { ARDUINOJSON_ASSERT(var != nullptr); + auto s = var->getOwnedString(); + if (s) + pool->dereferenceString(s); + auto c = var->asCollection(); if (c) { for (auto slot = c->head(); slot; slot = slot->next())