<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[165797] trunk/Source/JavaScriptCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/165797">165797</a></dd>
<dt>Author</dt> <dd>msaboff@apple.com</dd>
<dt>Date</dt> <dd>2014-03-17 20:46:10 -0700 (Mon, 17 Mar 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>V8 regexp spends most of its time in operationGetById
https://bugs.webkit.org/show_bug.cgi?id=130380

Reviewed by Filip Pizlo.

Added String.length case to tryCacheGetByID that will only help the BaseLine JIT.
When V8 regexp is run from the command line, this nets a 2% performance improvement.
When the test is run for a longer amount of time, there is much less benefit as the
DFG will emit the appropriate code for String.length.  This does remove
operationGetById as the hottest function whne run from the command line.

* jit/Repatch.cpp:
(JSC::tryCacheGetByID):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (165796 => 165797)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2014-03-18 03:28:19 UTC (rev 165796)
+++ trunk/Source/JavaScriptCore/ChangeLog        2014-03-18 03:46:10 UTC (rev 165797)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2014-03-17  Michael Saboff  &lt;msaboff@apple.com&gt;
+
+        V8 regexp spends most of its time in operationGetById
+        https://bugs.webkit.org/show_bug.cgi?id=130380
+
+        Reviewed by Filip Pizlo.
+
+        Added String.length case to tryCacheGetByID that will only help the BaseLine JIT.
+        When V8 regexp is run from the command line, this nets a 2% performance improvement.
+        When the test is run for a longer amount of time, there is much less benefit as the
+        DFG will emit the appropriate code for String.length.  This does remove
+        operationGetById as the hottest function whne run from the command line.
+
+        * jit/Repatch.cpp:
+        (JSC::tryCacheGetByID):
+
</ins><span class="cx"> 2014-03-17  Andreas Kling  &lt;akling@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Add one-deep cache to opaque roots hashset.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (165796 => 165797)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp        2014-03-18 03:28:19 UTC (rev 165796)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp        2014-03-18 03:46:10 UTC (rev 165797)
</span><span class="lines">@@ -374,66 +374,97 @@
</span><span class="cx">     CodeBlock* codeBlock = exec-&gt;codeBlock();
</span><span class="cx">     VM* vm = &amp;exec-&gt;vm();
</span><span class="cx">     
</span><del>-    if (isJSArray(baseValue) &amp;&amp; propertyName == exec-&gt;propertyNames().length) {
</del><ins>+    if ((isJSArray(baseValue) || isJSString(baseValue)) &amp;&amp; propertyName == exec-&gt;propertyNames().length) {
</ins><span class="cx">         GPRReg baseGPR = static_cast&lt;GPRReg&gt;(stubInfo.patch.baseGPR);
</span><span class="cx"> #if USE(JSVALUE32_64)
</span><span class="cx">         GPRReg resultTagGPR = static_cast&lt;GPRReg&gt;(stubInfo.patch.valueTagGPR);
</span><span class="cx"> #endif
</span><span class="cx">         GPRReg resultGPR = static_cast&lt;GPRReg&gt;(stubInfo.patch.valueGPR);
</span><del>-        GPRReg scratchGPR = TempRegisterSet(stubInfo.patch.usedRegisters).getFreeGPR();
-        bool needToRestoreScratch = false;
-        
</del><ins>+
</ins><span class="cx">         MacroAssembler stubJit;
</span><del>-        
-        if (scratchGPR == InvalidGPRReg) {
</del><ins>+
+        if (isJSArray(baseValue)) {
+            GPRReg scratchGPR = TempRegisterSet(stubInfo.patch.usedRegisters).getFreeGPR();
+            bool needToRestoreScratch = false;
+
+            if (scratchGPR == InvalidGPRReg) {
</ins><span class="cx"> #if USE(JSVALUE64)
</span><del>-            scratchGPR = AssemblyHelpers::selectScratchGPR(baseGPR, resultGPR);
</del><ins>+                scratchGPR = AssemblyHelpers::selectScratchGPR(baseGPR, resultGPR);
</ins><span class="cx"> #else
</span><del>-            scratchGPR = AssemblyHelpers::selectScratchGPR(baseGPR, resultGPR, resultTagGPR);
</del><ins>+                scratchGPR = AssemblyHelpers::selectScratchGPR(baseGPR, resultGPR, resultTagGPR);
</ins><span class="cx"> #endif
</span><del>-            stubJit.pushToSave(scratchGPR);
-            needToRestoreScratch = true;
</del><ins>+                stubJit.pushToSave(scratchGPR);
+                needToRestoreScratch = true;
+            }
+
+            MacroAssembler::JumpList failureCases;
+
+            stubJit.load8(MacroAssembler::Address(baseGPR, JSCell::indexingTypeOffset()), scratchGPR);
+            failureCases.append(stubJit.branchTest32(MacroAssembler::Zero, scratchGPR, MacroAssembler::TrustedImm32(IsArray)));
+            failureCases.append(stubJit.branchTest32(MacroAssembler::Zero, scratchGPR, MacroAssembler::TrustedImm32(IndexingShapeMask)));
+
+            stubJit.loadPtr(MacroAssembler::Address(baseGPR, JSObject::butterflyOffset()), scratchGPR);
+            stubJit.load32(MacroAssembler::Address(scratchGPR, ArrayStorage::lengthOffset()), scratchGPR);
+            failureCases.append(stubJit.branch32(MacroAssembler::LessThan, scratchGPR, MacroAssembler::TrustedImm32(0)));
+
+            stubJit.move(scratchGPR, resultGPR);
+#if USE(JSVALUE64)
+            stubJit.or64(AssemblyHelpers::TrustedImm64(TagTypeNumber), resultGPR);
+#elif USE(JSVALUE32_64)
+            stubJit.move(AssemblyHelpers::TrustedImm32(0xffffffff), resultTagGPR); // JSValue::Int32Tag
+#endif
+
+            MacroAssembler::Jump success, fail;
+
+            emitRestoreScratch(stubJit, needToRestoreScratch, scratchGPR, success, fail, failureCases);
+            
+            LinkBuffer patchBuffer(*vm, &amp;stubJit, codeBlock);
+
+            linkRestoreScratch(patchBuffer, needToRestoreScratch, stubInfo, success, fail, failureCases);
+
+            stubInfo.stubRoutine = FINALIZE_CODE_FOR_STUB(
+                exec-&gt;codeBlock(), patchBuffer,
+                (&quot;GetById array length stub for %s, return point %p&quot;,
+                    toCString(*exec-&gt;codeBlock()).data(), stubInfo.callReturnLocation.labelAtOffset(
+                        stubInfo.patch.deltaCallToDone).executableAddress()));
+
+            RepatchBuffer repatchBuffer(codeBlock);
+            replaceWithJump(repatchBuffer, stubInfo, stubInfo.stubRoutine-&gt;code().code());
+            repatchCall(repatchBuffer, stubInfo.callReturnLocation, operationGetById);
+
+            return true;
</ins><span class="cx">         }
</span><del>-        
-        MacroAssembler::JumpList failureCases;
-       
-        stubJit.load8(MacroAssembler::Address(baseGPR, JSCell::indexingTypeOffset()), scratchGPR);
-        failureCases.append(stubJit.branchTest32(MacroAssembler::Zero, scratchGPR, MacroAssembler::TrustedImm32(IsArray)));
-        failureCases.append(stubJit.branchTest32(MacroAssembler::Zero, scratchGPR, MacroAssembler::TrustedImm32(IndexingShapeMask)));
-        
-        stubJit.loadPtr(MacroAssembler::Address(baseGPR, JSObject::butterflyOffset()), scratchGPR);
-        stubJit.load32(MacroAssembler::Address(scratchGPR, ArrayStorage::lengthOffset()), scratchGPR);
-        failureCases.append(stubJit.branch32(MacroAssembler::LessThan, scratchGPR, MacroAssembler::TrustedImm32(0)));
</del><span class="cx"> 
</span><del>-        stubJit.move(scratchGPR, resultGPR);
</del><ins>+        // String.length case
+        MacroAssembler::Jump failure = stubJit.branch8(MacroAssembler::NotEqual, MacroAssembler::Address(baseGPR, JSCell::typeInfoTypeOffset()), MacroAssembler::TrustedImm32(StringType));
+
+        stubJit.load32(MacroAssembler::Address(baseGPR, JSString::offsetOfLength()), resultGPR);
+
</ins><span class="cx"> #if USE(JSVALUE64)
</span><span class="cx">         stubJit.or64(AssemblyHelpers::TrustedImm64(TagTypeNumber), resultGPR);
</span><span class="cx"> #elif USE(JSVALUE32_64)
</span><span class="cx">         stubJit.move(AssemblyHelpers::TrustedImm32(0xffffffff), resultTagGPR); // JSValue::Int32Tag
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-        MacroAssembler::Jump success, fail;
-        
-        emitRestoreScratch(stubJit, needToRestoreScratch, scratchGPR, success, fail, failureCases);
-        
</del><ins>+        MacroAssembler::Jump success = stubJit.jump();
+
</ins><span class="cx">         LinkBuffer patchBuffer(*vm, &amp;stubJit, codeBlock);
</span><del>-        
-        linkRestoreScratch(patchBuffer, needToRestoreScratch, stubInfo, success, fail, failureCases);
-        
</del><ins>+
+        patchBuffer.link(success, stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToDone));
+        patchBuffer.link(failure, stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
+
</ins><span class="cx">         stubInfo.stubRoutine = FINALIZE_CODE_FOR_STUB(
</span><span class="cx">             exec-&gt;codeBlock(), patchBuffer,
</span><del>-            (&quot;GetById array length stub for %s, return point %p&quot;,
</del><ins>+            (&quot;GetById string length stub for %s, return point %p&quot;,
</ins><span class="cx">                 toCString(*exec-&gt;codeBlock()).data(), stubInfo.callReturnLocation.labelAtOffset(
</span><span class="cx">                     stubInfo.patch.deltaCallToDone).executableAddress()));
</span><del>-        
</del><ins>+
</ins><span class="cx">         RepatchBuffer repatchBuffer(codeBlock);
</span><span class="cx">         replaceWithJump(repatchBuffer, stubInfo, stubInfo.stubRoutine-&gt;code().code());
</span><span class="cx">         repatchCall(repatchBuffer, stubInfo.callReturnLocation, operationGetById);
</span><del>-        
</del><ins>+
</ins><span class="cx">         return true;
</span><span class="cx">     }
</span><del>-    
-    // FIXME: should support length access for String.
</del><span class="cx"> 
</span><span class="cx">     // FIXME: Cache property access for immediates.
</span><span class="cx">     if (!baseValue.isCell())
</span></span></pre>
</div>
</div>

</body>
</html>