Title: [231741] trunk/Source/_javascript_Core
Revision
231741
Author
fpi...@apple.com
Date
2018-05-13 09:54:55 -0700 (Sun, 13 May 2018)

Log Message

CachedCall::call() should be faster
https://bugs.webkit.org/show_bug.cgi?id=185583

Reviewed by Yusuke Suzuki.
        
CachedCall is an optimization for String.prototype.replace(r, f) where f is a function.
Unfortunately, because of a combination of abstraction and assertions, this code path had a
lot of overhead. This patch reduces this overhead by:
        
- Turning off some assertions. These assertions don't look to have security value; they're
  mostly for sanity. I turned off stack alignment checks and VM state checks having to do
  with whether the JSLock is held. The JSLock checks are not relevant when doing a cached
  call, considering that the caller would have already been strongly assuming that the JSLock
  is held.
        
- Making more things inlineable.
        
This looks like a small (4% ish) speed-up on SunSpider/string-unpack-code.

* _javascript_Core.xcodeproj/project.pbxproj:
* interpreter/CachedCall.h:
(JSC::CachedCall::call):
* interpreter/Interpreter.cpp:
(JSC::checkedReturn): Deleted.
* interpreter/Interpreter.h:
(JSC::Interpreter::checkedReturn):
* interpreter/InterpreterInlines.h:
(JSC::Interpreter::execute):
* jit/JITCode.cpp:
(JSC::JITCode::execute): Deleted.
* jit/JITCodeInlines.h: Added.
(JSC::JITCode::execute):
* llint/LowLevelInterpreter.asm:
* runtime/StringPrototype.cpp:

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (231740 => 231741)


--- trunk/Source/_javascript_Core/ChangeLog	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-05-13 16:54:55 UTC (rev 231741)
@@ -1,3 +1,40 @@
+2018-05-12  Filip Pizlo  <fpi...@apple.com>
+
+        CachedCall::call() should be faster
+        https://bugs.webkit.org/show_bug.cgi?id=185583
+
+        Reviewed by Yusuke Suzuki.
+        
+        CachedCall is an optimization for String.prototype.replace(r, f) where f is a function.
+        Unfortunately, because of a combination of abstraction and assertions, this code path had a
+        lot of overhead. This patch reduces this overhead by:
+        
+        - Turning off some assertions. These assertions don't look to have security value; they're
+          mostly for sanity. I turned off stack alignment checks and VM state checks having to do
+          with whether the JSLock is held. The JSLock checks are not relevant when doing a cached
+          call, considering that the caller would have already been strongly assuming that the JSLock
+          is held.
+        
+        - Making more things inlineable.
+        
+        This looks like a small (4% ish) speed-up on SunSpider/string-unpack-code.
+
+        * _javascript_Core.xcodeproj/project.pbxproj:
+        * interpreter/CachedCall.h:
+        (JSC::CachedCall::call):
+        * interpreter/Interpreter.cpp:
+        (JSC::checkedReturn): Deleted.
+        * interpreter/Interpreter.h:
+        (JSC::Interpreter::checkedReturn):
+        * interpreter/InterpreterInlines.h:
+        (JSC::Interpreter::execute):
+        * jit/JITCode.cpp:
+        (JSC::JITCode::execute): Deleted.
+        * jit/JITCodeInlines.h: Added.
+        (JSC::JITCode::execute):
+        * llint/LowLevelInterpreter.asm:
+        * runtime/StringPrototype.cpp:
+
 2018-05-13  Andy VanWagoner  <andy@vanwagoner.family>
 
         [INTL] Improve spec & test262 compliance for Intl APIs

Modified: trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj (231740 => 231741)


--- trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj	2018-05-13 16:54:55 UTC (rev 231741)
@@ -712,6 +712,7 @@
 		0FF9CE741B9CD6D0004EDCA6 /* PolymorphicAccess.h in Headers */ = {isa = PBXBuildFile; fileRef = 0FF9CE721B9CD6D0004EDCA6 /* PolymorphicAccess.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		0FFA549816B8835300B3A982 /* A64DOpcode.h in Headers */ = {isa = PBXBuildFile; fileRef = 652A3A231651C69700A80AFE /* A64DOpcode.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		0FFB6C391AF48DDC00DB1BF7 /* TypeofType.h in Headers */ = {isa = PBXBuildFile; fileRef = 0FFB6C371AF48DDC00DB1BF7 /* TypeofType.h */; settings = {ATTRIBUTES = (Private, ); }; };
+		0FFB80BC20A794730006AAF6 /* JITCodeInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 0FFB80BB20A794700006AAF6 /* JITCodeInlines.h */; };
 		0FFB921A16D02EC50055A5DB /* DFGBasicBlockInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 0FD5652216AB780A00197653 /* DFGBasicBlockInlines.h */; };
 		0FFB921C16D02F110055A5DB /* DFGOSRExitCompilationInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 65987F2C167FE84B003C2F8D /* DFGOSRExitCompilationInfo.h */; };
 		0FFB921D16D02F300055A5DB /* DFGSlowPathGenerator.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F1E3A501537C2CB000F9456 /* DFGSlowPathGenerator.h */; };
@@ -2991,6 +2992,7 @@
 		0FF9CE721B9CD6D0004EDCA6 /* PolymorphicAccess.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PolymorphicAccess.h; sourceTree = "<group>"; };
 		0FFB6C361AF48DDC00DB1BF7 /* TypeofType.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TypeofType.cpp; sourceTree = "<group>"; };
 		0FFB6C371AF48DDC00DB1BF7 /* TypeofType.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TypeofType.h; sourceTree = "<group>"; };
+		0FFB80BB20A794700006AAF6 /* JITCodeInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JITCodeInlines.h; sourceTree = "<group>"; };
 		0FFC920F1B94D4DF0071DD66 /* InferredTypeTable.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = InferredTypeTable.cpp; sourceTree = "<group>"; };
 		0FFC92101B94D4DF0071DD66 /* InferredTypeTable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = InferredTypeTable.h; sourceTree = "<group>"; };
 		0FFC92131B94E83E0071DD66 /* DFGDesiredInferredType.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGDesiredInferredType.h; path = dfg/DFGDesiredInferredType.h; sourceTree = "<group>"; };
@@ -5517,6 +5519,7 @@
 				146FE51111A710430087AE66 /* JITCall32_64.cpp */,
 				0F8F94431667635200D61971 /* JITCode.cpp */,
 				86CCEFDD0F413F8900FD7F9E /* JITCode.h */,
+				0FFB80BB20A794700006AAF6 /* JITCodeInlines.h */,
 				FE476FF3207E85D40093CA2D /* JITCodeMap.h */,
 				0F0776BD14FF002800102332 /* JITCompilationEffort.h */,
 				0FAF7EFA165BA919000C8455 /* JITDisassembler.cpp */,
@@ -9249,6 +9252,7 @@
 				E34E657520668EAA00FB81AC /* ParseHash.h in Headers */,
 				37C738D21EDB56E4003F2B0B /* ParseInt.h in Headers */,
 				BC18C44B0E16F5CD00B34460 /* Parser.h in Headers */,
+				0FFB80BC20A794730006AAF6 /* JITCodeInlines.h in Headers */,
 				93052C350FB792190048FDC3 /* ParserArena.h in Headers */,
 				0FCCAE4516D0CF7400D0C65B /* ParserError.h in Headers */,
 				A77F1825164192C700640A47 /* ParserModes.h in Headers */,

Modified: trunk/Source/_javascript_Core/interpreter/CachedCall.h (231740 => 231741)


--- trunk/Source/_javascript_Core/interpreter/CachedCall.h	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/interpreter/CachedCall.h	2018-05-13 16:54:55 UTC (rev 231741)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -60,7 +60,7 @@
             m_valid = !scope.exception();
         }
         
-        JSValue call()
+        ALWAYS_INLINE JSValue call()
         { 
             ASSERT(m_valid);
             ASSERT(m_arguments.size() == static_cast<size_t>(m_protoCallFrame.argumentCount()));

Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (231740 => 231741)


--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp	2018-05-13 16:54:55 UTC (rev 231741)
@@ -47,6 +47,7 @@
 #include "FrameTracers.h"
 #include "FunctionCodeBlock.h"
 #include "InterpreterInlines.h"
+#include "JITCodeInlines.h"
 #include "JSArrayInlines.h"
 #include "JSBoundFunction.h"
 #include "JSCInlines.h"
@@ -769,18 +770,6 @@
     exception->setDidNotifyInspectorOfThrow();
 }
 
