[MERGE #3465 @obastemur] JSONStack: Improve performance

Merge pull request #3465 from obastemur:imp_jsonstack 1% Acme Air LTO gain. Majority of the time, there is at max 1 item on the stacks and SList too expensive to maintain 1item in/out during the whole JSON.stringify
chakra-core · Aug 2, 2017 · 9432774 · 9432774
2 parents ff22ab8 + 2e19ab7
commit 9432774
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 6 deletions.
diff --git a/lib/Common/DataStructures/SList.h b/lib/Common/DataStructures/SList.h
@@ -17,6 +17,8 @@ class FakeCount
     void DecrementCount() {}
     void SetCount(uint count) {}
     void AddCount(FakeCount& c) {}
+public:
+    uint Count() const { return 0; }
 };
 
 class RealCount

diff --git a/lib/Runtime/Library/JSONStack.cpp b/lib/Runtime/Library/JSONStack.cpp
@@ -6,13 +6,26 @@
 #include "JSONStack.h"
 namespace JSON
 {
+#if defined(_JS_VALUE) || (_DOM_VALUE)
+#error "Something went wrong!"
+#endif
+
+#define _JS_VALUE tempValues[0]
+#define _DOM_VALUE   tempValues[1]
+
     bool StrictEqualsObjectComparer::Equals(Js::Var x, Js::Var y)
     {
         return JSONStack::Equals(x,y);
     }
 
-    JSONStack::JSONStack(ArenaAllocator *allocator, Js::ScriptContext *context) : jsObjectStack(allocator), domObjectStack(nullptr), alloc(allocator), scriptContext(context)
+    JSONStack::JSONStack(ArenaAllocator *allocator, Js::ScriptContext *context)
+      : jsObjectStack(allocator), domObjectStack(nullptr),
+        alloc(allocator), scriptContext(context)
     {
+        // most of the time, the size of the stack is 1 object.
+        // use direct member instead of expensive Push / Pop / Has
+        _JS_VALUE = nullptr;
+        _DOM_VALUE = nullptr;
     }
 
     bool JSONStack::Equals(Js::Var x, Js::Var y)
@@ -24,10 +37,18 @@ namespace JSON
     {
         if (bJsObject)
         {
+            if (_JS_VALUE)
+            {
+                return (data == _JS_VALUE);
+            }
             return jsObjectStack.Has(data);
         }
         else if (domObjectStack)
         {
+            if (_DOM_VALUE)
+            {
+                return (data == _DOM_VALUE);
+            }
             return domObjectStack->Contains(data);
         }
         return false;
@@ -37,22 +58,68 @@ namespace JSON
     {
         if (bJsObject)
         {
-            return jsObjectStack.Push(data);
+            bool result = true;
+            if (_JS_VALUE)
+            {
+                jsObjectStack.Push(_JS_VALUE);
+                _JS_VALUE = nullptr;
+            }
+
+            if (jsObjectStack.Count())
+            {
+                result = jsObjectStack.Push(data);
+            }
+            else
+            {
+                _JS_VALUE = data;
+            }
+
+            return result;
         }
+
         EnsuresDomObjectStack();
-        domObjectStack->Add(data);
+
+        if (_DOM_VALUE)
+        {
+            domObjectStack->Add(_DOM_VALUE);
+            _DOM_VALUE = nullptr;
+        }
+
+        if (domObjectStack->Count())
+        {
+            domObjectStack->Add(data);
+        }
+        else
+        {
+            _DOM_VALUE = data;
+        }
         return true;
     }
 
     void JSONStack::Pop(bool bJsObject)
     {
         if (bJsObject)
         {
-            jsObjectStack.Pop();
+            if (_JS_VALUE)
+            {
+                _JS_VALUE = nullptr;
+            }
+            else
+            {
+                jsObjectStack.Pop();
+            }
             return;
         }
         AssertMsg(domObjectStack != NULL, "Misaligned pop");
-        domObjectStack->RemoveAtEnd();
+
+        if (_DOM_VALUE)
+        {
+            _DOM_VALUE = nullptr;
+        }
+        else
+        {
+            domObjectStack->RemoveAtEnd();
+        }
     }
 
     void JSONStack::EnsuresDomObjectStack(void)
@@ -62,4 +129,7 @@ namespace JSON
             domObjectStack = DOMObjectStack::New(alloc);
         }
     }
-}// namespace JSON
+} // namespace JSON
+
+#undef _JS_VALUE
+#undef _DOM_VALUE
diff --git a/lib/Runtime/Library/JSONStack.h b/lib/Runtime/Library/JSONStack.h
@@ -15,6 +15,7 @@ namespace JSON
     class JSONStack
     {
     private:
+        Js::Var        tempValues[2];
         SList<Js::Var> jsObjectStack; // Consider: key-only dictionary here
         typedef JsUtil::List<Js::Var, ArenaAllocator, false, Js::CopyRemovePolicy,
             SpecializedComparer<Js::Var, JSON::StrictEqualsObjectComparer>::TComparerType> DOMObjectStack;
@@ -31,6 +32,12 @@ namespace JSON
         bool Push(Js::Var data, bool bJsObject = true);
         void Pop(bool bJsObject = true);
 
+        ~JSONStack()
+        {
+            tempValues[0] = nullptr; // _JS_VALUE
+            tempValues[1] = nullptr; // _DOM_VALUE
+        }
+
     private:
         void EnsuresDomObjectStack(void);
     };