diff --git a/extras/tests/ResourceManager/CMakeLists.txt b/extras/tests/ResourceManager/CMakeLists.txt index bfa370bb..e8e1d9d1 100644 --- a/extras/tests/ResourceManager/CMakeLists.txt +++ b/extras/tests/ResourceManager/CMakeLists.txt @@ -9,6 +9,7 @@ add_executable(ResourceManagerTests shrinkToFit.cpp size.cpp StringBuilder.cpp + swap.cpp ) add_compile_definitions(ResourceManagerTests diff --git a/extras/tests/ResourceManager/swap.cpp b/extras/tests/ResourceManager/swap.cpp new file mode 100644 index 00000000..bfbc99f4 --- /dev/null +++ b/extras/tests/ResourceManager/swap.cpp @@ -0,0 +1,95 @@ +// ArduinoJson - https://arduinojson.org +// Copyright © 2014-2023, Benoit BLANCHON +// MIT License + +#include +#include +#include +#include + +#include "Allocators.hpp" + +using namespace ArduinoJson::detail; + +static void fullPreallocatedPools(ResourceManager& resources) { + for (int i = 0; + i < ARDUINOJSON_INITIAL_POOL_COUNT * ARDUINOJSON_POOL_CAPACITY; i++) + resources.allocSlot(); +} + +TEST_CASE("ResourceManager::swap()") { + SECTION("Both using preallocated pool list") { + SpyingAllocator allocator; + ResourceManager a(&allocator); + ResourceManager b(&allocator); + + auto a1 = a.allocSlot(); + auto b1 = b.allocSlot(); + + swap(a, b); + + REQUIRE(a1->data() == b.getSlot(a1.id())->data()); + REQUIRE(b1->data() == a.getSlot(b1.id())->data()); + + REQUIRE(allocator.log() == AllocatorLog() + << AllocatorLog::Allocate(sizeofPool()) * 2); + } + + SECTION("Only left using preallocated pool list") { + SpyingAllocator allocator; + ResourceManager a(&allocator); + ResourceManager b(&allocator); + fullPreallocatedPools(b); + + auto a1 = a.allocSlot(); + auto b1 = b.allocSlot(); + swap(a, b); + + REQUIRE(a1->data() == b.getSlot(a1.id())->data()); + REQUIRE(b1->data() == a.getSlot(b1.id())->data()); + + REQUIRE(allocator.log() == AllocatorLog() + << AllocatorLog::Allocate(sizeofPool()) * + (ARDUINOJSON_INITIAL_POOL_COUNT + 1) + << AllocatorLog::Allocate(sizeofPoolList( + ARDUINOJSON_INITIAL_POOL_COUNT * 2)) + << AllocatorLog::Allocate(sizeofPool())); + } + + SECTION("Only right using preallocated pool list") { + SpyingAllocator allocator; + ResourceManager a(&allocator); + fullPreallocatedPools(a); + ResourceManager b(&allocator); + + auto a1 = a.allocSlot(); + auto b1 = b.allocSlot(); + swap(a, b); + + REQUIRE(a1->data() == b.getSlot(a1.id())->data()); + REQUIRE(b1->data() == a.getSlot(b1.id())->data()); + + REQUIRE(allocator.log() == AllocatorLog() + << AllocatorLog::Allocate(sizeofPool()) * + ARDUINOJSON_INITIAL_POOL_COUNT + << AllocatorLog::Allocate(sizeofPoolList( + ARDUINOJSON_INITIAL_POOL_COUNT * 2)) + << AllocatorLog::Allocate(sizeofPool()) * 2); + } + + SECTION("None is using preallocated pool list") { + SpyingAllocator allocator; + ResourceManager a(&allocator); + fullPreallocatedPools(a); + ResourceManager b(&allocator); + fullPreallocatedPools(b); + + auto a1 = a.allocSlot(); + auto b1 = b.allocSlot(); + + swap(a, b); + + REQUIRE(a1->data() == b.getSlot(a1.id())->data()); + REQUIRE(b1->data() == a.getSlot(b1.id())->data()); + } +} diff --git a/src/ArduinoJson/Document/JsonDocument.hpp b/src/ArduinoJson/Document/JsonDocument.hpp index 15684ce5..22d4629e 100644 --- a/src/ArduinoJson/Document/JsonDocument.hpp +++ b/src/ArduinoJson/Document/JsonDocument.hpp @@ -30,9 +30,8 @@ class JsonDocument : public detail::VariantOperators { } // Move-constructor - JsonDocument(JsonDocument&& src) : resources_(src.allocator()) { - // TODO: use the copy and swap idiom - moveAssignFrom(src); + JsonDocument(JsonDocument&& src) : JsonDocument() { + swap(*this, src); } // Construct from variant, array, or object @@ -56,15 +55,8 @@ class JsonDocument : public detail::VariantOperators { set(src); } - JsonDocument& operator=(const JsonDocument& src) { - // TODO: use the copy and swap idiom - copyAssignFrom(src); - return *this; - } - - JsonDocument& operator=(JsonDocument&& src) { - // TODO: use the copy and swap idiom - moveAssignFrom(src); + JsonDocument& operator=(JsonDocument src) { + swap(*this, src); return *this; } @@ -87,11 +79,11 @@ class JsonDocument : public detail::VariantOperators { // Reclaims the memory leaked when removing and replacing values. // https://arduinojson.org/v6/api/jsondocument/garbagecollect/ bool garbageCollect() { - // make a temporary clone and move assign + // make a temporary clone and swap JsonDocument tmp(*this); if (tmp.overflowed()) return false; - moveAssignFrom(tmp); + swap(*this, tmp); return true; } @@ -346,6 +338,11 @@ class JsonDocument : public detail::VariantOperators { return getSlot(); } + friend void swap(JsonDocument& a, JsonDocument& b) { + swap(a.resources_, b.resources_); + swap_(a.data_, b.data_); + } + private: JsonVariant getSlot() { return JsonVariant(&data_, &resources_); @@ -355,16 +352,6 @@ class JsonDocument : public detail::VariantOperators { return JsonVariantConst(&data_, &resources_); } - void copyAssignFrom(const JsonDocument& src) { - set(src); - } - - void moveAssignFrom(JsonDocument& src) { - data_ = src.data_; - src.data_.reset(); - resources_ = move(src.resources_); - } - detail::ResourceManager* getResourceManager() { return &resources_; } diff --git a/src/ArduinoJson/Memory/ResourceManager.hpp b/src/ArduinoJson/Memory/ResourceManager.hpp index 5398e8a9..e6f31be7 100644 --- a/src/ArduinoJson/Memory/ResourceManager.hpp +++ b/src/ArduinoJson/Memory/ResourceManager.hpp @@ -29,14 +29,11 @@ class ResourceManager { ResourceManager(const ResourceManager&) = delete; ResourceManager& operator=(const ResourceManager& src) = delete; - ResourceManager& operator=(ResourceManager&& src) { - stringPool_.clear(allocator_); - variantPools_.clear(allocator_); - allocator_ = src.allocator_; - variantPools_ = detail::move(src.variantPools_); - overflowed_ = src.overflowed_; - stringPool_ = detail::move(src.stringPool_); - return *this; + friend void swap(ResourceManager& a, ResourceManager& b) { + swap(a.stringPool_, b.stringPool_); + swap(a.variantPools_, b.variantPools_); + swap_(a.allocator_, b.allocator_); + swap_(a.overflowed_, b.overflowed_); } Allocator* allocator() const { diff --git a/src/ArduinoJson/Memory/StringPool.hpp b/src/ArduinoJson/Memory/StringPool.hpp index 742f9f14..87ddfd7e 100644 --- a/src/ArduinoJson/Memory/StringPool.hpp +++ b/src/ArduinoJson/Memory/StringPool.hpp @@ -19,15 +19,14 @@ class StringPool { public: StringPool() = default; StringPool(const StringPool&) = delete; + void operator=(StringPool&& src) = delete; ~StringPool() { ARDUINOJSON_ASSERT(strings_ == nullptr); } - void operator=(StringPool&& src) { - ARDUINOJSON_ASSERT(strings_ == nullptr); - strings_ = src.strings_; - src.strings_ = nullptr; + friend void swap(StringPool& a, StringPool& b) { + swap_(a.strings_, b.strings_); } void clear(Allocator* allocator) { diff --git a/src/ArduinoJson/Memory/VariantPoolList.hpp b/src/ArduinoJson/Memory/VariantPoolList.hpp index 9cbf86fd..08096fad 100644 --- a/src/ArduinoJson/Memory/VariantPoolList.hpp +++ b/src/ArduinoJson/Memory/VariantPoolList.hpp @@ -19,6 +19,37 @@ class VariantPoolList { ARDUINOJSON_ASSERT(count_ == 0); } + friend void swap(VariantPoolList& a, VariantPoolList& b) { + bool aUsedPreallocated = a.pools_ == a.preallocatedPools_; + bool bUsedPreallocated = b.pools_ == b.preallocatedPools_; + + // Who is using preallocated pools? + if (aUsedPreallocated && bUsedPreallocated) { + // both of us => swap preallocated pools + for (PoolCount i = 0; i < ARDUINOJSON_INITIAL_POOL_COUNT; i++) + swap_(a.preallocatedPools_[i], b.preallocatedPools_[i]); + } else if (bUsedPreallocated) { + // only b => copy b's preallocated pools and give him a's pointer + for (PoolCount i = 0; i < b.count_; i++) + a.preallocatedPools_[i] = b.preallocatedPools_[i]; + b.pools_ = a.pools_; + a.pools_ = a.preallocatedPools_; + } else if (aUsedPreallocated) { + // only a => copy a's preallocated pools and give him b's pointer + for (PoolCount i = 0; i < a.count_; i++) + b.preallocatedPools_[i] = a.preallocatedPools_[i]; + a.pools_ = b.pools_; + b.pools_ = b.preallocatedPools_; + } else { + // neither => swap pointers + swap_(a.pools_, b.pools_); + } + + swap_(a.count_, b.count_); + swap_(a.capacity_, b.capacity_); + swap_(a.freeList_, b.freeList_); + } + VariantPoolList& operator=(VariantPoolList&& src) { ARDUINOJSON_ASSERT(count_ == 0); if (src.pools_ == src.preallocatedPools_) { diff --git a/src/ArduinoJson/Polyfills/utility.hpp b/src/ArduinoJson/Polyfills/utility.hpp index d96cafe7..cc0a17eb 100644 --- a/src/ArduinoJson/Polyfills/utility.hpp +++ b/src/ArduinoJson/Polyfills/utility.hpp @@ -20,4 +20,14 @@ typename remove_reference::type&& move(T&& t) { return static_cast::type&&>(t); } +// Polyfull for std::swap +// Don't use the name "swap" because it makes calls ambiguous for types in the +// detail namespace +template +void swap_(T& a, T& b) { + T tmp = move(a); + a = move(b); + b = move(tmp); +} + ARDUINOJSON_END_PRIVATE_NAMESPACE diff --git a/src/ArduinoJson/Variant/VariantData.hpp b/src/ArduinoJson/Variant/VariantData.hpp index d9fa4e03..f9618619 100644 --- a/src/ArduinoJson/Variant/VariantData.hpp +++ b/src/ArduinoJson/Variant/VariantData.hpp @@ -302,11 +302,6 @@ class VariantData { return var->nesting(resources); } - void operator=(const VariantData& src) { - content_ = src.content_; - flags_ = uint8_t((flags_ & OWNED_KEY_BIT) | (src.flags_ & ~OWNED_KEY_BIT)); - } - void removeElement(size_t index, ResourceManager* resources) { ArrayData::removeElement(asArray(), index, resources); }