-static inline JSValue checkedReturn(JSValue returnValue)
-{
-    ASSERT(returnValue);
-    return returnValue;
-}
-
-static inline JSObject* checkedReturn(JSObject* returnValue)
-{
-    ASSERT(returnValue);
-    return returnValue;
-}
-
 JSValue Interpreter::executeProgram(const SourceCode& source, CallFrame* callFrame, JSObject* thisObj)
 {
     JSScope* scope = thisObj->globalObject()->globalScope();
@@ -1128,31 +1117,6 @@
     return result;
 }
 
-JSValue Interpreter::execute(CallFrameClosure& closure) 
-{
-    VM& vm = *closure.vm;
-    auto throwScope = DECLARE_THROW_SCOPE(vm);
-
-    ASSERT(!vm.isCollectorBusyOnCurrentThread());
-    RELEASE_ASSERT(vm.currentThreadIsHoldingAPILock());
-    if (vm.isCollectorBusyOnCurrentThread())
-        return jsNull();
-
-    StackStats::CheckPoint stackCheckPoint;
-
-    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
-    if (UNLIKELY(vm.needTrapHandling(mask))) {
-        vm.handleTraps(closure.oldCallFrame, mask);
-        RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
-    }
-
-    // Execute the code:
-    throwScope.release();
-    JSValue result = closure.functionExecutable->generatedJITCodeForCall()->execute(&vm, closure.protoCallFrame);
-
-    return checkedReturn(result);
-}
-
 JSValue Interpreter::execute(EvalExecutable* eval, CallFrame* callFrame, JSValue thisValue, JSScope* scope)
 {
     VM& vm = *scope->vm();

Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.h (231740 => 231741)


--- trunk/Source/_javascript_Core/interpreter/Interpreter.h	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.h	2018-05-13 16:54:55 UTC (rev 231741)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2018 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Research In Motion Limited. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -133,6 +133,18 @@
 
     private:
         enum ExecutionFlag { Normal, InitializeAndReturn };
+        
+        static JSValue checkedReturn(JSValue returnValue)
+        {
+            ASSERT(returnValue);
+            return returnValue;
+        }
+        
+        static JSObject* checkedReturn(JSObject* returnValue)
+        {
+            ASSERT(returnValue);
+            return returnValue;
+        }
 
         CallFrameClosure prepareForRepeatCall(FunctionExecutable*, CallFrame*, ProtoCallFrame*, JSFunction*, int argumentCountIncludingThis, JSScope*, const ArgList&);
 

Modified: trunk/Source/_javascript_Core/interpreter/InterpreterInlines.h (231740 => 231741)


--- trunk/Source/_javascript_Core/interpreter/InterpreterInlines.h	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/interpreter/InterpreterInlines.h	2018-05-13 16:54:55 UTC (rev 231741)
@@ -26,6 +26,8 @@
 
 #pragma once
 
+#include "CallFrameClosure.h"
+#include "Exception.h"
 #include "Instruction.h"
 #include "Interpreter.h"
 #include "JSCPtrTag.h"
@@ -71,4 +73,27 @@
     return instruction.u.opcode;
 }
 
+ALWAYS_INLINE JSValue Interpreter::execute(CallFrameClosure& closure)
+{
+    VM& vm = *closure.vm;
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+
+    ASSERT(!vm.isCollectorBusyOnCurrentThread());
+    ASSERT(vm.currentThreadIsHoldingAPILock());
+
+    StackStats::CheckPoint stackCheckPoint;
+
+    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
+    if (UNLIKELY(vm.needTrapHandling(mask))) {
+        vm.handleTraps(closure.oldCallFrame, mask);
+        RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
+    }
+
+    // Execute the code:
+    throwScope.release();
+    JSValue result = closure.functionExecutable->generatedJITCodeForCall()->execute(&vm, closure.protoCallFrame);
+
+    return checkedReturn(result);
+}
+
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/jit/JITCode.cpp (231740 => 231741)


