Store static strings in a dedicated pool

Because a slot id is smaller than a pointer, this change will ultimately allow reducing the slot size.
This commit is contained in:
Benoit Blanchon
2025-02-24 15:35:09 +01:00
parent 509807d3c2
commit cc077c1b63
36 changed files with 282 additions and 91 deletions

View File

@@ -54,7 +54,7 @@ TEST_CASE("BasicJsonDocument") {
doc["hello"] = "world";
auto copy = doc;
REQUIRE(copy.as<std::string>() == "{\"hello\":\"world\"}");
REQUIRE(allocatorLog == "AA");
REQUIRE(allocatorLog == "AAAA");
}
SECTION("capacity") {

View File

@@ -275,6 +275,12 @@ inline size_t sizeofPool(
return MemoryPool<VariantData>::slotsToBytes(n);
}
inline size_t sizeofStaticStringPool(
ArduinoJson::detail::SlotCount n = ARDUINOJSON_POOL_CAPACITY) {
using namespace ArduinoJson::detail;
return MemoryPool<const char*>::slotsToBytes(n);
}
inline size_t sizeofStringBuffer(size_t iteration = 1) {
// returns 31, 63, 127, 255, etc.
auto capacity = ArduinoJson::detail::StringBuilder::initialCapacity;

View File

@@ -56,6 +56,7 @@ TEST_CASE("JsonArray::add(T)") {
REQUIRE(array[0].is<int>() == false);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}

View File

@@ -104,6 +104,8 @@ TEST_CASE("deserializeJson(MemberProxy)") {
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.as<std::string>() == "{\"hello\":\"world\",\"value\":[42]}");
REQUIRE(spy.log() == AllocatorLog{});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofStaticStringPool()),
});
}
}

View File

@@ -825,7 +825,9 @@ TEST_CASE("shrink filter") {
deserializeJson(doc, "{}", DeserializationOption::Filter(filter));
REQUIRE(spy.log() == AllocatorLog{
Reallocate(sizeofPool(), sizeofObject(1)),
});
REQUIRE(spy.log() ==
AllocatorLog{
Reallocate(sizeofPool(), sizeofObject(1)),
Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(1)),
});
}

View File

@@ -31,6 +31,7 @@ TEST_CASE("ElementProxy::add()") {
REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}

View File

@@ -25,6 +25,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[42]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}
@@ -34,6 +35,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}
@@ -44,6 +46,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Allocate(sizeofString("world")),
});
}
@@ -55,8 +58,8 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Allocate(sizeofString("world")),
});
}
@@ -71,6 +74,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Allocate(sizeofString("world")),
});
}
@@ -399,7 +403,7 @@ TEST_CASE("MemberProxy under memory constraints") {
}
SECTION("value slot allocation fails") {
timebomb.setCountdown(1);
timebomb.setCountdown(2);
// fill the pool entirely, but leave one slot for the key
doc["foo"][ARDUINOJSON_POOL_CAPACITY - 4] = 1;
@@ -412,6 +416,7 @@ TEST_CASE("MemberProxy under memory constraints") {
REQUIRE(doc.overflowed() == true);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
AllocateFail(sizeofPool()),
});
}

View File

@@ -32,6 +32,7 @@ TEST_CASE("JsonDocument::add(T)") {
REQUIRE(doc.as<std::string>() == "[\"hello\"]");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}

View File

