Skip to content

Commit b7c8e0d

Browse files
bblanchonbblanchon
committed
Decouple VariantData from MemoryPool 
1 parent 30c111f commit b7c8e0d

File tree

17 files changed

+395
-266
lines changed

17 files changed

+395
-266
lines changed

‎extras/tests/JsonDocument/MemberProxy.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
#include <catch.hpp>
1212

13+
#include "Allocators.hpp"
14+
1315
using ArduinoJson::detail::sizeofArray;
1416
using ArduinoJson::detail::sizeofObject;
1517
using ArduinoJson::detail::sizeofString;
@@ -388,3 +390,19 @@ TEST_CASE("Deduplicate keys") {
388390
CHECK(key1 == key2);
389391
}
390392
}
393+
394+
TEST_CASE("MemberProxy under memory constraints") {
395+
ControllableAllocator allocator;
396+
JsonDocument doc(4096, &allocator);
397+
398+
SECTION("key allocation fails") {
399+
allocator.disable();
400+
401+
doc[std::string("hello")] = "world";
402+
403+
REQUIRE(doc.is<JsonObject>());
404+
REQUIRE(doc.size() == 0);
405+
REQUIRE(doc.memoryUsage() == sizeofObject(1));
406+
REQUIRE(doc.overflowed() == true);
407+
}
408+
}

‎extras/tests/JsonVariant/copy.cpp

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44

55
#include <ArduinoJson.h>
66
#include <catch.hpp>
7+
#include "Allocators.hpp"
78

89
using ArduinoJson::detail::sizeofString;
910

