[JSC] Root wrapper object in JSON.stringify is not necessary if replacer is not callable
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2018 18:27:08 +0000 (18:27 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2018 18:27:08 +0000 (18:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187752

Reviewed by Mark Lam.

JSON.stringify has an implicit root wrapper object since we would like to call replacer
with a wrapper object and a property name. While we always create this wrapper object,
it is unnecessary if the given replacer is not callable.

This patch removes wrapper object creation when a replacer is not callable to avoid unnecessary
allocations. This change slightly improves the performance of Kraken/json-stringify-tinderbox.

                                   baseline                  patched

json-stringify-tinderbox        39.730+-0.590      ^      38.853+-0.266         ^ definitely 1.0226x faster

* runtime/JSONObject.cpp:
(JSC::Stringifier::isCallableReplacer const):
(JSC::Stringifier::Stringifier):
(JSC::Stringifier::stringify):
(JSC::Stringifier::appendStringifiedValue):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233917 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSONObject.cpp

index f25a75c..dce8c21 100644 (file)
@@ -1,3 +1,27 @@
+2018-07-18  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Root wrapper object in JSON.stringify is not necessary if replacer is not callable
+        https://bugs.webkit.org/show_bug.cgi?id=187752
+
+        Reviewed by Mark Lam.
+
+        JSON.stringify has an implicit root wrapper object since we would like to call replacer
+        with a wrapper object and a property name. While we always create this wrapper object,
+        it is unnecessary if the given replacer is not callable.
+
+        This patch removes wrapper object creation when a replacer is not callable to avoid unnecessary
+        allocations. This change slightly improves the performance of Kraken/json-stringify-tinderbox.
+
+                                           baseline                  patched
+
+        json-stringify-tinderbox        39.730+-0.590      ^      38.853+-0.266         ^ definitely 1.0226x faster
+
+        * runtime/JSONObject.cpp:
+        (JSC::Stringifier::isCallableReplacer const):
+        (JSC::Stringifier::Stringifier):
+        (JSC::Stringifier::stringify):
+        (JSC::Stringifier::appendStringifiedValue):
+
 2018-07-18  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GLIB] Add jsc_context_check_syntax() to GLib API
index dc78588..07bcd74 100644 (file)
@@ -120,12 +120,13 @@ private:
     void indent();
     void unindent();
     void startNewLine(StringBuilder&) const;
+    bool isCallableReplacer() const { return m_replacerCallType != CallType::None; }
 
     ExecState* const m_exec;
     JSValue m_replacer;
-    bool m_usingArrayReplacer;
+    bool m_usingArrayReplacer { false };
     PropertyNameArray m_arrayReplacerPropertyNames;
-    CallType m_replacerCallType;
+    CallType m_replacerCallType { CallType::None };
     CallData m_replacerCallData;
     String m_gap;
 
@@ -220,9 +221,7 @@ JSValue PropertyNameForFunctionCall::value(ExecState* exec) const
 Stringifier::Stringifier(ExecState* exec, JSValue replacer, JSValue space)
     : m_exec(exec)
     , m_replacer(replacer)
-    , m_usingArrayReplacer(false)
     , m_arrayReplacerPropertyNames(&exec->vm(), PropertyNameMode::Strings, PrivateSymbolMode::Exclude)
-    , m_replacerCallType(CallType::None)
 {
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
@@ -264,11 +263,17 @@ JSValue Stringifier::stringify(JSValue value)
 {
     VM& vm = m_exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
-    JSObject* object = constructEmptyObject(m_exec);
-    RETURN_IF_EXCEPTION(scope, jsNull());
 
     PropertyNameForFunctionCall emptyPropertyName(vm.propertyNames->emptyIdentifier);
-    object->putDirect(vm, vm.propertyNames->emptyIdentifier, value);
+
+    // If the replacer is not callable, root object wrapper is non-user-observable.
+    // We can skip creating this wrapper object.
+    JSObject* object = nullptr;
+    if (isCallableReplacer()) {
+        object = constructEmptyObject(m_exec);
+        RETURN_IF_EXCEPTION(scope, jsNull());
+        object->putDirect(vm, vm.propertyNames->emptyIdentifier, value);
+    }
 
     StringBuilder result;
     Holder root(Holder::RootHolder, object);
@@ -325,11 +330,12 @@ Stringifier::StringifyResult Stringifier::appendStringifiedValue(StringBuilder&
     RETURN_IF_EXCEPTION(scope, StringifyFailed);
 
     // Call the replacer function.
-    if (m_replacerCallType != CallType::None) {
+    if (isCallableReplacer()) {
         MarkedArgumentBuffer args;
         args.append(propertyName.value(m_exec));
         args.append(value);
         ASSERT(!args.hasOverflowed());
+        ASSERT(holder.object());
         value = call(m_exec, m_replacer, m_replacerCallType, m_replacerCallData, holder.object(), args);
         RETURN_IF_EXCEPTION(scope, StringifyFailed);
     }