@@ -64,6 +64,7 @@ TEST_CASE("JsonDocument constructor") {
REQUIRE(doc2.as<std::string>() == "{\"hello\":\"world\"}");
REQUIRE(spyingAllocator.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}
@@ -87,6 +88,7 @@ TEST_CASE("JsonDocument constructor") {
REQUIRE(doc2.as<std::string>() == "[\"hello\"]");
REQUIRE(spyingAllocator.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}

View File

@@ -22,10 +22,10 @@ TEST_CASE("JsonDocument::remove()") {
SECTION("string literal") {
doc["a"] = 1;
doc["a\0b"_s] = 2;
doc["x"] = 2;
doc["b"] = 3;
doc.remove("a\0b");
doc.remove("x");
REQUIRE(doc.as<std::string>() == "{\"a\":1,\"b\":3}");
}

View File

@@ -37,7 +37,9 @@ TEST_CASE("JsonDocument::set()") {
doc.set("example");
REQUIRE(doc.as<const char*>() == "example"_s);
REQUIRE(spy.log() == AllocatorLog{});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofStaticStringPool()),
});
}
SECTION("const char*") {

View File

@@ -75,7 +75,11 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
doc.shrinkToFit();
REQUIRE(doc.as<std::string>() == "hello");
REQUIRE(spyingAllocator.log() == AllocatorLog{});
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofStaticStringPool()),
Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(1)),
});
}
SECTION("owned string") {
@@ -110,7 +114,9 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Reallocate(sizeofPool(), sizeofObject(1)),
Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(1)),
});
}
@@ -137,7 +143,9 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Reallocate(sizeofPool(), sizeofArray(1)),
Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(1)),
});
}
@@ -164,20 +172,23 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Reallocate(sizeofPool(), sizeofObject(1)),
Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(2)),
});
}
SECTION("owned string in object") {
doc["key"] = "abcdefg"_s;
doc["key1"_s] = "value"_s;
doc.shrinkToFit();
REQUIRE(doc.as<std::string>() == "{\"key\":\"abcdefg\"}");
REQUIRE(doc.as<std::string>() == "{\"key1\":\"value\"}");
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("abcdefg")),
Allocate(sizeofString("key1")),
Allocate(sizeofString("value")),
Reallocate(sizeofPool(), sizeofPool(2)),
});
}

View File

