Store static string in a dedicated pool

bblanchon · bblanchon · commit 84008478e2fc · 2025-02-24T18:18:45.000+01:00
diff --git a/extras/tests/Deprecated/BasicJsonDocument.cpp b/extras/tests/Deprecated/BasicJsonDocument.cpp
@@ -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") {
diff --git a/extras/tests/Helpers/Allocators.hpp b/extras/tests/Helpers/Allocators.hpp
@@ -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;
diff --git a/extras/tests/JsonArray/add.cpp b/extras/tests/JsonArray/add.cpp
@@ -56,6 +56,7 @@ TEST_CASE("JsonArray::add(T)") {
     REQUIRE(array[0].is<int>() == false);
     REQUIRE(spy.log() == AllocatorLog{
                              Allocate(sizeofPool()),
+                             Allocate(sizeofStaticStringPool()),
                          });
   }
 
diff --git a/extras/tests/JsonDeserializer/destination_types.cpp b/extras/tests/JsonDeserializer/destination_types.cpp
@@ -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()),
+                         });
   }
 }
diff --git a/extras/tests/JsonDeserializer/filter.cpp b/extras/tests/JsonDeserializer/filter.cpp
@@ -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)),
+          });
 }
diff --git a/extras/tests/JsonDocument/ElementProxy.cpp b/extras/tests/JsonDocument/ElementProxy.cpp
@@ -31,6 +31,7 @@ TEST_CASE("ElementProxy::add()") {
     REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
     REQUIRE(spy.log() == AllocatorLog{
                              Allocate(sizeofPool()),
+                             Allocate(sizeofStaticStringPool()),
                          });
   }
 
diff --git a/extras/tests/JsonDocument/MemberProxy.cpp b/extras/tests/JsonDocument/MemberProxy.cpp
@@ -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()),
                          });
   }
diff --git a/extras/tests/JsonDocument/add.cpp b/extras/tests/JsonDocument/add.cpp
@@ -32,6 +32,7 @@ TEST_CASE("JsonDocument::add(T)") {
     REQUIRE(doc.as<std::string>() == "[\"hello\"]");
     REQUIRE(spy.log() == AllocatorLog{
                              Allocate(sizeofPool()),
+                             Allocate(sizeofStaticStringPool()),
                          });
   }
 
diff --git a/extras/tests/JsonDocument/constructor.cpp b/extras/tests/JsonDocument/constructor.cpp
@@ -62,6 +62,7 @@ TEST_CASE("JsonDocument constructor") {
     REQUIRE(doc2.as<std::string>() == "{\"hello\":\"world\"}");
     REQUIRE(spyingAllocator.log() == AllocatorLog{
                                          Allocate(sizeofPool()),
+                                         Allocate(sizeofStaticStringPool()),
                                      });
   }
 
@@ -85,6 +86,7 @@ TEST_CASE("JsonDocument constructor") {
     REQUIRE(doc2.as<std::string>() == "[\"hello\"]");
     REQUIRE(spyingAllocator.log() == AllocatorLog{
                                          Allocate(sizeofPool()),
+                                         Allocate(sizeofStaticStringPool()),
                                      });
   }
 
diff --git a/extras/tests/JsonDocument/set.cpp b/extras/tests/JsonDocument/set.cpp
@@ -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*") {
diff --git a/extras/tests/JsonDocument/shrinkToFit.cpp b/extras/tests/JsonDocument/shrinkToFit.cpp
@@ -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["key"_s] = "value"_s;
 
     doc.shrinkToFit();
 
-    REQUIRE(doc.as<std::string>() == "{\"key\":\"abcdefg\"}");
+    REQUIRE(doc.as<std::string>() == "{\"key\":\"value\"}");
     REQUIRE(spyingAllocator.log() ==
             AllocatorLog{
                 Allocate(sizeofPool()),
-                Allocate(sizeofString("abcdefg")),
+                Allocate(sizeofString("key")),
+                Allocate(sizeofString("value")),
                 Reallocate(sizeofPool(), sizeofPool(2)),
             });
   }
diff --git a/extras/tests/JsonDocument/subscript.cpp b/extras/tests/JsonDocument/subscript.cpp
@@ -112,6 +112,7 @@ TEST_CASE("JsonDocument::operator[] key storage") {
     REQUIRE(doc.as<std::string>() == "{\"hello\":0}");
     REQUIRE(spy.log() == AllocatorLog{
                              Allocate(sizeofPool()),
+                             Allocate(sizeofStaticStringPool()),
                          });
   }
 
diff --git a/extras/tests/JsonObject/set.cpp b/extras/tests/JsonObject/set.cpp
@@ -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>();
 
diff --git a/extras/tests/JsonObject/subscript.cpp b/extras/tests/JsonObject/subscript.cpp
@@ -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,17 +163,18 @@ 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")),
                          });
   }
 
   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()),
                          });
   }
 
diff --git a/extras/tests/JsonVariant/copy.cpp b/extras/tests/JsonVariant/copy.cpp
@@ -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") {
diff --git a/extras/tests/JsonVariant/set.cpp b/extras/tests/JsonVariant/set.cpp
@@ -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*") {
@@ -137,7 +139,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") {
diff --git a/extras/tests/MsgPackDeserializer/destination_types.cpp b/extras/tests/MsgPackDeserializer/destination_types.cpp
@@ -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()),
+                         });
   }
 }
