Changeset 249650 in webkit

Timestamp:

Sep 9, 2019 10:52:07 AM (5 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Promise resolve/reject functions should be created more efficiently
​https://bugs.webkit.org/show_bug.cgi?id=201488

Reviewed by Mark Lam.

JSTests:

• microbenchmarks/promise-creation-many.js: Added.

(executor):

Source/JavaScriptCore:

While r246553 fixed an important issue, it makes anonymous-builtin-function creation costly since it enforces FunctionRareData allocations.
Unfortunately, anonymous-builtin-function function can be created frequently since this type of function is used
for resolve and reject arguments of Promise's executor (e.g. new Promise((resolve, reject) => ...)'s resolve and reject).
Since we are now always creating FunctionRareData for these functions, this additional allocation makes promise creation slower.

In this patch, we use isAnonymousBuiltinFunction information for hasReifiedName correctly. And we propagate isAnonymousBuiltinFunction information
to FunctionRareData to initialize m_hasReifiedName correctly. Then we can avoid unnecessary FunctionRareData allocation, which makes
anonymous-builtin-function creation faster.

We can ensure that this patch does not revert r246553's fix by running JSTests/stress/builtin-private-function-name.js test.
The simple microbenchmark shows 1.7x improvement.

ToT Patched

promise-creation-many 45.6701+-0.1488 ^{26.8663+-1.8336} definitely 1.6999x faster

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileNewFunctionCommon):

• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNewFunction):

• runtime/FunctionRareData.cpp:

(JSC::FunctionRareData::create):
(JSC::FunctionRareData::FunctionRareData):

• runtime/FunctionRareData.h:
• runtime/JSFunction.cpp:

(JSC::JSFunction::finishCreation):
(JSC::JSFunction::allocateRareData):
(JSC::JSFunction::allocateAndInitializeRareData):

• runtime/JSFunctionInlines.h:

(JSC::JSFunction::hasReifiedName const):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/microbenchmarks/promise-creation-many.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (modified) (1 diff)
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (modified) (2 diffs)
• Source/JavaScriptCore/runtime/FunctionRareData.cpp (modified) (3 diffs)
• Source/JavaScriptCore/runtime/FunctionRareData.h (modified) (2 diffs)
• Source/JavaScriptCore/runtime/JSFunction.cpp (modified) (3 diffs)
• Source/JavaScriptCore/runtime/JSFunctionInlines.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-09-09  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Promise resolve/reject functions should be created more efficiently
+        https://bugs.webkit.org/show_bug.cgi?id=201488
+
+        Reviewed by Mark Lam.
+
+        * microbenchmarks/promise-creation-many.js: Added.
+        (executor):
+
 2019-09-09  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-09-09  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Promise resolve/reject functions should be created more efficiently
+        https://bugs.webkit.org/show_bug.cgi?id=201488
+
+        Reviewed by Mark Lam.
+
+        While r246553 fixed an important issue, it makes anonymous-builtin-function creation costly since it enforces FunctionRareData allocations.
+        Unfortunately, anonymous-builtin-function function can be created frequently since this type of function is used
+        for `resolve` and `reject` arguments of Promise's executor (e.g. `new Promise((resolve, reject) => ...)`'s resolve and reject).
+        Since we are now always creating FunctionRareData for these functions, this additional allocation makes promise creation slower.
+
+        In this patch, we use `isAnonymousBuiltinFunction` information for `hasReifiedName` correctly. And we propagate `isAnonymousBuiltinFunction` information
+        to FunctionRareData to initialize `m_hasReifiedName` correctly. Then we can avoid unnecessary FunctionRareData allocation, which makes
+        anonymous-builtin-function creation faster.
+
+        We can ensure that this patch does not revert r246553's fix by running JSTests/stress/builtin-private-function-name.js test.
+        The simple microbenchmark shows 1.7x improvement.
+
+                                              ToT                     Patched
+
+            promise-creation-many       45.6701+-0.1488     ^     26.8663+-1.8336        ^ definitely 1.6999x faster
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileNewFunctionCommon):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNewFunction):
+        * runtime/FunctionRareData.cpp:
+        (JSC::FunctionRareData::create):
+        (JSC::FunctionRareData::FunctionRareData):
+        * runtime/FunctionRareData.h:
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::finishCreation):
+        (JSC::JSFunction::allocateRareData):
+        (JSC::JSFunction::allocateAndInitializeRareData):
+        * runtime/JSFunctionInlines.h:
+        (JSC::JSFunction::hasReifiedName const):
+
 2019-09-07  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -7213 +7213 @@
     m_jit.storePtr(TrustedImmPtr::weakPointer(m_jit.graph(), executable), JITCompiler::Address(resultGPR, JSFunction::offsetOfExecutable()));
     m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(resultGPR, JSFunction::offsetOfRareData()));
