Recycle removed slots

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

extras/tests/JsonArray/clear.cpp

@@ -5,6 +5,8 @@
 #include <ArduinoJson.h>
 #include <catch.hpp>
 
+#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<JsonArray>();
+
+    // 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
+    );
+  }
 }

extras/tests/JsonArray/remove.cpp

@@ -5,6 +5,8 @@
 #include <ArduinoJson.h>
 #include <catch.hpp>
 
+#include "Allocators.hpp"
+
 TEST_CASE("JsonArray::remove()") {
   JsonDocument doc;
   JsonArray array = doc.to<JsonArray>();
@@ -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<JsonArray>();
+
+  // 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
+  );
+}

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;
 

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

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);
   }

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

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