diff --git a/src/ArduinoJson/Memory/MemoryPool.hpp b/src/ArduinoJson/Memory/MemoryPool.hpp
@@ -34,6 +34,11 @@ class Slot {
     return ptr_;
   }
 
+  T& operator*() const {
+    ARDUINOJSON_ASSERT(ptr_ != nullptr);
+    return *ptr_;
+  }
+
   T* operator->() const {
     ARDUINOJSON_ASSERT(ptr_ != nullptr);
     return ptr_;
diff --git a/src/ArduinoJson/Memory/ResourceManager.hpp b/src/ArduinoJson/Memory/ResourceManager.hpp
diff --git a/src/ArduinoJson/Object/JsonPair.hpp b/src/ArduinoJson/Object/JsonPair.hpp
diff --git a/src/ArduinoJson/Object/ObjectImpl.hpp b/src/ArduinoJson/Object/ObjectImpl.hpp
diff --git a/src/ArduinoJson/Variant/ConverterImpl.hpp b/src/ArduinoJson/Variant/ConverterImpl.hpp
diff --git a/src/ArduinoJson/Variant/VariantContent.hpp b/src/ArduinoJson/Variant/VariantContent.hpp
diff --git a/src/ArduinoJson/Variant/VariantData.hpp b/src/ArduinoJson/Variant/VariantData.hpp
diff --git a/src/ArduinoJson/Variant/VariantImpl.hpp b/src/ArduinoJson/Variant/VariantImpl.hpp

Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ TEST_CASE("BasicJsonDocument") {`
`54`	`54`	`doc["hello"] = "world";`
`55`	`55`	`auto copy = doc;`
`56`	`56`	`REQUIRE(copy.as<std::string>() == "{\"hello\":\"world\"}");`
`57`		`- REQUIRE(allocatorLog == "AA");`
	`57`	`+ REQUIRE(allocatorLog == "AAAA");`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`SECTION("capacity") {`
Original file line number	Diff line number	Diff line change
`@@ -56,6 +56,7 @@ TEST_CASE("JsonArray::add(T)") {`
`56`	`56`	`REQUIRE(array[0].is<int>() == false);`
`57`	`57`	`REQUIRE(spy.log() == AllocatorLog{`
`58`	`58`	`Allocate(sizeofPool()),`
	`59`	`+ Allocate(sizeofStaticStringPool()),`
`59`	`60`	`});`
`60`	`61`	`}`
`61`	`62`
Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,8 @@ TEST_CASE("deserializeJson(MemberProxy)") {`
`104`	`104`
`105`	`105`	`REQUIRE(err == DeserializationError::Ok);`
`106`	`106`	`REQUIRE(doc.as<std::string>() == "{\"hello\":\"world\",\"value\":[42]}");`
`107`		`- REQUIRE(spy.log() == AllocatorLog{});`
	`107`	`+ REQUIRE(spy.log() == AllocatorLog{`
	`108`	`+ Allocate(sizeofStaticStringPool()),`
	`109`	`+ });`
`108`	`110`	`}`
`109`	`111`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,6 +31,7 @@ TEST_CASE("ElementProxy::add()") {`
`31`	`31`	`REQUIRE(doc.as<std::string>() == "[[\"world\"]]");`
`32`	`32`	`REQUIRE(spy.log() == AllocatorLog{`
`33`	`33`	`Allocate(sizeofPool()),`
	`34`	`+ Allocate(sizeofStaticStringPool()),`
`34`	`35`	`});`
`35`	`36`	`}`
`36`	`37`
Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,7 @@ TEST_CASE("JsonDocument::add(T)") {`
`32`	`32`	`REQUIRE(doc.as<std::string>() == "[\"hello\"]");`
`33`	`33`	`REQUIRE(spy.log() == AllocatorLog{`
`34`	`34`	`Allocate(sizeofPool()),`
	`35`	`+ Allocate(sizeofStaticStringPool()),`
`35`	`36`	`});`
`36`	`37`	`}`
`37`	`38`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ TEST_CASE("JsonDocument constructor") {`
`62`	`62`	`REQUIRE(doc2.as<std::string>() == "{\"hello\":\"world\"}");`
`63`	`63`	`REQUIRE(spyingAllocator.log() == AllocatorLog{`
`64`	`64`	`Allocate(sizeofPool()),`
	`65`	`+ Allocate(sizeofStaticStringPool()),`
`65`	`66`	`});`
`66`	`67`	`}`
`67`	`68`
`@@ -85,6 +86,7 @@ TEST_CASE("JsonDocument constructor") {`
`85`	`86`	`REQUIRE(doc2.as<std::string>() == "[\"hello\"]");`
`86`	`87`	`REQUIRE(spyingAllocator.log() == AllocatorLog{`
`87`	`88`	`Allocate(sizeofPool()),`
	`89`	`+ Allocate(sizeofStaticStringPool()),`
`88`	`90`	`});`
`89`	`91`	`}`
`90`	`92`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,9 @@ TEST_CASE("JsonDocument::set()") {`
`37`	`37`	`doc.set("example");`
`38`	`38`
`39`	`39`	`REQUIRE(doc.as<const char*>() == "example"_s);`
`40`		`- REQUIRE(spy.log() == AllocatorLog{});`
	`40`	`+ REQUIRE(spy.log() == AllocatorLog{`
	`41`	`+ Allocate(sizeofStaticStringPool()),`
	`42`	`+ });`
`41`	`43`	`}`
`42`	`44`
`43`	`45`	`SECTION("const char*") {`