@@ -25,8 +25,6 @@ TEST_CASE("JsonDocument::operator[]") {
SECTION("string literal") {
REQUIRE(doc["abc"] == "ABC");
REQUIRE(cdoc["abc"] == "ABC");
REQUIRE(doc["abc\0d"] == "ABCD");
REQUIRE(cdoc["abc\0d"] == "ABCD");
}
SECTION("std::string") {
@@ -114,6 +112,7 @@ TEST_CASE("JsonDocument::operator[] key storage") {
REQUIRE(doc.as<std::string>() == "{\"hello\":0}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}

View File

@@ -26,25 +26,12 @@ TEST_CASE("JsonObject::set()") {
REQUIRE(obj2["hello"] == "world"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}
SECTION("copy local string value") {
obj1["hello"] = "world"_s;
spy.clearLog();
bool success = obj2.set(obj1);
REQUIRE(success == true);
REQUIRE(obj2["hello"] == "world"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}
SECTION("copy local key") {
obj1["hello"_s] = "world";
SECTION("copy local string key and value") {
obj1["hello"_s] = "world"_s;
spy.clearLog();
bool success = obj2.set(obj1);
@@ -54,6 +41,7 @@ TEST_CASE("JsonObject::set()") {
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
}
@@ -110,7 +98,7 @@ TEST_CASE("JsonObject::set()") {
}
SECTION("copy fails in the middle of an array") {
TimebombAllocator timebomb(1);
TimebombAllocator timebomb(2);
JsonDocument doc3(&timebomb);
JsonObject obj3 = doc3.to<JsonObject>();

View File

@@ -102,21 +102,25 @@ TEST_CASE("JsonObject::operator[]") {
REQUIRE(42 == obj[key]);
}
SECTION("should not duplicate const char*") {
SECTION("string literals") {
obj["hello"] = "world";
REQUIRE(spy.log() == AllocatorLog{Allocate(sizeofPool())});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}
SECTION("should duplicate char* value") {
obj["hello"] = const_cast<char*>("world");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Allocate(sizeofString("world")),
});
}
SECTION("should duplicate char* key") {
obj[const_cast<char*>("hello")] = "world";
obj[const_cast<char*>("hello")] = 42;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
@@ -136,12 +140,13 @@ TEST_CASE("JsonObject::operator[]") {
obj["hello"] = "world"_s;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
Allocate(sizeofString("world")),
});
}
SECTION("should duplicate std::string key") {
obj["hello"_s] = "world";
obj["hello"_s] = 42;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
@@ -158,7 +163,7 @@ TEST_CASE("JsonObject::operator[]") {
}
SECTION("should duplicate a non-static JsonString key") {
obj[JsonString("hello", false)] = "world";
obj[JsonString("hello", false)] = 42;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
@@ -166,9 +171,10 @@ TEST_CASE("JsonObject::operator[]") {
}
SECTION("should not duplicate a static JsonString key") {
obj[JsonString("hello", true)] = "world";
obj[JsonString("hello", true)] = 42;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofStaticStringPool()),
});
}

View File

@@ -38,13 +38,15 @@ TEST_CASE("JsonVariant::set(JsonVariant)") {
REQUIRE(var1.as<std::string>() == "{\"value\":[42]}");
}
SECTION("stores const char* by reference") {
SECTION("stores string literals by pointer") {
var1.set("hello!!");
spyingAllocator.clearLog();
var2.set(var1);
REQUIRE(spyingAllocator.log() == AllocatorLog{});
REQUIRE(spyingAllocator.log() == AllocatorLog{
Allocate(sizeofStaticStringPool()),
});
}
SECTION("stores char* by copy") {

View File

@@ -23,7 +23,9 @@ TEST_CASE("JsonVariant::set() when there is enough memory") {
REQUIRE(result == true);
CHECK(variant ==
"hello"_s); // linked string cannot contain '\0' at the moment
CHECK(spy.log() == AllocatorLog{});
CHECK(spy.log() == AllocatorLog{
Allocate(sizeofStaticStringPool()),
});
}
SECTION("const char*") {
@@ -149,7 +151,9 @@ TEST_CASE("JsonVariant::set() when there is enough memory") {
REQUIRE(result == true);
REQUIRE(variant == "world"); // stores by pointer
REQUIRE(spy.log() == AllocatorLog{});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofStaticStringPool()),
});
}
SECTION("non-static JsonString") {
@@ -265,6 +269,20 @@ TEST_CASE("JsonVariant::set() with not enough memory") {
JsonVariant v = doc.to<JsonVariant>();
SECTION("string literal") {
bool result = v.set("hello world");
REQUIRE(result == false);
REQUIRE(v.isNull());
}
SECTION("static JsonString") {
bool result = v.set(JsonString("hello world", true));
REQUIRE(result == false);
REQUIRE(v.isNull());
}
SECTION("std::string") {
bool result = v.set("hello world!!"_s);

View File

@@ -57,7 +57,7 @@ TEST_CASE("JsonVariantConst::operator[]") {
SECTION("string literal") {
REQUIRE(var["ab"] == "AB"_s);
REQUIRE(var["abc"] == "ABC"_s);
REQUIRE(var["abc\0d"] == "ABCD"_s);
REQUIRE(var["abc\0d"] == "ABC"_s);
REQUIRE(var["def"].isNull());
REQUIRE(var[0].isNull());
}

View File

@@ -21,7 +21,7 @@ TEST_CASE("adaptString()") {
auto s = adaptString("bravo\0alpha");
CHECK(s.isNull() == false);
CHECK(s.size() == 11);
CHECK(s.size() == 5);
CHECK(s.isStatic() == true);
}

View File

@@ -104,6 +104,8 @@ TEST_CASE("deserializeMsgPack(MemberProxy)") {
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.as<std::string>() == "{\"hello\":\"world\",\"value\":[42]}");
REQUIRE(spy.log() == AllocatorLog{});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofStaticStringPool()),
});
}
}

View File

@@ -5,6 +5,7 @@
add_executable(ResourceManagerTests
allocVariant.cpp
clear.cpp
saveStaticString.cpp
saveString.cpp
shrinkToFit.cpp
size.cpp

View File

@@ -2,7 +2,7 @@
// Copyright © 2014-2025, Benoit BLANCHON
// MIT License
#include <ArduinoJson/Memory/StringBuffer.hpp>
#include <ArduinoJson.hpp>
#include <catch.hpp>
#include "Allocators.hpp"
@@ -22,7 +22,7 @@ TEST_CASE("StringBuffer") {
sb.save(&variant);
REQUIRE(variant.type() == VariantType::TinyString);
REQUIRE(variant.asString() == "hi!");
REQUIRE(variant.asString(&resources) == "hi!");
}
SECTION("Tiny string can't contain NUL") {
@@ -32,7 +32,7 @@ TEST_CASE("StringBuffer") {
REQUIRE(variant.type() == VariantType::OwnedString);
auto str = variant.asString();
auto str = variant.asString(&resources);
REQUIRE(str.size() == 3);
REQUIRE(str.c_str()[0] == 'a');
REQUIRE(str.c_str()[1] == 0);
@@ -45,6 +45,6 @@ TEST_CASE("StringBuffer") {
sb.save(&variant);
REQUIRE(variant.type() == VariantType::OwnedString);
REQUIRE(variant.asString() == "alfa");
REQUIRE(variant.asString(&resources) == "alfa");
}
}

View File

@@ -2,7 +2,7 @@
// Copyright © 2014-2025, Benoit BLANCHON
// MIT License
#include <ArduinoJson/Memory/StringBuilder.hpp>
#include <ArduinoJson.hpp>
#include <catch.hpp>
#include "Allocators.hpp"
@@ -46,7 +46,7 @@ TEST_CASE("StringBuilder") {
REQUIRE(resources.overflowed() == false);
REQUIRE(data.type() == VariantType::TinyString);
REQUIRE(data.asString() == "url");
REQUIRE(data.asString(&resources) == "url");
}
SECTION("Short string fits in first allocation") {
@@ -134,9 +134,10 @@ TEST_CASE("StringBuilder::save() deduplicates strings") {
auto s2 = saveString(builder, "world");
auto s3 = saveString(builder, "hello");
REQUIRE(s1.asString() == "hello");
REQUIRE(s2.asString() == "world");
REQUIRE(+s1.asString().c_str() == +s3.asString().c_str()); // same address
REQUIRE(s1.asString(&resources) == "hello");
REQUIRE(s2.asString(&resources) == "world");
REQUIRE(+s1.asString(&resources).c_str() ==
+s3.asString(&resources).c_str()); // same address
REQUIRE(spy.log() ==
AllocatorLog{
@@ -152,10 +153,10 @@ TEST_CASE("StringBuilder::save() deduplicates strings") {
auto s1 = saveString(builder, "hello world");
auto s2 = saveString(builder, "hello");
REQUIRE(s1.asString() == "hello world");
REQUIRE(s2.asString() == "hello");
REQUIRE(+s2.asString().c_str() !=
+s1.asString().c_str()); // different address
REQUIRE(s1.asString(&resources) == "hello world");
REQUIRE(s2.asString(&resources) == "hello");
REQUIRE(+s2.asString(&resources).c_str() !=
+s1.asString(&resources).c_str()); // different address
REQUIRE(spy.log() ==
AllocatorLog{
@@ -170,10 +171,10 @@ TEST_CASE("StringBuilder::save() deduplicates strings") {
auto s1 = saveString(builder, "hello world");
auto s2 = saveString(builder, "worl");
REQUIRE(s1.asString() == "hello world");
REQUIRE(s2.asString() == "worl");
REQUIRE(s2.asString().c_str() !=
s1.asString().c_str()); // different address
REQUIRE(s1.asString(&resources) == "hello world");
REQUIRE(s2.asString(&resources) == "worl");
REQUIRE(s2.asString(&resources).c_str() !=
s1.asString(&resources).c_str()); // different address
REQUIRE(spy.log() ==
AllocatorLog{

View File

@@ -0,0 +1,47 @@
// ArduinoJson - https://arduinojson.org
// Copyright © 2014-2025, Benoit BLANCHON
// MIT License
#include <ArduinoJson/Memory/ResourceManager.hpp>
#include <ArduinoJson/Strings/StringAdapters.hpp>
#include <catch.hpp>
#include "Allocators.hpp"
using namespace ArduinoJson::detail;
TEST_CASE("ResourceManager::saveStaticString() deduplicates strings") {
SpyingAllocator spy;
ResourceManager resources(&spy);
auto str1 = "hello";
auto str2 = "world";
auto id1 = resources.saveStaticString(str1);
auto id2 = resources.saveStaticString(str2);
REQUIRE(id1 != id2);
auto id3 = resources.saveStaticString(str1);
REQUIRE(id1 == id3);
resources.shrinkToFit();
REQUIRE(spy.log() ==
AllocatorLog{
Allocate(sizeofStaticStringPool()),
Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(2)),
});
REQUIRE(resources.overflowed() == false);
}
TEST_CASE("ResourceManager::saveStaticString() when allocation fails") {
SpyingAllocator spy(FailingAllocator::instance());
ResourceManager resources(&spy);
auto slotId = resources.saveStaticString("hello");
REQUIRE(slotId == NULL_SLOT);
REQUIRE(resources.overflowed() == true);
REQUIRE(spy.log() == AllocatorLog{
AllocateFail(sizeofStaticStringPool()),
});
}