Modified: trunk/Source/_javascript_Core/ChangeLog (233916 => 233917)
--- trunk/Source/_javascript_Core/ChangeLog 2018-07-18 18:10:08 UTC (rev 233916)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-07-18 18:27:08 UTC (rev 233917)
@@ -1,3 +1,27 @@
+2018-07-18 Yusuke Suzuki <utatane....@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 <cgar...@igalia.com>
[GLIB] Add jsc_context_check_syntax() to GLib API
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (233916 => 233917)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-07-18 18:10:08 UTC (rev 233916)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-07-18 18:27:08 UTC (rev 233917)
@@ -120,12 +120,13 @@
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 @@
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,12 +263,18 @@
{
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);
auto stringifyResult = appendStringifiedValue(result, value, root, emptyPropertyName);
@@ -325,11 +330,12 @@
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);
}