--- trunk/Source/_javascript_Core/jit/JITCode.cpp	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/jit/JITCode.cpp	2018-05-13 16:54:55 UTC (rev 231741)
@@ -26,7 +26,6 @@
 #include "config.h"
 #include "JITCode.h"
 
-#include "LLIntThunks.h"
 #include "JSCInlines.h"
 #include "ProtoCallFrame.h"
 #include <wtf/PrintStream.h>
@@ -67,15 +66,6 @@
 {
 }
 
-JSValue JITCode::execute(VM* vm, ProtoCallFrame* protoCallFrame)
-{
-    auto scope = DECLARE_THROW_SCOPE(*vm);
-    void* entryAddress;
-    entryAddress = addressForCall(MustCheckArity).executableAddress();
-    JSValue result = JSValue::decode(vmEntryToJavaScript(entryAddress, vm, protoCallFrame));
-    return scope.exception() ? jsNull() : result;
-}
-
 DFG::CommonData* JITCode::dfgCommon()
 {
     RELEASE_ASSERT_NOT_REACHED();

Added: trunk/Source/_javascript_Core/jit/JITCodeInlines.h (0 => 231741)


--- trunk/Source/_javascript_Core/jit/JITCodeInlines.h	                        (rev 0)
+++ trunk/Source/_javascript_Core/jit/JITCodeInlines.h	2018-05-13 16:54:55 UTC (rev 231741)
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ */
+
+#pragma once
+
+#include "JITCode.h"
+#include "LLIntThunks.h"
+
+namespace JSC {
+
+ALWAYS_INLINE JSValue JITCode::execute(VM* vm, ProtoCallFrame* protoCallFrame)
+{
+    auto scope = DECLARE_THROW_SCOPE(*vm);
+    void* entryAddress;
+    entryAddress = addressForCall(MustCheckArity).executableAddress();
+    JSValue result = JSValue::decode(vmEntryToJavaScript(entryAddress, vm, protoCallFrame));
+    return scope.exception() ? jsNull() : result;
+}
+
+} // namespave JSC
+

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm (231740 => 231741)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm	2018-05-13 16:54:55 UTC (rev 231741)
@@ -542,22 +542,24 @@
 end
 
 macro checkStackPointerAlignment(tempReg, location)
-    if ARM64 or ARM64E or C_LOOP
-        # ARM64 and ARM64E will check for us!
-        # C_LOOP does not need the alignment, and can use a little perf
-        # improvement from avoiding useless work.
-    else
-        if ARM or ARMv7 or ARMv7_TRADITIONAL
-            # ARM can't do logical ops with the sp as a source
-            move sp, tempReg
-            andp StackAlignmentMask, tempReg
+    if ASSERT_ENABLED
+        if ARM64 or ARM64E or C_LOOP
+            # ARM64 and ARM64E will check for us!
+            # C_LOOP does not need the alignment, and can use a little perf
+            # improvement from avoiding useless work.
         else
-            andp sp, StackAlignmentMask, tempReg
+            if ARM or ARMv7 or ARMv7_TRADITIONAL
+                # ARM can't do logical ops with the sp as a source
+                move sp, tempReg
+                andp StackAlignmentMask, tempReg
+            else
+                andp sp, StackAlignmentMask, tempReg
+            end
+            btpz tempReg, .stackPointerOkay
+            move location, tempReg
+            break
+        .stackPointerOkay:
         end
-        btpz tempReg, .stackPointerOkay
-        move location, tempReg
-        break
-    .stackPointerOkay:
     end
 end
 

Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (231740 => 231741)


--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2018-05-13 14:28:39 UTC (rev 231740)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2018-05-13 16:54:55 UTC (rev 231741)
@@ -28,7 +28,9 @@
 #include "CachedCall.h"
 #include "Error.h"
 #include "FrameTracers.h"
+#include "InterpreterInlines.h"
 #include "IntlObject.h"
+#include "JITCodeInlines.h"
 #include "JSArray.h"
 #include "JSCBuiltins.h"
 #include "JSCInlines.h"
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to