Title: [197729] trunk/Source/_javascript_Core
Revision
197729
Author
fpi...@apple.com
Date
2016-03-07 22:53:32 -0800 (Mon, 07 Mar 2016)

Log Message

createRegExpMatchesArray should allocate substrings more quickly
https://bugs.webkit.org/show_bug.cgi?id=155160

Reviewed by Sam Weinig.

This was calling a version of jsSubstring() that isn't inlineable because it was doing a lot
of checks in finishCreation(). In particular, it was checking that the base string is not
itself a substring and that it's been resolved. We don't need those checks here, since the
string must have been resolved prior to regexp processing.

This patch is also smart about whether to do checks for the empty and full substrings. In
the matches array loop, these checks are super unlikely to be profitable, so we just
unconditionally allocate the substring.

This removes those checks and makes the allocation inlineable. It looks like a 1% speed-up
on Octane/regexp.

* runtime/JSString.h:
(JSC::jsSubstring):
(JSC::jsSubstringOfResolved):
* runtime/RegExpMatchesArray.cpp:
(JSC::createRegExpMatchesArray):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (197728 => 197729)


--- trunk/Source/_javascript_Core/ChangeLog	2016-03-08 06:44:59 UTC (rev 197728)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-03-08 06:53:32 UTC (rev 197729)
@@ -1,3 +1,28 @@
+2016-03-07  Filip Pizlo  <fpi...@apple.com>
+
+        createRegExpMatchesArray should allocate substrings more quickly
+        https://bugs.webkit.org/show_bug.cgi?id=155160
+
+        Reviewed by Sam Weinig.
+
+        This was calling a version of jsSubstring() that isn't inlineable because it was doing a lot
+        of checks in finishCreation(). In particular, it was checking that the base string is not
+        itself a substring and that it's been resolved. We don't need those checks here, since the
+        string must have been resolved prior to regexp processing.
+
+        This patch is also smart about whether to do checks for the empty and full substrings. In
+        the matches array loop, these checks are super unlikely to be profitable, so we just
+        unconditionally allocate the substring.
+
+        This removes those checks and makes the allocation inlineable. It looks like a 1% speed-up
+        on Octane/regexp.
+
+        * runtime/JSString.h:
+        (JSC::jsSubstring):
+        (JSC::jsSubstringOfResolved):
+        * runtime/RegExpMatchesArray.cpp:
+        (JSC::createRegExpMatchesArray):
+
 2016-03-07  Benjamin Poulain  <bpoul...@apple.com>
 
         [JSC] Small clean up of how we use SSA's valuesAtHead

Modified: trunk/Source/_javascript_Core/runtime/JSString.h (197728 => 197729)


--- trunk/Source/_javascript_Core/runtime/JSString.h	2016-03-08 06:44:59 UTC (rev 197728)
+++ trunk/Source/_javascript_Core/runtime/JSString.h	2016-03-08 06:53:32 UTC (rev 197729)
@@ -266,7 +266,7 @@
     };
 
 private:
-    JSRopeString(VM& vm)
+    ALWAYS_INLINE JSRopeString(VM& vm)
         : JSString(vm)
     {
     }
@@ -317,6 +317,18 @@
         }
     }
 
+    ALWAYS_INLINE void finishCreationSubstringOfResolved(VM& vm, JSString* base, unsigned offset, unsigned length)
+    {
+        Base::finishCreation(vm);
+        ASSERT(!sumOverflows<int32_t>(offset, length));
+        ASSERT(offset + length <= base->length());
+        m_length = length;
+        setIs8Bit(base->is8Bit());
+        setIsSubstring(true);
+        substringBase().set(vm, this, base);
+        substringOffset() = offset;
+    }
+
     void finishCreation(VM& vm)
     {
         JSString::finishCreation(vm);
@@ -362,6 +374,13 @@
         return newString;
     }
 
+    ALWAYS_INLINE static JSString* createSubstringOfResolved(VM& vm, JSString* base, unsigned offset, unsigned length)
+    {
+        JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
+        newString->finishCreationSubstringOfResolved(vm, base, offset, length);
+        return newString;
+    }
+
     void visitFibers(SlotVisitor&);
 
     static ptrdiff_t offsetOfFibers() { return OBJECT_OFFSETOF(JSRopeString, u); }
@@ -553,6 +572,18 @@
     return JSRopeString::create(vm, exec, s, offset, length);
 }
 
+inline JSString* jsSubstringOfResolved(VM& vm, JSString* s, unsigned offset, unsigned length)
+{
+    ASSERT(offset <= static_cast<unsigned>(s->length()));
+    ASSERT(length <= static_cast<unsigned>(s->length()));
+    ASSERT(offset + length <= static_cast<unsigned>(s->length()));
+    if (!length)
+        return vm.smallStrings.emptyString();
+    if (!offset && length == s->length())
+        return s;
+    return JSRopeString::createSubstringOfResolved(vm, s, offset, length);
+}
+
 inline JSString* jsSubstring(ExecState* exec, JSString* s, unsigned offset, unsigned length)
 {
     return jsSubstring(exec->vm(), exec, s, offset, length);

Modified: trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.cpp (197728 => 197729)


--- trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.cpp	2016-03-08 06:44:59 UTC (rev 197728)
+++ trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.cpp	2016-03-08 06:53:32 UTC (rev 197729)
@@ -76,13 +76,13 @@
     if (UNLIKELY(globalObject->isHavingABadTime())) {
         array = JSArray::tryCreateUninitialized(vm, globalObject->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1);
         
-        array->initializeIndex(vm, 0, jsSubstring(vm, exec, input, result.start, result.end - result.start));
+        array->initializeIndex(vm, 0, jsSubstringOfResolved(vm, input, result.start, result.end - result.start));
         
         if (unsigned numSubpatterns = regExp->numSubpatterns()) {
             for (unsigned i = 1; i <= numSubpatterns; ++i) {
                 int start = subpatternResults[2 * i];
                 if (start >= 0)
-                    array->initializeIndex(vm, i, jsSubstring(vm, exec, input, start, subpatternResults[2 * i + 1] - start));
+                    array->initializeIndex(vm, i, JSRopeString::createSubstringOfResolved(vm, input, start, subpatternResults[2 * i + 1] - start));
                 else
                     array->initializeIndex(vm, i, jsUndefined());
             }
@@ -91,13 +91,13 @@
         array = tryCreateUninitializedRegExpMatchesArray(vm, globalObject->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1);
         RELEASE_ASSERT(array);
         
-        array->initializeIndex(vm, 0, jsSubstring(vm, exec, input, result.start, result.end - result.start), ArrayWithContiguous);
+        array->initializeIndex(vm, 0, jsSubstringOfResolved(vm, input, result.start, result.end - result.start), ArrayWithContiguous);
         
         if (unsigned numSubpatterns = regExp->numSubpatterns()) {
             for (unsigned i = 1; i <= numSubpatterns; ++i) {
                 int start = subpatternResults[2 * i];
                 if (start >= 0)
-                    array->initializeIndex(vm, i, jsSubstring(vm, exec, input, start, subpatternResults[2 * i + 1] - start), ArrayWithContiguous);
+                    array->initializeIndex(vm, i, JSRopeString::createSubstringOfResolved(vm, input, start, subpatternResults[2 * i + 1] - start), ArrayWithContiguous);
                 else
                     array->initializeIndex(vm, i, jsUndefined(), ArrayWithContiguous);
             }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to