From 727a1013ca040ca19250dc2064c7eaa981000e70 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Thu, 20 Jul 2023 17:54:38 +0200 Subject: [PATCH] Recycle removed slots --- CHANGELOG.md | 1 + extras/tests/JsonArray/clear.cpp | 23 ++++++++++++++++++ extras/tests/JsonArray/remove.cpp | 24 +++++++++++++++++++ extras/tests/ResourceManager/allocVariant.cpp | 18 ++++++++++++++ src/ArduinoJson/Collection/CollectionImpl.hpp | 1 + src/ArduinoJson/Memory/ResourceManager.hpp | 4 ++++ src/ArduinoJson/Memory/VariantPoolImpl.hpp | 13 ++++++++++ src/ArduinoJson/Memory/VariantPoolList.hpp | 10 ++++++++ 8 files changed, 94 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 067bb3b6..148f2e19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,3 +20,4 @@ HEAD * Include `ARDUINOJSON_SLOT_OFFSET_SIZE` in the namespace name * Remove `JsonVariant::shallowCopy()` * `JsonDocument`'s capacity grows as needed, no need to pass it to the constructor anymore +* `JsonDocument`'s allocator is not monotonic anymore, removed values get recycled diff --git a/extras/tests/JsonArray/clear.cpp b/extras/tests/JsonArray/clear.cpp index faeac9a5..ec22ab9c 100644 --- a/extras/tests/JsonArray/clear.cpp +++ b/extras/tests/JsonArray/clear.cpp @@ -5,6 +5,8 @@ #include #include +#include "Allocators.hpp" + TEST_CASE("JsonArray::clear()") { SECTION("No-op on null JsonArray") { JsonArray array; @@ -22,4 +24,25 @@ TEST_CASE("JsonArray::clear()") { REQUIRE(array.size() == 0); REQUIRE(array.isNull() == false); } + + SECTION("Removed elements are recycled") { + SpyingAllocator allocator; + JsonDocument doc(&allocator); + JsonArray array = doc.to(); + + // fill the pool entirely + for (int i = 0; i < ARDUINOJSON_POOL_CAPACITY; i++) + array.add(i); + + // clear and fill again + array.clear(); + for (int i = 0; i < ARDUINOJSON_POOL_CAPACITY; i++) + array.add(i); + + REQUIRE( + allocator.log() == + AllocatorLog() << AllocatorLog::Allocate(sizeofPoolList()) + << AllocatorLog::Allocate(sizeofPool()) // only one pool + ); + } } diff --git a/extras/tests/JsonArray/remove.cpp b/extras/tests/JsonArray/remove.cpp index 4e002c59..acfe2114 100644 --- a/extras/tests/JsonArray/remove.cpp +++ b/extras/tests/JsonArray/remove.cpp @@ -5,6 +5,8 @@ #include #include +#include "Allocators.hpp" + TEST_CASE("JsonArray::remove()") { JsonDocument doc; JsonArray array = doc.to(); @@ -87,3 +89,25 @@ TEST_CASE("JsonArray::remove()") { unboundArray.remove(unboundArray.begin()); } } + +TEST_CASE("Removed elements are recycled") { + SpyingAllocator allocator; + JsonDocument doc(&allocator); + JsonArray array = doc.to(); + + // fill the pool entirely + for (int i = 0; i < ARDUINOJSON_POOL_CAPACITY; i++) + array.add(i); + + // free one slot in the pool + array.remove(0); + + // add one element; it should use the free slot + array.add(42); + + REQUIRE( + allocator.log() == + AllocatorLog() << AllocatorLog::Allocate(sizeofPoolList()) + << AllocatorLog::Allocate(sizeofPool()) // only one pool + ); +} diff --git a/extras/tests/ResourceManager/allocVariant.cpp b/extras/tests/ResourceManager/allocVariant.cpp index 99d55d46..fae36c6c 100644 --- a/extras/tests/ResourceManager/allocVariant.cpp +++ b/extras/tests/ResourceManager/allocVariant.cpp @@ -23,6 +23,24 @@ TEST_CASE("ResourceManager::allocSlot()") { REQUIRE(s1 != s2); } + SECTION("Returns the same slot after calling freeSlot()") { + ResourceManager resources; + + auto s1 = resources.allocSlot(); + auto s2 = resources.allocSlot(); + resources.freeSlot(s1); + resources.freeSlot(s2); + auto s3 = resources.allocSlot(); + auto s4 = resources.allocSlot(); + auto s5 = resources.allocSlot(); + + REQUIRE(s2.id() != s1.id()); + REQUIRE(s3.id() == s2.id()); + REQUIRE(s4.id() == s1.id()); + REQUIRE(s5.id() != s1.id()); + REQUIRE(s5.id() != s2.id()); + } + SECTION("Returns aligned pointers") { ResourceManager resources; diff --git a/src/ArduinoJson/Collection/CollectionImpl.hpp b/src/ArduinoJson/Collection/CollectionImpl.hpp index 916865d9..027a13ad 100644 --- a/src/ArduinoJson/Collection/CollectionImpl.hpp +++ b/src/ArduinoJson/Collection/CollectionImpl.hpp @@ -133,6 +133,7 @@ inline void CollectionData::releaseSlot(iterator it, if (it.ownsKey()) resources->dereferenceString(it.key()); it->setNull(resources); + resources->freeSlot(SlotWithId(it.slot_, it.currentId_)); } ARDUINOJSON_END_PRIVATE_NAMESPACE diff --git a/src/ArduinoJson/Memory/ResourceManager.hpp b/src/ArduinoJson/Memory/ResourceManager.hpp index 03d8d00a..5398e8a9 100644 --- a/src/ArduinoJson/Memory/ResourceManager.hpp +++ b/src/ArduinoJson/Memory/ResourceManager.hpp @@ -59,6 +59,10 @@ class ResourceManager { return p; } + void freeSlot(SlotWithId id) { + variantPools_.freeSlot(id); + } + VariantSlot* getSlot(SlotId id) const { return variantPools_.getSlot(id); } diff --git a/src/ArduinoJson/Memory/VariantPoolImpl.hpp b/src/ArduinoJson/Memory/VariantPoolImpl.hpp index 1d3461e1..c616efc3 100644 --- a/src/ArduinoJson/Memory/VariantPoolImpl.hpp +++ b/src/ArduinoJson/Memory/VariantPoolImpl.hpp @@ -61,4 +61,17 @@ inline size_t VariantPool::slotsToBytes(SlotCount n) { return n * sizeof(VariantSlot); } +inline SlotWithId VariantPoolList::allocFromFreeList() { + ARDUINOJSON_ASSERT(freeList_ != NULL_SLOT); + auto id = freeList_; + auto slot = getSlot(freeList_); + freeList_ = slot->next(); + return {new (slot) VariantSlot, id}; +} + +inline void VariantPoolList::freeSlot(SlotWithId slot) { + slot->setNext(freeList_); + freeList_ = slot.id(); +} + ARDUINOJSON_END_PRIVATE_NAMESPACE diff --git a/src/ArduinoJson/Memory/VariantPoolList.hpp b/src/ArduinoJson/Memory/VariantPoolList.hpp index 80eafa52..c92768ef 100644 --- a/src/ArduinoJson/Memory/VariantPoolList.hpp +++ b/src/ArduinoJson/Memory/VariantPoolList.hpp @@ -29,6 +29,11 @@ class VariantPoolList { } SlotWithId allocSlot(Allocator* allocator) { + // try to allocate from free list + if (freeList_ != NULL_SLOT) { + return allocFromFreeList(); + } + // try to allocate from last pool (other pools are full) if (pools_) { auto slot = allocFromLastPool(); @@ -44,6 +49,8 @@ class VariantPoolList { return allocFromLastPool(); } + void freeSlot(SlotWithId slot); + VariantSlot* getSlot(SlotId id) const { auto poolIndex = SlotId(id / ARDUINOJSON_POOL_CAPACITY); auto indexInPool = SlotId(id % ARDUINOJSON_POOL_CAPACITY); @@ -82,6 +89,8 @@ class VariantPoolList { } private: + SlotWithId allocFromFreeList(); + SlotWithId allocFromLastPool() { ARDUINOJSON_ASSERT(pools_ != nullptr); auto poolIndex = SlotId(count_ - 1); @@ -121,6 +130,7 @@ class VariantPoolList { VariantPool* pools_ = nullptr; size_t count_ = 0; size_t capacity_ = 0; + SlotId freeList_ = NULL_SLOT; }; ARDUINOJSON_END_PRIVATE_NAMESPACE