[scheme] Allow to create a new null value which shares a pool with the current value without directly attaching it as child

3 files changed, 99 insertions, 1 deletions

diff --git a/library/cpp/scheme/scheme.cpp b/library/cpp/scheme/scheme.cpp
index e6b59e8346f..5ef2e80c8cb 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 b4cb98c9677..78336d16c8a 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 1a5d07c31bc..cfe39cc5390 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());
+    }
 };