-    
-    if (executable->isAnonymousBuiltinFunction()) {
-        VM& vm = this->vm();
-        m_jit.mutatorFence(vm);
-        GPRTemporary allocator(this);
-        Allocator allocatorValue = allocatorForNonVirtualConcurrently<FunctionRareData>(vm, sizeof(FunctionRareData), AllocatorForMode::AllocatorIfExists);
-        emitAllocateJSCell(scratch1GPR, JITAllocator::constant(allocatorValue), allocator.gpr(), TrustedImmPtr(m_jit.graph().registerStructure(vm.functionRareDataStructure.get())), scratch2GPR, slowPath);
-
-        ptrdiff_t objectAllocationProfileOffset = FunctionRareData::offsetOfObjectAllocationProfile();
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratch1GPR, objectAllocationProfileOffset + ObjectAllocationProfileWithPrototype::offsetOfAllocator()));
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratch1GPR, objectAllocationProfileOffset + ObjectAllocationProfileWithPrototype::offsetOfStructure()));
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratch1GPR, objectAllocationProfileOffset + ObjectAllocationProfileWithPrototype::offsetOfPrototype()));
-        m_jit.storePtr(TrustedImmPtr(0x1), JITCompiler::Address(scratch1GPR, FunctionRareData::offsetOfAllocationProfileWatchpointSet()));
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratch1GPR, FunctionRareData::offsetOfInternalFunctionAllocationProfile() + InternalFunctionAllocationProfile::offsetOfStructure()));
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratch1GPR, FunctionRareData::offsetOfBoundFunctionStructure()));
-        m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(scratch1GPR, FunctionRareData::offsetOfAllocationProfileClearingWatchpoint()));
-        m_jit.store8(TrustedImm32(0), JITCompiler::Address(scratch1GPR, FunctionRareData::offsetOfHasReifiedLength()));
-        m_jit.store8(TrustedImm32(1), JITCompiler::Address(scratch1GPR, FunctionRareData::offsetOfHasReifiedName()));
-        m_jit.mutatorFence(vm);
-        m_jit.storePtr(scratch1GPR, JITCompiler::Address(resultGPR, JSFunction::offsetOfRareData()));
-    } else
-        m_jit.mutatorFence(vm());
-
+    m_jit.mutatorFence(vm());
 }
 

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -5576 +5576 @@
         m_out.storePtr(weakPointer(executable), fastObject, m_heaps.JSFunction_executable);
         m_out.storePtr(m_out.intPtrZero, fastObject, m_heaps.JSFunction_rareData);
-        
-        VM& vm = this->vm();
-        if (executable->isAnonymousBuiltinFunction()) {
-            mutatorFence();
-            Allocator allocator = allocatorForNonVirtualConcurrently<FunctionRareData>(vm, sizeof(FunctionRareData), AllocatorForMode::AllocatorIfExists);
-            LValue rareData = allocateCell(m_out.constIntPtr(allocator.localAllocator()), vm.functionRareDataStructure.get(), slowPath);
-            m_out.storePtr(m_out.intPtrZero, rareData, m_heaps.FunctionRareData_allocator);
-            m_out.storePtr(m_out.intPtrZero, rareData, m_heaps.FunctionRareData_structure);
-            m_out.storePtr(m_out.intPtrZero, rareData, m_heaps.FunctionRareData_prototype);
-            m_out.storePtr(m_out.intPtrOne, rareData, m_heaps.FunctionRareData_allocationProfileWatchpointSet);
-            m_out.storePtr(m_out.intPtrZero, rareData, m_heaps.FunctionRareData_internalFunctionAllocationProfile_structure);
-            m_out.storePtr(m_out.intPtrZero, rareData, m_heaps.FunctionRareData_boundFunctionStructure);
-            m_out.storePtr(m_out.intPtrZero, rareData, m_heaps.FunctionRareData_allocationProfileClearingWatchpoint);
-            m_out.store32As8(m_out.int32One, rareData, m_heaps.FunctionRareData_hasReifiedName);
-            m_out.store32As8(m_out.int32Zero, rareData, m_heaps.FunctionRareData_hasReifiedLength);
-            mutatorFence();
-            m_out.storePtr(rareData, fastObject, m_heaps.JSFunction_rareData);
-        } else
-            mutatorFence();
+        mutatorFence();
 
         ValueFromBlock fastResult = m_out.anchor(fastObject);
