From 5faa3df43f54a1ea3bfffcdccc9e8e8d6a8379e5 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Mon, 20 Mar 2023 12:28:34 +0100 Subject: [PATCH] `MemoryPool` calls the `Allocator` directly --- extras/tests/Helpers/Allocators.hpp | 28 +++++++++ extras/tests/MemoryPool/StringCopier.cpp | 13 ++-- extras/tests/MemoryPool/allocVariant.cpp | 14 ++--- extras/tests/MemoryPool/clear.cpp | 3 +- extras/tests/MemoryPool/saveString.cpp | 9 +-- extras/tests/MemoryPool/size.cpp | 8 +-- extras/tests/Misc/Utf8.cpp | 3 +- src/ArduinoJson/Document/JsonDocument.hpp | 60 ++++-------------- src/ArduinoJson/Memory/MemoryPool.hpp | 74 ++++++++++++++++++++--- src/ArduinoJson/Polyfills/utility.hpp | 5 ++ 10 files changed, 131 insertions(+), 86 deletions(-) create mode 100644 extras/tests/Helpers/Allocators.hpp diff --git a/extras/tests/Helpers/Allocators.hpp b/extras/tests/Helpers/Allocators.hpp new file mode 100644 index 00000000..8bae2cd6 --- /dev/null +++ b/extras/tests/Helpers/Allocators.hpp @@ -0,0 +1,28 @@ +// ArduinoJson - https://arduinojson.org +// Copyright © 2014-2023, Benoit BLANCHON +// MIT License + +#pragma once + +#include + +struct FailingAllocator : ArduinoJson::Allocator { + static FailingAllocator* instance() { + static FailingAllocator allocator; + return &allocator; + } + + private: + FailingAllocator() = default; + ~FailingAllocator() = default; + + void* allocate(size_t) override { + return nullptr; + } + + void deallocate(void*) override {} + + void* reallocate(void*, size_t) override { + return nullptr; + } +}; diff --git a/extras/tests/MemoryPool/StringCopier.cpp b/extras/tests/MemoryPool/StringCopier.cpp index 0342bc4f..1d8c000a 100644 --- a/extras/tests/MemoryPool/StringCopier.cpp +++ b/extras/tests/MemoryPool/StringCopier.cpp @@ -8,10 +8,8 @@ using namespace ArduinoJson::detail; TEST_CASE("StringCopier") { - char buffer[4096]; - SECTION("Works when buffer is big enough") { - MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(5))); + MemoryPool pool(addPadding(JSON_STRING_SIZE(5))); StringCopier str(&pool); str.startString(); @@ -23,7 +21,7 @@ TEST_CASE("StringCopier") { } SECTION("Returns null when too small") { - MemoryPool pool(buffer, sizeof(void*)); + MemoryPool pool(sizeof(void*)); StringCopier str(&pool); str.startString(); @@ -34,7 +32,7 @@ TEST_CASE("StringCopier") { } SECTION("Increases size of memory pool") { - MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(6))); + MemoryPool pool(addPadding(JSON_STRING_SIZE(6))); StringCopier str(&pool); str.startString(); @@ -45,7 +43,7 @@ TEST_CASE("StringCopier") { } SECTION("Works when memory pool is 0 bytes") { - MemoryPool pool(buffer, 0); + MemoryPool pool(0); StringCopier str(&pool); str.startString(); @@ -62,8 +60,7 @@ static const char* addStringToPool(MemoryPool& pool, const char* s) { } TEST_CASE("StringCopier::save() deduplicates strings") { - char buffer[4096]; - MemoryPool pool(buffer, 4096); + MemoryPool pool(4096); SECTION("Basic") { const char* s1 = addStringToPool(pool, "hello"); diff --git a/extras/tests/MemoryPool/allocVariant.cpp b/extras/tests/MemoryPool/allocVariant.cpp index 5123a4e2..32f9d1d6 100644 --- a/extras/tests/MemoryPool/allocVariant.cpp +++ b/extras/tests/MemoryPool/allocVariant.cpp @@ -5,13 +5,13 @@ #include #include +#include "Allocators.hpp" + using namespace ArduinoJson::detail; TEST_CASE("MemoryPool::allocVariant()") { - char buffer[4096]; - SECTION("Returns different pointer") { - MemoryPool pool(buffer, sizeof(buffer)); + MemoryPool pool(4096); VariantSlot* s1 = pool.allocVariant(); REQUIRE(s1 != 0); @@ -22,26 +22,26 @@ TEST_CASE("MemoryPool::allocVariant()") { } SECTION("Returns aligned pointers") { - MemoryPool pool(buffer, sizeof(buffer)); + MemoryPool pool(4096); REQUIRE(isAligned(pool.allocVariant())); REQUIRE(isAligned(pool.allocVariant())); } SECTION("Returns zero if capacity is 0") { - MemoryPool pool(buffer, 0); + MemoryPool pool(0); REQUIRE(pool.allocVariant() == 0); } SECTION("Returns zero if buffer is null") { - MemoryPool pool(0, sizeof(buffer)); + MemoryPool pool(4096, FailingAllocator::instance()); REQUIRE(pool.allocVariant() == 0); } SECTION("Returns zero if capacity is insufficient") { - MemoryPool pool(buffer, sizeof(VariantSlot)); + MemoryPool pool(sizeof(VariantSlot)); pool.allocVariant(); diff --git a/extras/tests/MemoryPool/clear.cpp b/extras/tests/MemoryPool/clear.cpp index 9e33b471..a904819b 100644 --- a/extras/tests/MemoryPool/clear.cpp +++ b/extras/tests/MemoryPool/clear.cpp @@ -11,8 +11,7 @@ using namespace ArduinoJson::detail; static const size_t poolCapacity = 512; TEST_CASE("MemoryPool::clear()") { - char buffer[poolCapacity]; - MemoryPool pool(buffer, sizeof(buffer)); + MemoryPool pool(poolCapacity); SECTION("Discards allocated variants") { pool.allocVariant(); diff --git a/extras/tests/MemoryPool/saveString.cpp b/extras/tests/MemoryPool/saveString.cpp index ba8d5b91..81e1f881 100644 --- a/extras/tests/MemoryPool/saveString.cpp +++ b/extras/tests/MemoryPool/saveString.cpp @@ -6,6 +6,8 @@ #include #include +#include "Allocators.hpp" + using namespace ArduinoJson::detail; static const char* saveString(MemoryPool& pool, const char* s) { @@ -17,8 +19,7 @@ static const char* saveString(MemoryPool& pool, const char* s, size_t n) { } TEST_CASE("MemoryPool::saveString()") { - char buffer[32]; - MemoryPool pool(buffer, 32); + MemoryPool pool(32); SECTION("Duplicates different strings") { const char* a = saveString(pool, "hello"); @@ -72,12 +73,12 @@ TEST_CASE("MemoryPool::saveString()") { } SECTION("Returns NULL when buffer is NULL") { - MemoryPool pool2(0, 32); + MemoryPool pool2(32, FailingAllocator::instance()); REQUIRE(0 == saveString(pool2, "a")); } SECTION("Returns NULL when capacity is 0") { - MemoryPool pool2(buffer, 0); + MemoryPool pool2(0); REQUIRE(0 == saveString(pool2, "a")); } diff --git a/extras/tests/MemoryPool/size.cpp b/extras/tests/MemoryPool/size.cpp index 057f4ab3..1ac3ebb5 100644 --- a/extras/tests/MemoryPool/size.cpp +++ b/extras/tests/MemoryPool/size.cpp @@ -8,22 +8,20 @@ using namespace ArduinoJson::detail; TEST_CASE("MemoryPool::capacity()") { - char buffer[4096]; const size_t capacity = 64; - MemoryPool pool(buffer, capacity); + MemoryPool pool(capacity); REQUIRE(capacity == pool.capacity()); } TEST_CASE("MemoryPool::size()") { - char buffer[4096]; - MemoryPool pool(buffer, sizeof(buffer)); + MemoryPool pool(4096); SECTION("Initial size is 0") { REQUIRE(0 == pool.size()); } SECTION("Doesn't grow when memory pool is full") { - const size_t variantCount = sizeof(buffer) / sizeof(VariantSlot); + const size_t variantCount = pool.capacity() / sizeof(VariantSlot); for (size_t i = 0; i < variantCount; i++) pool.allocVariant(); diff --git a/extras/tests/Misc/Utf8.cpp b/extras/tests/Misc/Utf8.cpp index 0c62033d..beae262e 100644 --- a/extras/tests/Misc/Utf8.cpp +++ b/extras/tests/Misc/Utf8.cpp @@ -10,8 +10,7 @@ using namespace ArduinoJson::detail; static void testCodepoint(uint32_t codepoint, std::string expected) { - char buffer[4096]; - MemoryPool pool(buffer, 4096); + MemoryPool pool(4096); StringCopier str(&pool); str.startString(); diff --git a/src/ArduinoJson/Document/JsonDocument.hpp b/src/ArduinoJson/Document/JsonDocument.hpp index 9f74dfd3..4d1fbb20 100644 --- a/src/ArduinoJson/Document/JsonDocument.hpp +++ b/src/ArduinoJson/Document/JsonDocument.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -23,16 +24,16 @@ class JsonDocument : public detail::VariantOperators { public: explicit JsonDocument(size_t capa, Allocator* alloc = detail::DefaultAllocator::instance()) - : _allocator(alloc), _pool(allocPool(capa)) {} + : _pool(capa, alloc) {} // Copy-constructor JsonDocument(const JsonDocument& src) - : JsonDocument(src.capacity(), src._allocator) { + : JsonDocument(src.capacity(), src.allocator()) { set(src); } // Move-constructor - JsonDocument(JsonDocument&& src) : _allocator(src._allocator), _pool(0, 0) { + JsonDocument(JsonDocument&& src) : _pool(0, src.allocator()) { // TODO: use the copy and swap idiom moveAssignFrom(src); } @@ -57,10 +58,6 @@ class JsonDocument : public detail::VariantOperators { set(src); } - ~JsonDocument() { - freePool(); - } - JsonDocument& operator=(const JsonDocument& src) { // TODO: use the copy and swap idiom copyAssignFrom(src); @@ -77,26 +74,19 @@ class JsonDocument : public detail::VariantOperators { JsonDocument& operator=(const T& src) { size_t requiredSize = src.memoryUsage(); if (requiredSize > capacity()) - reallocPool(requiredSize); + _pool.reallocPool(requiredSize); set(src); return *this; } + Allocator* allocator() const { + return _pool.allocator(); + } + // Reduces the capacity of the memory pool to match the current usage. // https://arduinojson.org/v6/api/JsonDocument/shrinktofit/ void shrinkToFit() { - ptrdiff_t bytes_reclaimed = _pool.squash(); - if (bytes_reclaimed == 0) - return; - - void* old_ptr = _pool.buffer(); - void* new_ptr = _allocator->reallocate(old_ptr, _pool.capacity()); - - ptrdiff_t ptr_offset = - static_cast(new_ptr) - static_cast(old_ptr); - - _pool.movePointers(ptr_offset); - _data.movePointers(ptr_offset, ptr_offset - bytes_reclaimed); + _pool.shrinkToFit(_data); } // Reclaims the memory leaked when removing and replacing values. @@ -365,10 +355,6 @@ class JsonDocument : public detail::VariantOperators { } private: - void replacePool(detail::MemoryPool pool) { - _pool = pool; - } - JsonVariant getVariant() { return JsonVariant(&_pool, &_data); } @@ -377,36 +363,15 @@ class JsonDocument : public detail::VariantOperators { return JsonVariantConst(&_data); } - detail::MemoryPool allocPool(size_t requiredSize) { - size_t capa = detail::addPadding(requiredSize); - return {reinterpret_cast(_allocator->allocate(capa)), capa}; - } - - void reallocPool(size_t requiredSize) { - size_t capa = detail::addPadding(requiredSize); - if (capa == _pool.capacity()) - return; - freePool(); - replacePool(allocPool(detail::addPadding(requiredSize))); - } - - void freePool() { - auto p = getPool()->buffer(); - if (p) - _allocator->deallocate(p); - } - void copyAssignFrom(const JsonDocument& src) { - reallocPool(src.capacity()); + _pool.reallocPool(src.capacity()); set(src); } void moveAssignFrom(JsonDocument& src) { - freePool(); _data = src._data; - _pool = src._pool; src._data.setNull(); - src._pool = {0, 0}; + _pool = move(src._pool); } detail::MemoryPool* getPool() { @@ -425,7 +390,6 @@ class JsonDocument : public detail::VariantOperators { return &_data; } - Allocator* _allocator; detail::MemoryPool _pool; detail::VariantData _data; }; diff --git a/src/ArduinoJson/Memory/MemoryPool.hpp b/src/ArduinoJson/Memory/MemoryPool.hpp index d1996f1b..d8fe56b2 100644 --- a/src/ArduinoJson/Memory/MemoryPool.hpp +++ b/src/ArduinoJson/Memory/MemoryPool.hpp @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include #include @@ -36,15 +37,42 @@ ARDUINOJSON_BEGIN_PRIVATE_NAMESPACE class MemoryPool { public: - MemoryPool(char* buf, size_t capa) - : _begin(buf), - _left(buf), - _right(buf ? buf + capa : 0), - _end(buf ? buf + capa : 0), - _overflowed(false) { - ARDUINOJSON_ASSERT(isAligned(_begin)); - ARDUINOJSON_ASSERT(isAligned(_right)); - ARDUINOJSON_ASSERT(isAligned(_end)); + MemoryPool(size_t capa, Allocator* allocator = DefaultAllocator::instance()) + : _allocator(allocator), _overflowed(false) { + allocPool(addPadding(capa)); + } + + ~MemoryPool() { + if (_begin) + _allocator->deallocate(_begin); + } + + MemoryPool(const MemoryPool&) = delete; + MemoryPool& operator=(const MemoryPool& src) = delete; + + MemoryPool& operator=(MemoryPool&& src) { + if (_begin) + _allocator->deallocate(_begin); + _allocator = src._allocator; + _begin = src._begin; + _end = src._end; + _left = src._left; + _right = src._right; + _overflowed = src._overflowed; + src._begin = src._end = src._left = src._right = nullptr; + return *this; + } + + Allocator* allocator() const { + return _allocator; + } + + void reallocPool(size_t requiredSize) { + size_t capa = addPadding(requiredSize); + if (capa == capacity()) + return; + _allocator->deallocate(_begin); + allocPool(requiredSize); } void* buffer() { @@ -132,6 +160,23 @@ class MemoryPool { return p; } + void shrinkToFit(VariantData& variant) { + ptrdiff_t bytes_reclaimed = squash(); + if (bytes_reclaimed == 0) + return; + + void* old_ptr = _begin; + void* new_ptr = _allocator->reallocate(old_ptr, capacity()); + + ptrdiff_t ptr_offset = + static_cast(new_ptr) - static_cast(old_ptr); + + movePointers(ptr_offset); + reinterpret_cast(variant).movePointers( + ptr_offset, ptr_offset - bytes_reclaimed); + } + + private: // Squash the free space between strings and variants // // _begin _end @@ -166,7 +211,6 @@ class MemoryPool { _end += offset; } - private: void checkInvariants() { ARDUINOJSON_ASSERT(_begin <= _left); ARDUINOJSON_ASSERT(_left <= _right); @@ -215,6 +259,16 @@ class MemoryPool { return _right; } + void allocPool(size_t capa) { + auto buf = capa ? reinterpret_cast(_allocator->allocate(capa)) : 0; + _begin = _left = buf; + _end = _right = buf ? buf + capa : 0; + ARDUINOJSON_ASSERT(isAligned(_begin)); + ARDUINOJSON_ASSERT(isAligned(_right)); + ARDUINOJSON_ASSERT(isAligned(_end)); + } + + Allocator* _allocator; char *_begin, *_left, *_right, *_end; bool _overflowed; }; diff --git a/src/ArduinoJson/Polyfills/utility.hpp b/src/ArduinoJson/Polyfills/utility.hpp index 13fb3e70..4180f0f8 100644 --- a/src/ArduinoJson/Polyfills/utility.hpp +++ b/src/ArduinoJson/Polyfills/utility.hpp @@ -13,4 +13,9 @@ T&& forward(typename remove_reference::type& t) noexcept { return static_cast(t); } +template +typename remove_reference::type&& move(T&& t) { + return static_cast::type&&>(t); +} + ARDUINOJSON_END_PRIVATE_NAMESPACE