From d8d939660b6410d6c0f5c3e08309bbd9ac67bf86 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Fri, 19 Oct 2018 19:40:21 +0200 Subject: [PATCH] JsonArray::remove() and JsonObject::remove() now release the memory of the variant --- CHANGELOG.md | 1 + src/ArduinoJson/Data/ArrayFunctions.hpp | 11 ++-- src/ArduinoJson/Data/ObjectFunctions.hpp | 8 ++- src/ArduinoJson/Data/Slot.hpp | 3 +- src/ArduinoJson/Data/SlotFunctions.hpp | 12 ++++ src/ArduinoJson/JsonArray.hpp | 4 +- src/ArduinoJson/JsonObject.hpp | 4 +- .../Memory/AllocableInMemoryPool.hpp | 19 ------ src/ArduinoJson/Memory/DynamicMemoryPool.hpp | 2 +- src/ArduinoJson/Memory/MemoryPool.hpp | 28 +++++++++ src/ArduinoJson/Memory/StaticMemoryPool.hpp | 10 +-- test/DynamicMemoryPool/CMakeLists.txt | 1 + test/DynamicMemoryPool/allocSlot.cpp | 27 ++++++++ test/DynamicMemoryPool/size.cpp | 19 ++++++ test/JsonDocument/DynamicJsonDocument.cpp | 62 +++++++++++++++++++ 15 files changed, 174 insertions(+), 37 deletions(-) delete mode 100644 src/ArduinoJson/Memory/AllocableInMemoryPool.hpp create mode 100644 test/DynamicMemoryPool/allocSlot.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index b88a6d64..75813fb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ HEAD * Removed `JsonObject::is(k)` and `JsonObject::set(k,v)` * Replaced `T JsonArray::get(i)` with `JsonVariant JsonArray::get(i)` * Replaced `T JsonObject::get(k)` with `JsonVariant JsonObject::get(k)` +* `JsonArray::remove()` and `JsonObject::remove()` now release the memory of the variant v6.5.0-beta (2018-10-13) ----------- diff --git a/src/ArduinoJson/Data/ArrayFunctions.hpp b/src/ArduinoJson/Data/ArrayFunctions.hpp index 3b680f49..708265ba 100644 --- a/src/ArduinoJson/Data/ArrayFunctions.hpp +++ b/src/ArduinoJson/Data/ArrayFunctions.hpp @@ -13,10 +13,11 @@ namespace ARDUINOJSON_NAMESPACE { inline JsonVariantData* arrayAdd(JsonArrayData* arr, MemoryPool* pool) { if (!arr) return 0; - Slot* slot = new (pool) Slot(); + Slot* slot = pool->allocSlot(); if (!slot) return 0; slot->next = 0; + slot->value.type = JSON_NULL; if (arr->tail) { slot->prev = arr->tail; @@ -41,7 +42,7 @@ inline JsonVariantData* arrayGet(const JsonArrayData* arr, size_t index) { return slot ? &slot->value : 0; } -inline void arrayRemove(JsonArrayData* arr, Slot* slot) { +inline void arrayRemove(JsonArrayData* arr, Slot* slot, MemoryPool* pool) { if (!arr || !slot) return; if (slot->prev) @@ -52,10 +53,12 @@ inline void arrayRemove(JsonArrayData* arr, Slot* slot) { slot->next->prev = slot->prev; else arr->tail = slot->prev; + + slotFree(slot, pool); } -inline void arrayRemove(JsonArrayData* arr, size_t index) { - arrayRemove(arr, arrayGetSlot(arr, index)); +inline void arrayRemove(JsonArrayData* arr, size_t index, MemoryPool* pool) { + arrayRemove(arr, arrayGetSlot(arr, index), pool); } inline void arrayClear(JsonArrayData* arr) { diff --git a/src/ArduinoJson/Data/ObjectFunctions.hpp b/src/ArduinoJson/Data/ObjectFunctions.hpp index cb5a3457..30dc7338 100644 --- a/src/ArduinoJson/Data/ObjectFunctions.hpp +++ b/src/ArduinoJson/Data/ObjectFunctions.hpp @@ -4,6 +4,7 @@ #pragma once +#include "../Memory/MemoryPool.hpp" #include "JsonVariantData.hpp" #include "SlotFunctions.hpp" @@ -28,10 +29,11 @@ inline bool objectContainsKey(const JsonObjectData* obj, const TKey& key) { template inline JsonVariantData* objectAdd(JsonObjectData* obj, TKey key, MemoryPool* pool) { - Slot* slot = new (pool) Slot(); + Slot* slot = pool->allocSlot(); if (!slot) return 0; slot->next = 0; + slot->value.type = JSON_NULL; if (obj->tail) { slot->prev = obj->tail; @@ -74,7 +76,7 @@ inline void objectClear(JsonObjectData* obj) { obj->tail = 0; } -inline void objectRemove(JsonObjectData* obj, Slot* slot) { +inline void objectRemove(JsonObjectData* obj, Slot* slot, MemoryPool* pool) { if (!obj) return; if (!slot) return; if (slot->prev) @@ -85,6 +87,8 @@ inline void objectRemove(JsonObjectData* obj, Slot* slot) { slot->next->prev = slot->prev; else obj->tail = slot->prev; + + slotFree(slot, pool); } inline size_t objectSize(const JsonObjectData* obj) { diff --git a/src/ArduinoJson/Data/Slot.hpp b/src/ArduinoJson/Data/Slot.hpp index db603b95..7c8d113c 100644 --- a/src/ArduinoJson/Data/Slot.hpp +++ b/src/ArduinoJson/Data/Slot.hpp @@ -4,12 +4,11 @@ #pragma once -#include "../Memory/AllocableInMemoryPool.hpp" #include "JsonVariantData.hpp" namespace ARDUINOJSON_NAMESPACE { -struct Slot : AllocableInMemoryPool { +struct Slot { JsonVariantData value; struct Slot* next; struct Slot* prev; diff --git a/src/ArduinoJson/Data/SlotFunctions.hpp b/src/ArduinoJson/Data/SlotFunctions.hpp index 9c3dc4ad..283a2c60 100644 --- a/src/ArduinoJson/Data/SlotFunctions.hpp +++ b/src/ArduinoJson/Data/SlotFunctions.hpp @@ -4,6 +4,7 @@ #pragma once +#include "../Memory/MemoryPool.hpp" #include "../Strings/StringTypes.hpp" #include "JsonVariantData.hpp" #include "Slot.hpp" @@ -56,4 +57,15 @@ inline size_t slotSize(const Slot* slot) { } return n; } + +inline void slotFree(Slot* slot, MemoryPool* pool) { + const JsonVariantData& v = slot->value; + if (v.type == JSON_ARRAY || v.type == JSON_OBJECT) { + for (Slot* s = v.content.asObject.head; s; s = s->next) { + slotFree(s, pool); + } + } + + pool->freeSlot(slot); +} } // namespace ARDUINOJSON_NAMESPACE diff --git a/src/ArduinoJson/JsonArray.hpp b/src/ArduinoJson/JsonArray.hpp index 12a909ef..e37ca723 100644 --- a/src/ArduinoJson/JsonArray.hpp +++ b/src/ArduinoJson/JsonArray.hpp @@ -197,12 +197,12 @@ class JsonArray : public JsonArrayProxy, public Visitable { // Removes element at specified position. FORCE_INLINE void remove(iterator it) const { - arrayRemove(_data, it.internal()); + arrayRemove(_data, it.internal(), _memoryPool); } // Removes element at specified index. FORCE_INLINE void remove(size_t index) const { - arrayRemove(_data, index); + arrayRemove(_data, index, _memoryPool); } template diff --git a/src/ArduinoJson/JsonObject.hpp b/src/ArduinoJson/JsonObject.hpp index 9c0737ad..68d61852 100644 --- a/src/ArduinoJson/JsonObject.hpp +++ b/src/ArduinoJson/JsonObject.hpp @@ -227,7 +227,7 @@ class JsonObject : public JsonObjectProxy, public Visitable { } FORCE_INLINE void remove(iterator it) const { - objectRemove(_data, it.internal()); + objectRemove(_data, it.internal(), _memoryPool); } // Removes the specified key and the associated value. @@ -278,7 +278,7 @@ class JsonObject : public JsonObjectProxy, public Visitable { template FORCE_INLINE void remove_impl(TStringRef key) const { - objectRemove(_data, objectFindSlot(_data, key)); + objectRemove(_data, objectFindSlot(_data, key), _memoryPool); } MemoryPool* _memoryPool; diff --git a/src/ArduinoJson/Memory/AllocableInMemoryPool.hpp b/src/ArduinoJson/Memory/AllocableInMemoryPool.hpp deleted file mode 100644 index 7462bccc..00000000 --- a/src/ArduinoJson/Memory/AllocableInMemoryPool.hpp +++ /dev/null @@ -1,19 +0,0 @@ -// ArduinoJson - arduinojson.org -// Copyright Benoit Blanchon 2014-2018 -// MIT License - -#pragma once - -#include "MemoryPool.hpp" - -namespace ARDUINOJSON_NAMESPACE { - -class AllocableInMemoryPool { - public: - void *operator new(size_t n, MemoryPool *memoryPool) NOEXCEPT { - return memoryPool->alloc(n); - } - - void operator delete(void *, MemoryPool *)NOEXCEPT {} -}; -} // namespace ARDUINOJSON_NAMESPACE diff --git a/src/ArduinoJson/Memory/DynamicMemoryPool.hpp b/src/ArduinoJson/Memory/DynamicMemoryPool.hpp index d8c8af34..194c9281 100644 --- a/src/ArduinoJson/Memory/DynamicMemoryPool.hpp +++ b/src/ArduinoJson/Memory/DynamicMemoryPool.hpp @@ -58,7 +58,7 @@ class DynamicMemoryPoolBase : public MemoryPool { } // Gets the number of bytes occupied in the memoryPool - size_t size() const { + virtual size_t allocated_bytes() const { size_t total = 0; for (const Block* b = _head; b; b = b->next) total += b->size; return total; diff --git a/src/ArduinoJson/Memory/MemoryPool.hpp b/src/ArduinoJson/Memory/MemoryPool.hpp index dd9ca7bc..f0d07ee0 100644 --- a/src/ArduinoJson/Memory/MemoryPool.hpp +++ b/src/ArduinoJson/Memory/MemoryPool.hpp @@ -9,6 +9,7 @@ #include #include "../Configuration.hpp" +#include "../Data/Slot.hpp" #include "../Polyfills/attributes.hpp" namespace ARDUINOJSON_NAMESPACE { @@ -23,12 +24,36 @@ class MemoryPool { virtual char *realloc(char *oldPtr, size_t oldSize, size_t newSize) = 0; + Slot *allocSlot() { + if (_freeSlots) { + Slot *s = _freeSlots; + _freeSlots = s->next; + return s; + } + return reinterpret_cast(alloc(sizeof(Slot))); + } + + void freeSlot(Slot *slot) { + slot->next = _freeSlots; + _freeSlots = slot; + } + + size_t size() const { + size_t result = allocated_bytes(); + for (Slot *s = _freeSlots; s; s = s->next) result -= sizeof(Slot); + return result; + } + protected: + MemoryPool() : _freeSlots(0) {} + // CAUTION: NO VIRTUAL DESTRUCTOR! // If we add a virtual constructor the Arduino compiler will add malloc() // and free() to the binary, adding 706 useless bytes. ~MemoryPool() {} + virtual size_t allocated_bytes() const = 0; + // Preserve aligment if necessary static FORCE_INLINE size_t round_size_up(size_t bytes) { #if ARDUINOJSON_ENABLE_ALIGNMENT @@ -38,5 +63,8 @@ class MemoryPool { return bytes; #endif } + + private: + Slot *_freeSlots; }; } // namespace ARDUINOJSON_NAMESPACE diff --git a/src/ArduinoJson/Memory/StaticMemoryPool.hpp b/src/ArduinoJson/Memory/StaticMemoryPool.hpp index 9477952c..bbf3f8c9 100644 --- a/src/ArduinoJson/Memory/StaticMemoryPool.hpp +++ b/src/ArduinoJson/Memory/StaticMemoryPool.hpp @@ -18,11 +18,6 @@ class StaticMemoryPoolBase : public MemoryPool { return _capacity; } - // Gets the current usage of the memoryPool in bytes - size_t size() const { - return _size; - } - // Allocates the specified amount of bytes in the memoryPool virtual char* alloc(size_t bytes) { alignNextAlloc(); @@ -53,6 +48,11 @@ class StaticMemoryPoolBase : public MemoryPool { ~StaticMemoryPoolBase() {} + // Gets the current usage of the memoryPool in bytes + virtual size_t allocated_bytes() const { + return _size; + } + private: void alignNextAlloc() { _size = round_size_up(_size); diff --git a/test/DynamicMemoryPool/CMakeLists.txt b/test/DynamicMemoryPool/CMakeLists.txt index 35cf4133..063b3a9c 100644 --- a/test/DynamicMemoryPool/CMakeLists.txt +++ b/test/DynamicMemoryPool/CMakeLists.txt @@ -4,6 +4,7 @@ add_executable(DynamicMemoryPoolTests alloc.cpp + allocSlot.cpp no_memory.cpp size.cpp startString.cpp diff --git a/test/DynamicMemoryPool/allocSlot.cpp b/test/DynamicMemoryPool/allocSlot.cpp new file mode 100644 index 00000000..15d48e57 --- /dev/null +++ b/test/DynamicMemoryPool/allocSlot.cpp @@ -0,0 +1,27 @@ +// ArduinoJson - arduinojson.org +// Copyright Benoit Blanchon 2014-2018 +// MIT License + +#include +#include + +using namespace ARDUINOJSON_NAMESPACE; + +TEST_CASE("DynamicMemoryPool::allocSlot()") { + DynamicMemoryPool memoryPool; + + SECTION("Returns different pointer") { + Slot* s1 = memoryPool.allocSlot(); + Slot* s2 = memoryPool.allocSlot(); + + REQUIRE(s1 != s2); + } + + SECTION("Returns same pointer after freeSlot()") { + Slot* s1 = memoryPool.allocSlot(); + memoryPool.freeSlot(s1); + Slot* s2 = memoryPool.allocSlot(); + + REQUIRE(s1 == s2); + } +} diff --git a/test/DynamicMemoryPool/size.cpp b/test/DynamicMemoryPool/size.cpp index e6938f17..d6ef7abc 100644 --- a/test/DynamicMemoryPool/size.cpp +++ b/test/DynamicMemoryPool/size.cpp @@ -26,4 +26,23 @@ TEST_CASE("DynamicMemoryPool::size()") { memoryPool.clear(); REQUIRE(0 == memoryPool.size()); } + + SECTION("Increases after allocSlot()") { + memoryPool.allocSlot(); + REQUIRE(sizeof(Slot) == memoryPool.size()); + + memoryPool.allocSlot(); + REQUIRE(2 * sizeof(Slot) == memoryPool.size()); + } + + SECTION("Decreases after freeSlot()") { + Slot* s1 = memoryPool.allocSlot(); + Slot* s2 = memoryPool.allocSlot(); + + memoryPool.freeSlot(s1); + REQUIRE(sizeof(Slot) == memoryPool.size()); + + memoryPool.freeSlot(s2); + REQUIRE(0 == memoryPool.size()); + } } diff --git a/test/JsonDocument/DynamicJsonDocument.cpp b/test/JsonDocument/DynamicJsonDocument.cpp index 9aa16c3b..5700624a 100644 --- a/test/JsonDocument/DynamicJsonDocument.cpp +++ b/test/JsonDocument/DynamicJsonDocument.cpp @@ -92,4 +92,66 @@ TEST_CASE("DynamicJsonDocument") { REQUIRE(json == "{\"hello\":\"world\"}"); REQUIRE(ddoc.nestingLimit == 42); } + + SECTION("memoryUsage()") { + typedef ARDUINOJSON_NAMESPACE::Slot Slot; + + SECTION("Increases after adding value to array") { + JsonArray arr = doc.to(); + + arr.add(42); + REQUIRE(sizeof(Slot) == doc.memoryUsage()); + arr.add(43); + REQUIRE(2 * sizeof(Slot) == doc.memoryUsage()); + } + + SECTION("Decreases after remove value from array") { + JsonArray arr = doc.to(); + arr.add(42); + arr.add(43); + + arr.remove(1); + REQUIRE(sizeof(Slot) == doc.memoryUsage()); + arr.remove(0); + REQUIRE(0 == doc.memoryUsage()); + } + + SECTION("Increases after adding value to object") { + JsonObject obj = doc.to(); + + obj["a"] = 1; + REQUIRE(sizeof(Slot) == doc.memoryUsage()); + obj["b"] = 2; + REQUIRE(2 * sizeof(Slot) == doc.memoryUsage()); + } + + SECTION("Decreases after removing value from object") { + JsonObject obj = doc.to(); + obj["a"] = 1; + obj["b"] = 2; + + obj.remove("a"); + REQUIRE(sizeof(Slot) == doc.memoryUsage()); + obj.remove("b"); + REQUIRE(0 == doc.memoryUsage()); + } + + SECTION("Decreases after removing nested object from array") { + JsonArray arr = doc.to(); + JsonObject obj = arr.createNestedObject(); + obj["hello"] = "world"; + + arr.remove(0); + REQUIRE(0 == doc.memoryUsage()); + } + + SECTION("Decreases after removing nested array from object") { + JsonObject obj = doc.to(); + JsonArray arr = obj.createNestedArray("hello"); + arr.add("world"); + + obj.remove("hello"); + REQUIRE(0 == doc.memoryUsage()); + } + } }