From c4b879645a79ed94d56178c699cd471c043acbd0 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Sun, 2 Apr 2023 16:47:37 +0200 Subject: [PATCH] Remove `JsonDocument::capacity()` --- CHANGELOG.md | 1 + extras/tests/JsonDocument/CMakeLists.txt | 1 - extras/tests/JsonDocument/assignment.cpp | 10 -- extras/tests/JsonDocument/capacity.cpp | 22 --- extras/tests/JsonDocument/constructor.cpp | 22 +-- extras/tests/JsonDocument/garbageCollect.cpp | 16 ++- extras/tests/JsonDocument/shrinkToFit.cpp | 133 ++++++++++++++----- extras/tests/JsonDocument/swap.cpp | 2 - src/ArduinoJson/Document/JsonDocument.hpp | 17 +-- 9 files changed, 130 insertions(+), 94 deletions(-) delete mode 100644 extras/tests/JsonDocument/capacity.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 7857b142..ef0be16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,3 +10,4 @@ HEAD * Merge `DynamicJsonDocument` with `JsonDocument` * Remove `JSON_ARRAY_SIZE()`, `JSON_OBJECT_SIZE()`, and `JSON_STRING_SIZE()` * Remove `ARDUINOJSON_ENABLE_STRING_DEDUPLICATION` (string deduplication cannot be enabled anymore) +* Remove `JsonDocument::capacity()` diff --git a/extras/tests/JsonDocument/CMakeLists.txt b/extras/tests/JsonDocument/CMakeLists.txt index 24097f6c..12aab318 100644 --- a/extras/tests/JsonDocument/CMakeLists.txt +++ b/extras/tests/JsonDocument/CMakeLists.txt @@ -5,7 +5,6 @@ add_executable(JsonDocumentTests add.cpp assignment.cpp - capacity.cpp cast.cpp compare.cpp constructor.cpp diff --git a/extras/tests/JsonDocument/assignment.cpp b/extras/tests/JsonDocument/assignment.cpp index 574ece9c..b8d40cdd 100644 --- a/extras/tests/JsonDocument/assignment.cpp +++ b/extras/tests/JsonDocument/assignment.cpp @@ -22,7 +22,6 @@ TEST_CASE("JsonDocument assignment") { doc2 = doc1; REQUIRE(doc2.as() == "{\"hello\":\"world\"}"); - REQUIRE(doc2.capacity() == doc1.capacity()); } REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate(1024) @@ -40,7 +39,6 @@ TEST_CASE("JsonDocument assignment") { doc2 = doc1; REQUIRE(doc2.as() == "{\"hello\":\"world\"}"); - REQUIRE(doc2.capacity() == doc1.capacity()); } REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate(4096) @@ -60,7 +58,6 @@ TEST_CASE("JsonDocument assignment") { doc2 = doc1; REQUIRE(doc2.as() == "{\"hello\":\"world\"}"); - REQUIRE(doc2.capacity() == doc1.capacity()); } REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate(1024) @@ -81,8 +78,6 @@ TEST_CASE("JsonDocument assignment") { REQUIRE(doc2.as() == "The size of this string is 32!!"); REQUIRE(doc1.as() == "null"); - REQUIRE(doc1.capacity() == 0); - REQUIRE(doc2.capacity() == 4096); } REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate(4096) @@ -100,7 +95,6 @@ TEST_CASE("JsonDocument assignment") { doc2 = obj; REQUIRE(doc2.as() == "{\"hello\":\"world\"}"); - REQUIRE(doc2.capacity() == 4096); } SECTION("Assign from JsonArray") { @@ -112,7 +106,6 @@ TEST_CASE("JsonDocument assignment") { doc2 = arr; REQUIRE(doc2.as() == "[\"hello\"]"); - REQUIRE(doc2.capacity() == 4096); } SECTION("Assign from JsonVariant") { @@ -123,7 +116,6 @@ TEST_CASE("JsonDocument assignment") { doc2 = doc1.as(); REQUIRE(doc2.as() == "42"); - REQUIRE(doc2.capacity() == 4096); } SECTION("Assign from MemberProxy") { @@ -134,7 +126,6 @@ TEST_CASE("JsonDocument assignment") { doc2 = doc1["value"]; REQUIRE(doc2.as() == "42"); - REQUIRE(doc2.capacity() == 4096); } SECTION("Assign from ElementProxy") { @@ -145,6 +136,5 @@ TEST_CASE("JsonDocument assignment") { doc2 = doc1[0]; REQUIRE(doc2.as() == "42"); - REQUIRE(doc2.capacity() == 4096); } } diff --git a/extras/tests/JsonDocument/capacity.cpp b/extras/tests/JsonDocument/capacity.cpp deleted file mode 100644 index b8d780b9..00000000 --- a/extras/tests/JsonDocument/capacity.cpp +++ /dev/null @@ -1,22 +0,0 @@ -// ArduinoJson - https://arduinojson.org -// Copyright © 2014-2023, Benoit BLANCHON -// MIT License - -#include -#include - -TEST_CASE("JsonDocument") { - JsonDocument doc(4096); - - SECTION("capacity()") { - SECTION("matches constructor argument") { - JsonDocument doc2(256); - REQUIRE(doc2.capacity() == 256); - } - - SECTION("rounds up constructor argument") { - JsonDocument doc2(253); - REQUIRE(doc2.capacity() == 256); - } - } -} diff --git a/extras/tests/JsonDocument/constructor.cpp b/extras/tests/JsonDocument/constructor.cpp index 59d73b06..a325a82b 100644 --- a/extras/tests/JsonDocument/constructor.cpp +++ b/extras/tests/JsonDocument/constructor.cpp @@ -28,7 +28,6 @@ TEST_CASE("JsonDocument constructor") { REQUIRE(doc1.as() == "The size of this string is 32!!"); REQUIRE(doc2.as() == "The size of this string is 32!!"); - REQUIRE(doc2.capacity() == 4096); } REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate(4096) @@ -46,8 +45,6 @@ TEST_CASE("JsonDocument constructor") { REQUIRE(doc2.as() == "The size of this string is 32!!"); REQUIRE(doc1.as() == "null"); - REQUIRE(doc1.capacity() == 0); - REQUIRE(doc2.capacity() == 4096); } REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate(4096) @@ -59,10 +56,11 @@ TEST_CASE("JsonDocument constructor") { JsonObject obj = doc1.to(); obj["hello"] = "world"; - JsonDocument doc2 = obj; + JsonDocument doc2(obj, &spyingAllocator); REQUIRE(doc2.as() == "{\"hello\":\"world\"}"); - REQUIRE(doc2.capacity() == addPadding(doc1.memoryUsage())); + REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate( + addPadding(doc1.memoryUsage()))); } SECTION("Construct from JsonArray") { @@ -70,19 +68,21 @@ TEST_CASE("JsonDocument constructor") { JsonArray arr = doc1.to(); arr.add("hello"); - JsonDocument doc2 = arr; + JsonDocument doc2(arr, &spyingAllocator); REQUIRE(doc2.as() == "[\"hello\"]"); - REQUIRE(doc2.capacity() == addPadding(doc1.memoryUsage())); + REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate( + addPadding(doc1.memoryUsage()))); } SECTION("Construct from JsonVariant") { JsonDocument doc1(200); - deserializeJson(doc1, "42"); + deserializeJson(doc1, "\"hello\""); - JsonDocument doc2 = doc1.as(); + JsonDocument doc2(doc1.as(), &spyingAllocator); - REQUIRE(doc2.as() == "42"); - REQUIRE(doc2.capacity() == addPadding(doc1.memoryUsage())); + REQUIRE(doc2.as() == "hello"); + REQUIRE(spyingAllocator.log() == AllocatorLog() << AllocatorLog::Allocate( + addPadding(doc1.memoryUsage()))); } } diff --git a/extras/tests/JsonDocument/garbageCollect.cpp b/extras/tests/JsonDocument/garbageCollect.cpp index 27e564db..6f949bd2 100644 --- a/extras/tests/JsonDocument/garbageCollect.cpp +++ b/extras/tests/JsonDocument/garbageCollect.cpp @@ -12,13 +12,12 @@ using ArduinoJson::detail::sizeofObject; TEST_CASE("JsonDocument::garbageCollect()") { - SpyingAllocator spyingAllocator; ControllableAllocator controllableAllocator; - JsonDocument doc(4096, &controllableAllocator); + SpyingAllocator spyingAllocator(&controllableAllocator); + JsonDocument doc(4096, &spyingAllocator); SECTION("when allocation succeeds") { deserializeJson(doc, "{\"blanket\":1,\"dancing\":2}"); - REQUIRE(doc.capacity() == 4096); REQUIRE(doc.memoryUsage() == sizeofObject(2) + 16); doc.remove("blanket"); @@ -26,13 +25,15 @@ TEST_CASE("JsonDocument::garbageCollect()") { REQUIRE(result == true); REQUIRE(doc.memoryUsage() == sizeofObject(1) + 8); - REQUIRE(doc.capacity() == 4096); REQUIRE(doc.as() == "{\"dancing\":2}"); + REQUIRE(spyingAllocator.log() == AllocatorLog() + << AllocatorLog::Allocate(4096) + << AllocatorLog::Allocate(4096) + << AllocatorLog::Deallocate(4096)); } SECTION("when allocation fails") { deserializeJson(doc, "{\"blanket\":1,\"dancing\":2}"); - REQUIRE(doc.capacity() == 4096); REQUIRE(doc.memoryUsage() == sizeofObject(2) + 16); doc.remove("blanket"); controllableAllocator.disable(); @@ -41,7 +42,10 @@ TEST_CASE("JsonDocument::garbageCollect()") { REQUIRE(result == false); REQUIRE(doc.memoryUsage() == sizeofObject(2) + 16); - REQUIRE(doc.capacity() == 4096); REQUIRE(doc.as() == "{\"dancing\":2}"); + + REQUIRE(spyingAllocator.log() == AllocatorLog() + << AllocatorLog::Allocate(4096) + << AllocatorLog::AllocateFail(4096)); } } diff --git a/extras/tests/JsonDocument/shrinkToFit.cpp b/extras/tests/JsonDocument/shrinkToFit.cpp index 58adeb6e..a43c8f03 100644 --- a/extras/tests/JsonDocument/shrinkToFit.cpp +++ b/extras/tests/JsonDocument/shrinkToFit.cpp @@ -8,8 +8,11 @@ #include // malloc, free #include +#include "Allocators.hpp" + using ArduinoJson::detail::sizeofArray; using ArduinoJson::detail::sizeofObject; +using ArduinoJson::detail::sizeofString; class ArmoredAllocator : public Allocator { public: @@ -46,87 +49,153 @@ class ArmoredAllocator : public Allocator { size_t _size; }; -void testShrinkToFit(JsonDocument& doc, std::string expected_json, - size_t expected_size) { - // test twice: shrinkToFit() should be idempotent - for (int i = 0; i < 2; i++) { - doc.shrinkToFit(); - - REQUIRE(doc.capacity() == expected_size); - REQUIRE(doc.memoryUsage() == expected_size); - - std::string json; - serializeJson(doc, json); - REQUIRE(json == expected_json); - } -} - TEST_CASE("JsonDocument::shrinkToFit()") { ArmoredAllocator armoredAllocator; - JsonDocument doc(4096, &armoredAllocator); + SpyingAllocator spyingAllocator(&armoredAllocator); + JsonDocument doc(4096, &spyingAllocator); SECTION("null") { - testShrinkToFit(doc, "null", 0); + doc.shrinkToFit(); + + REQUIRE(doc.as() == "null"); + REQUIRE(spyingAllocator.log() == AllocatorLog() + << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, 0)); } SECTION("empty object") { deserializeJson(doc, "{}"); - testShrinkToFit(doc, "{}", sizeofObject(0)); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "{}"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, sizeofObject(0))); } SECTION("empty array") { deserializeJson(doc, "[]"); - testShrinkToFit(doc, "[]", sizeofArray(0)); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "[]"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, sizeofArray(0))); } SECTION("linked string") { doc.set("hello"); - testShrinkToFit(doc, "\"hello\"", 0); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "hello"); + REQUIRE(spyingAllocator.log() == AllocatorLog() + << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, 0)); } SECTION("owned string") { doc.set(std::string("abcdefg")); - testShrinkToFit(doc, "\"abcdefg\"", 8); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "abcdefg"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, sizeofString(7))); } SECTION("linked raw") { doc.set(serialized("[{},123]")); - testShrinkToFit(doc, "[{},123]", 0); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "[{},123]"); + REQUIRE(spyingAllocator.log() == AllocatorLog() + << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, 0)); } SECTION("owned raw") { doc.set(serialized(std::string("[{},12]"))); - testShrinkToFit(doc, "[{},12]", 8); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "[{},12]"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, sizeofString(7))); } SECTION("linked key") { doc["key"] = 42; - testShrinkToFit(doc, "{\"key\":42}", sizeofObject(1)); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "{\"key\":42}"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, sizeofObject(1))); } SECTION("owned key") { doc[std::string("abcdefg")] = 42; - testShrinkToFit(doc, "{\"abcdefg\":42}", sizeofObject(1) + 8); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "{\"abcdefg\":42}"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate( + 4096, sizeofObject(1) + sizeofString(7))); } SECTION("linked string in array") { doc.add("hello"); - testShrinkToFit(doc, "[\"hello\"]", sizeofArray(1)); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "[\"hello\"]"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, sizeofArray(1))); } SECTION("owned string in array") { doc.add(std::string("abcdefg")); - testShrinkToFit(doc, "[\"abcdefg\"]", sizeofArray(1) + 8); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "[\"abcdefg\"]"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate( + 4096, sizeofArray(1) + sizeofString(7))); } SECTION("linked string in object") { doc["key"] = "hello"; - testShrinkToFit(doc, "{\"key\":\"hello\"}", sizeofObject(1)); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "{\"key\":\"hello\"}"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate(4096, sizeofObject(1))); } SECTION("owned string in object") { doc["key"] = std::string("abcdefg"); - testShrinkToFit(doc, "{\"key\":\"abcdefg\"}", sizeofArray(1) + 8); + + doc.shrinkToFit(); + + REQUIRE(doc.as() == "{\"key\":\"abcdefg\"}"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate( + 4096, sizeofObject(1) + sizeofString(7))); } SECTION("unaligned") { @@ -136,8 +205,10 @@ TEST_CASE("JsonDocument::shrinkToFit()") { doc.shrinkToFit(); // the new capacity should be padded to align the pointers - REQUIRE(doc.capacity() == sizeofObject(1) + sizeof(void*)); - REQUIRE(doc.memoryUsage() == sizeofObject(1) + 2); REQUIRE(doc[0] == "?"); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(4096) + << AllocatorLog::Reallocate( + 4096, sizeofArray(1) + sizeof(void*))); } } diff --git a/extras/tests/JsonDocument/swap.cpp b/extras/tests/JsonDocument/swap.cpp index 86106bb5..6dd5af56 100644 --- a/extras/tests/JsonDocument/swap.cpp +++ b/extras/tests/JsonDocument/swap.cpp @@ -19,9 +19,7 @@ TEST_CASE("std::swap") { swap(doc1, doc2); - CHECK(doc1.capacity() == 0x20); CHECK(doc1.as() == "world"); - CHECK(doc2.capacity() == 0x10); CHECK(doc2.as() == "hello"); } } diff --git a/src/ArduinoJson/Document/JsonDocument.hpp b/src/ArduinoJson/Document/JsonDocument.hpp index 4d1fbb20..0521f2f1 100644 --- a/src/ArduinoJson/Document/JsonDocument.hpp +++ b/src/ArduinoJson/Document/JsonDocument.hpp @@ -28,7 +28,7 @@ class JsonDocument : public detail::VariantOperators { // Copy-constructor JsonDocument(const JsonDocument& src) - : JsonDocument(src.capacity(), src.allocator()) { + : JsonDocument(src._pool.capacity(), src.allocator()) { set(src); } @@ -41,6 +41,7 @@ class JsonDocument : public detail::VariantOperators { // Construct from variant, array, or object template JsonDocument(const T& src, + Allocator* alloc = detail::DefaultAllocator::instance(), typename detail::enable_if< detail::is_same::value || detail::is_same::value || @@ -48,7 +49,7 @@ class JsonDocument : public detail::VariantOperators { detail::is_same::value || detail::is_same::value || detail::is_same::value>::type* = 0) - : JsonDocument(src.memoryUsage()) { + : JsonDocument(src.memoryUsage(), alloc) { set(src); } @@ -73,7 +74,7 @@ class JsonDocument : public detail::VariantOperators { template JsonDocument& operator=(const T& src) { size_t requiredSize = src.memoryUsage(); - if (requiredSize > capacity()) + if (requiredSize > _pool.capacity()) _pool.reallocPool(requiredSize); set(src); return *this; @@ -94,7 +95,7 @@ class JsonDocument : public detail::VariantOperators { bool garbageCollect() { // make a temporary clone and move assign JsonDocument tmp(*this); - if (!tmp.capacity()) + if (!tmp._pool.capacity()) return false; tmp.set(*this); moveAssignFrom(tmp); @@ -160,12 +161,6 @@ class JsonDocument : public detail::VariantOperators { return variantNesting(&_data); } - // Returns the capacity of the memory pool. - // https://arduinojson.org/v6/api/jsondocument/capacity/ - size_t capacity() const { - return _pool.capacity(); - } - // Returns the number of elements in the root array or object. // https://arduinojson.org/v6/api/jsondocument/size/ size_t size() const { @@ -364,7 +359,7 @@ class JsonDocument : public detail::VariantOperators { } void copyAssignFrom(const JsonDocument& src) { - _pool.reallocPool(src.capacity()); + _pool.reallocPool(src._pool.capacity()); set(src); }