1011
TEST_CASE("JsonVariant::set(JsonVariant)") {
11-
JsonDocument doc1(4096);
12-
JsonDocument doc2(4096);
12+
ControllableAllocator allocator;
13+
SpyingAllocator spyingAllocator(&allocator);
14+
JsonDocument doc1(4096, &spyingAllocator);
15+
JsonDocument doc2(4096, &spyingAllocator);
1316
JsonVariant var1 = doc1.to<JsonVariant>();
1417
JsonVariant var2 = doc2.to<JsonVariant>();
1518

@@ -37,53 +40,103 @@ TEST_CASE("JsonVariant::set(JsonVariant)") {
3740

3841
SECTION("stores const char* by reference") {
3942
var1.set("hello!!");
43+
spyingAllocator.clearLog();
44+
4045
var2.set(var1);
4146

4247
REQUIRE(doc1.memoryUsage() == 0);
4348
REQUIRE(doc2.memoryUsage() == 0);
49+
REQUIRE(spyingAllocator.log() == AllocatorLog());
4450
}
4551

4652
SECTION("stores char* by copy") {
4753
char str[] = "hello!!";
48-
4954
var1.set(str);
55+
spyingAllocator.clearLog();
56+
5057
var2.set(var1);
5158

5259
REQUIRE(doc1.memoryUsage() == sizeofString(7));
5360
REQUIRE(doc2.memoryUsage() == sizeofString(7));
61+
REQUIRE(spyingAllocator.log() ==
62+
AllocatorLog() << AllocatorLog::Allocate(sizeofString((7))));
63+
}
64+
65+
SECTION("fails gracefully if string allocation fails") {
66+
char str[] = "hello!!";
67+
var1.set(str);
68+
allocator.disable();
69+
spyingAllocator.clearLog();
70+
71+
var2.set(var1);
72+
73+
REQUIRE(doc1.memoryUsage() == sizeofString(7));
74+
REQUIRE(doc2.memoryUsage() == 0);
75+
REQUIRE(doc2.overflowed() == true);
76+
REQUIRE(spyingAllocator.log() ==
77+
AllocatorLog() << AllocatorLog::AllocateFail(sizeofString((7))));
5478
}
5579

5680
SECTION("stores std::string by copy") {
5781
var1.set(std::string("hello!!"));
82+
spyingAllocator.clearLog();
83+
5884
var2.set(var1);
5985

6086
REQUIRE(doc1.memoryUsage() == sizeofString(7));
6187
REQUIRE(doc2.memoryUsage() == sizeofString(7));
88+
REQUIRE(spyingAllocator.log() ==
89+
AllocatorLog() << AllocatorLog::Allocate(sizeofString((7))));
6290
}
6391

6492
SECTION("stores Serialized<const char*> by reference") {
6593
var1.set(serialized("hello!!", 8));
94+
spyingAllocator.clearLog();
95+
6696
var2.set(var1);
6797

6898
REQUIRE(doc1.memoryUsage() == 0);
6999
REQUIRE(doc2.memoryUsage() == 0);
100+
REQUIRE(spyingAllocator.log() == AllocatorLog());
70101
}
71102

72103
SECTION("stores Serialized<char*> by copy") {
73104
char str[] = "hello!!";
74105
var1.set(serialized(str, 7));
106+
spyingAllocator.clearLog();
107+
75108
var2.set(var1);
76109

77110
REQUIRE(doc1.memoryUsage() == sizeofString(7));
78111
REQUIRE(doc2.memoryUsage() == sizeofString(7));
112+
REQUIRE(spyingAllocator.log() ==
113+
AllocatorLog() << AllocatorLog::Allocate(sizeofString((7))));
79114
}
80115

81116
SECTION("stores Serialized<std::string> by copy") {
82117
var1.set(serialized(std::string("hello!!")));
118+
spyingAllocator.clearLog();
119+
83120
var2.set(var1);
84121

85122
REQUIRE(doc1.memoryUsage() == sizeofString(7));
86123
REQUIRE(doc2.memoryUsage() == sizeofString(7));
124+
REQUIRE(spyingAllocator.log() ==
125+
AllocatorLog() << AllocatorLog::Allocate(sizeofString((7))));
126+
}
127+
128+
SECTION("fails gracefully if raw string allocation fails") {
129+
var1.set(serialized(std::string("hello!!")));
130+
allocator.disable();
131+
spyingAllocator.clearLog();
132+
133+
var2.set(var1);
134+
135+
REQUIRE(doc1.memoryUsage() == sizeofString(7));
136+
REQUIRE(doc2.memoryUsage() == 0);
137+
REQUIRE(doc2.overflowed() == true);
138+
REQUIRE(spyingAllocator.log() ==
139+
AllocatorLog() << AllocatorLog::AllocateFail(sizeofString((7))));
87140
}
88141

89142
SECTION("destination is unbound") {

‎src/ArduinoJson/Array/JsonArray.hpp

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <ArduinoJson/Array/ElementProxy.hpp>
88
#include <ArduinoJson/Array/JsonArrayConst.hpp>
9+
#include <ArduinoJson/Collection/CollectionFunctions.hpp>
910

1011
ARDUINOJSON_BEGIN_PUBLIC_NAMESPACE
1112

@@ -43,9 +44,7 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
4344
// Returns a reference to the new element.
4445
// https://arduinojson.org/v6/api/jsonarray/add/
4546
JsonVariant add() const {
46-
if (!_data)
47-
return JsonVariant();
48-
return JsonVariant(_pool, _data->addElement(_pool));
47+
return JsonVariant(_pool, collectionAddElement(_data, _pool));
4948
}
5049

5150
// Appends a value to the array.
@@ -79,9 +78,7 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
7978
// Copies an array.
8079
// https://arduinojson.org/v6/api/jsonarray/set/
8180
FORCE_INLINE bool set(JsonArrayConst src) const {
82-
if (!_data || !src._data)
83-
return false;
84-
return _data->copyFrom(*src._data, _pool);
81+
return collectionCopy(_data, src._data, _pool);
8582
}
8683

8784
// Compares the content of two arrays.
@@ -93,27 +90,21 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
9390
// ⚠️ Doesn't release the memory associated with the removed element.
9491
// https://arduinojson.org/v6/api/jsonarray/remove/
9592
FORCE_INLINE void remove(iterator it) const {
96-
if (!_data)
97-
return;
98-
_data->removeSlot(it._slot);
93+
collectionRemove(_data, it._slot, _pool);
9994
}
10095

10196
// Removes the element at the specified index.
10297
// ⚠️ Doesn't release the memory associated with the removed element.
10398
// https://arduinojson.org/v6/api/jsonarray/remove/
10499
FORCE_INLINE void remove(size_t index) const {
105-
if (!_data)
106-
return;
107-
_data->removeElement(index);
100+
collectionRemoveElement(_data, index, _pool);
108101
}
109102

110103
// Removes all the elements of the array.
111104
// ⚠️ Doesn't release the memory associated with the removed elements.
112105
// https://arduinojson.org/v6/api/jsonarray/clear/
113106
void clear() const {
114-
if (!_data)
115-
return;
116-
_data->clear();
107+
collectionClear(_data, _pool);
117108
}
118109

119110
// Gets or sets the element at the specified index.

‎src/ArduinoJson/Collection/CollectionData.hpp

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
ARDUINOJSON_BEGIN_PRIVATE_NAMESPACE
1313

14-
class MemoryPool;
1514
class VariantData;
1615
class VariantSlot;
1716

@@ -28,30 +27,13 @@ class CollectionData {
2827

2928
// Array only
3029

31-
VariantData* addElement(MemoryPool* pool);
32-
3330
VariantData* getElement(size_t index) const;
3431

35-
VariantData* getOrAddElement(size_t index, MemoryPool* pool);
36-
37-
void removeElement(size_t index);
38-
3932
// Object only
4033

41-
template <typename TAdaptedString>
42-
VariantData* addMember(TAdaptedString key, MemoryPool* pool);
43-
4434
template <typename TAdaptedString>
4535
VariantData* getMember(TAdaptedString key) const;
4636

47-
template <typename TAdaptedString>
48-
VariantData* getOrAddMember(TAdaptedString key, MemoryPool* pool);
49-
50-
template <typename TAdaptedString>
51-
void removeMember(TAdaptedString key) {
52-
removeSlot(getSlot(key));
53-
}
54-
5537
template <typename TAdaptedString>
5638
bool containsKey(const TAdaptedString& key) const;
5739

@@ -61,23 +43,21 @@ class CollectionData {
6143
size_t memoryUsage() const;
6244
size_t size() const;
6345

64-
VariantSlot* addSlot(MemoryPool*);
46+
void addSlot(VariantSlot*);
6547
void removeSlot(VariantSlot* slot);
6648

67-
bool copyFrom(const CollectionData& src, MemoryPool* pool);
68-
6949
VariantSlot* head() const {
7050
return _head;
7151
}
7252

7353
void movePointers(ptrdiff_t variantDistance);
7454

75-
private:
7655
VariantSlot* getSlot(size_t index) const;
7756

7857
template <typename TAdaptedString>
7958
VariantSlot* getSlot(TAdaptedString key) const;
8059

60+
private:
8161
VariantSlot* getPreviousSlot(VariantSlot*) const;
8262
};
8363

‎src/ArduinoJson/Collection/CollectionFunctions.hpp

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// ArduinoJson - https://arduinojson.org
2+
// Copyright © 2014-2023, Benoit BLANCHON
3+
// MIT License
4+
5+
#pragma once
6+
7+
#include <ArduinoJson/Collection/CollectionData.hpp>
8+
9+
ARDUINOJSON_BEGIN_PRIVATE_NAMESPACE
10+
11+
inline VariantData* collectionAddElement(CollectionData* array,
12+
MemoryPool* pool) {
13+
if (!array)
14+
return nullptr;
15+
auto slot = pool->allocVariant();
16+
if (!slot)
17+
return nullptr;
18+
array->addSlot(slot);
19+
return slot->data();
20+
}
21+
22+
template <typename TAdaptedString>
23+
inline VariantData* collectionAddMember(CollectionData* obj, TAdaptedString key,
24+
MemoryPool* pool) {
25+
ARDUINOJSON_ASSERT(!key.isNull());
26+
ARDUINOJSON_ASSERT(obj != nullptr);
27+
if (!obj)
28+
return nullptr;
29+
auto slot = pool->allocVariant();
30+
if (!slot)
31+
return nullptr;
32+
auto storedKey = storeString(pool, key);
33+
if (!storedKey)
34+
return nullptr;
35+
slot->setKey(storedKey);
36+
obj->addSlot(slot);
37+
return slot->data();
38+
}
39+
40+
inline void collectionClear(CollectionData* c, MemoryPool* pool) {
41+
if (!c)
42+
return;
43+
for (auto slot = c->head(); slot; slot = slot->next())
44+
slotRelease(slot, pool);
45+
c->clear();
46+
}
47+
48+
inline bool collectionCopy(CollectionData* dst, const CollectionData* src,
49+
MemoryPool* pool) {
50+
if (!dst || !src)
51+
return false;
52+
53+
collectionClear(dst, pool);
54+
55+
for (VariantSlot* s = src->head(); s; s = s->next()) {
56+
VariantData* var;
57+
if (s->key() != 0) {
58+
JsonString key(s->key(),
59+
s->ownsKey() ? JsonString::Copied : JsonString::Linked);
60+
var = collectionAddMember(dst, adaptString(key), pool);
61+
} else {
62+
var = collectionAddElement(dst, pool);
63+
}
64+
if (!variantCopyFrom(var, s->data(), pool))
65+
return false;
66+
}
67+
return true;
68+
}
69+
70+
inline void collectionRemove(CollectionData* data, VariantSlot* slot,
71+
MemoryPool* pool) {
72+
if (!data || !slot)
73+
return;
74+
data->removeSlot(slot);
75+
slotRelease(slot, pool);
76+
}
77+
78+
inline void collectionRemoveElement(CollectionData* array, size_t index,
79+
MemoryPool* pool) {
80+
if (!array)
81+
return;
82+
collectionRemove(array, array->getSlot(index), pool);
83+
}
84+
85+
template <typename TAdaptedString>
86+
inline void collectionRemoveMember(CollectionData* obj, TAdaptedString key,
87+
MemoryPool* pool) {
88+
if (!obj)
89+
return;
90+
collectionRemove(obj, obj->getSlot(key), pool);
91+
}
92+
93+
ARDUINOJSON_END_PRIVATE_NAMESPACE

0 commit comments

Comments
 (0)