diff options
author | sobols <sobols@yandex-team.com> | 2022-10-12 19:31:35 +0300 |
---|---|---|
committer | sobols <sobols@yandex-team.com> | 2022-10-12 19:31:35 +0300 |
commit | 918fcce7e3028401867b3276871686c4add87705 (patch) | |
tree | eac49fbdbaebb83d97e0beb84ddc098396cd6a69 | |
parent | ee2715d277e3d307e86d4987a14abfb9986fab55 (diff) | |
download | ydb-918fcce7e3028401867b3276871686c4add87705.tar.gz |
[scheme] Allow to create a new null value which shares a pool with the current value without directly attaching it as child
-rw-r--r-- | library/cpp/scheme/scheme.cpp | 6 | ||||
-rw-r--r-- | library/cpp/scheme/scheme.h | 7 | ||||
-rw-r--r-- | library/cpp/scheme/tests/ut/scheme_ut.cpp | 87 |
3 files changed, 99 insertions, 1 deletions
diff --git a/library/cpp/scheme/scheme.cpp b/library/cpp/scheme/scheme.cpp index e6b59e8346..5ef2e80c8c 100644 --- a/library/cpp/scheme/scheme.cpp +++ b/library/cpp/scheme/scheme.cpp @@ -68,6 +68,10 @@ namespace NSc { return TValue().CopyFrom(*this); } + TValue TValue::CreateNew() const { + return Y_LIKELY(TheCore) ? TValue(TheCore->Pool) : TValue(); + } + double TValue::ForceNumber(double deflt) const { const TScCore& core = Core(); if (core.IsNumber()) { @@ -231,7 +235,7 @@ namespace NSc { } bool TValue::SamePool(const TValue& a, const TValue& b) { - return Same(a, b) || a.TheCore->Pool.Get() == b.TheCore->Pool.Get(); + return Same(a, b) || (a.TheCore && b.TheCore && a.TheCore->Pool.Get() == b.TheCore->Pool.Get()); } bool TValue::Equal(const TValue& a, const TValue& b) { diff --git a/library/cpp/scheme/scheme.h b/library/cpp/scheme/scheme.h index b4cb98c967..78336d16c8 100644 --- a/library/cpp/scheme/scheme.h +++ b/library/cpp/scheme/scheme.h @@ -50,6 +50,11 @@ namespace NSc { // A COW copy will see changes in its parent, but no change in the COW copy will propagate to its parent. public: + // XXX: A newly constructed standalone TValue instance (even null!) consumes ~4 KB of memory because it allocates a memory pool inside. + // Consider building a tree of TValues in top-down order (from root to leaves) to share a single pool + // instead of creating child TValues first and appending them to a parent TValue. + // All somehow cached values should be constant, otherwise the shared pool can grow infinitely. + inline TValue(); inline TValue(TValue& v); inline TValue(const TValue& v); @@ -386,6 +391,8 @@ namespace NSc { TValue Clone() const; // returns deep copy of self (on the separate pool) TValue& CopyFrom(const TValue& other); // deep copy other value into self, returns self + TValue CreateNew() const; // returns a new null value which shares a memory pool with self + TValue& Swap(TValue& v); static bool Same(const TValue&, const TValue&); // point to the same core diff --git a/library/cpp/scheme/tests/ut/scheme_ut.cpp b/library/cpp/scheme/tests/ut/scheme_ut.cpp index 1a5d07c31b..cfe39cc539 100644 --- a/library/cpp/scheme/tests/ut/scheme_ut.cpp +++ b/library/cpp/scheme/tests/ut/scheme_ut.cpp @@ -876,4 +876,91 @@ Y_UNIT_TEST_SUITE(TSchemeTest) { const NSc::TValue expectedResult = NSc::NUt::AssertFromJson("{a:[null,-1,2,3.4],b:3,c:{d:5,e:{f:42}}}"); UNIT_ASSERT_VALUES_EQUAL(v, expectedResult); } + + Y_UNIT_TEST(TestNewNodeOnCurrentPool) { + NSc::TValue parent; + parent["foo"] = "bar"; + + // standalone + NSc::TValue firstChild(10); + UNIT_ASSERT(!NSc::TValue::SamePool(parent, firstChild)); + + // shares a memory pool + NSc::TValue secondChild = parent.CreateNew(); + UNIT_ASSERT(secondChild.IsNull()); + UNIT_ASSERT(NSc::TValue::SamePool(parent, secondChild)); + secondChild = 20; + UNIT_ASSERT(secondChild.IsIntNumber()); + UNIT_ASSERT(NSc::TValue::SamePool(parent, secondChild)); + + // attach children to parent + parent["first"] = std::move(firstChild); + parent["second"] = std::move(secondChild); + UNIT_ASSERT_VALUES_EQUAL(parent["first"].GetIntNumber(), 10); + UNIT_ASSERT_VALUES_EQUAL(parent["second"].GetIntNumber(), 20); + UNIT_ASSERT(!NSc::TValue::SamePool(parent, parent["first"])); + UNIT_ASSERT(NSc::TValue::SamePool(parent, parent["second"])); + } + + Y_UNIT_TEST(TestNewNodeAfterMove) { + NSc::TValue a; + NSc::TValue b = std::move(a); // after this, `a` has no memory pool + NSc::TValue a2 = a.CreateNew(); // no crash here + NSc::TValue b2 = b.CreateNew(); + UNIT_ASSERT(!NSc::TValue::SamePool(a, b)); + UNIT_ASSERT(NSc::TValue::SamePool(b, b2)); + UNIT_ASSERT(!NSc::TValue::SamePool(a2, b2)); + } + + namespace { + bool FillChild(int x, NSc::TValue& result) { + if (x % 2 == 0) { + result["value"] = x; + return true; + } + return false; + } + } + + Y_UNIT_TEST(TestHierarchyCreationPatterns) { + const int n = 1000; + + // Good: the code looks so clear and intuitive. + // Bad: it creates one thousand of independent memory pools, half of them remain alive at the end. + NSc::TValue list1; + for (int i = 0; i < n; ++i) { + NSc::TValue child; + if (FillChild(i, child)) { + list1.Push(std::move(child)); + } + } + + // Good: a single memory pool is reused. + // Bad: we have to add and remove a child manually. + // Bad: some memory on pool remains unused (gaps are left after removing nodes). + NSc::TValue list2; + for (int i = 0; i < n; ++i) { + auto& child = list2.Push(); + if (!FillChild(i, child)) { + list2.Pop(); + } + } + + // Good: a single memory pool is reused. + // Good: the code looks quite intuitive. + // Good: there could be multiple parents on the same pool. + // Bad: some memory on pool remains unused. + // Bad: you have to know about the special method `CreateNew()`. + NSc::TValue list3; + for (int i = 0; i < n; ++i) { + auto child = list3.CreateNew(); + if (FillChild(i, child)) { + list3.Push(std::move(child)); + } + } + + // Results are the same + UNIT_ASSERT_VALUES_EQUAL(list1.ToJson(), list2.ToJson()); + UNIT_ASSERT_VALUES_EQUAL(list2.ToJson(), list3.ToJson()); + } }; |