@@ -5603 +5585 @@
         Vector<LValue> slowPathArguments;
         slowPathArguments.append(scope);
+        VM& vm = this->vm();
         LValue callResult = lazySlowPath(
             [=, &vm] (const Vector<Location>& locations) -> RefPtr<LazySlowPath::Generator> {

trunk/Source/JavaScriptCore/runtime/FunctionRareData.cpp

@@ -34 +34 @@
 const ClassInfo FunctionRareData::s_info = { "FunctionRareData", nullptr, nullptr, nullptr, CREATE_METHOD_TABLE(FunctionRareData) };
 
-FunctionRareData* FunctionRareData::create(VM& vm)
+FunctionRareData* FunctionRareData::create(VM& vm, JSFunction* function)
 {
-    FunctionRareData* rareData = new (NotNull, allocateCell<FunctionRareData>(vm.heap)) FunctionRareData(vm);
+    FunctionRareData* rareData = new (NotNull, allocateCell<FunctionRareData>(vm.heap)) FunctionRareData(vm, function);
     rareData->finishCreation(vm);
     return rareData;
@@ -63 +63 @@
 }
 
-FunctionRareData::FunctionRareData(VM& vm)
+FunctionRareData::FunctionRareData(VM& vm, JSFunction* function)
     : Base(vm, vm.functionRareDataStructure.get())
     , m_objectAllocationProfile()
@@ -70 +70 @@
     // function is unlikely to allocate a rare data until the first allocation anyway.
     , m_allocationProfileWatchpointSet(ClearWatchpoint)
+    , m_hasReifiedName(function->isAnonymousBuiltinFunction())
 {
 }

trunk/Source/JavaScriptCore/runtime/FunctionRareData.h

@@ -51 +51 @@
     static const unsigned StructureFlags = Base::StructureFlags | StructureIsImmortal;
 
-    static FunctionRareData* create(VM&);
+    static FunctionRareData* create(VM&, JSFunction*);
 
     static const bool needsDestruction = true;
@@ -120 +120 @@
 
 protected:
-    FunctionRareData(VM&);
+    FunctionRareData(VM&, JSFunction*);
     ~FunctionRareData();
 

trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

@@ -126 +126 @@
     ASSERT(jsDynamicCast<JSFunction*>(vm, this));
     ASSERT(type() == JSFunctionType);
-    if (isAnonymousBuiltinFunction()) {
-        // This is anonymous builtin function.
-        rareData(vm)->setHasReifiedName();
-    }
 }
 
@@ -147 +143 @@
 {
     ASSERT(!m_rareData);
-    FunctionRareData* rareData = FunctionRareData::create(vm);
+    FunctionRareData* rareData = FunctionRareData::create(vm, this);
 
     // A DFG compilation thread may be trying to read the rare data
@@ -187 +183 @@
     VM& vm = exec->vm();
     JSObject* prototype = prototypeForConstruction(vm, exec);
-    FunctionRareData* rareData = FunctionRareData::create(vm);
+    FunctionRareData* rareData = FunctionRareData::create(vm, this);
     rareData->initializeObjectAllocationProfile(vm, globalObject(vm), prototype, inlineCapacity, this);
 

trunk/Source/JavaScriptCore/runtime/JSFunctionInlines.h

@@ -111 +111 @@
 inline bool JSFunction::hasReifiedName() const
 {
-    return m_rareData ? m_rareData->hasReifiedName() : false;
+    if (m_rareData)
+        return m_rareData->hasReifiedName();
+    if (isAnonymousBuiltinFunction())
+        return true;
+    return false;
 }