Merge r175365 - Use plain JSArray for RegExp matches instead of a laz…

…ily populated custom object.

<https://webkit.org/b/138191>

Reviewed by Geoffrey Garen.

We're already offering two RegExp matching APIs, one that collects subpattern
matches (exec), and one that simply tests for a match (test).
Given that, it was pretty overkill to lazily populate the resulting array of
matches, since the user could simply use test() if they didn't need them.

This allows the JIT to generate better code for RegExp match arrays, and also
enables some fast paths in the JSC runtime that check if an object isJSArray().

Looks like ~1.5% improvement on Octane/regexp according to run-jsc-benchmarks.

* jit/Repatch.cpp:
(JSC::tryCacheGetByID):
* runtime/JSArray.h:
(JSC::createArrayButterflyWithExactLength): Deleted.
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
* runtime/RegExpCachedResult.cpp:
(JSC::RegExpCachedResult::visitChildren):
(JSC::RegExpCachedResult::lastResult):
(JSC::RegExpCachedResult::leftContext):
(JSC::RegExpCachedResult::rightContext):
* runtime/RegExpCachedResult.h:
(JSC::RegExpCachedResult::RegExpCachedResult):
(JSC::RegExpCachedResult::record):
(JSC::RegExpCachedResult::input):
* runtime/RegExpConstructor.cpp:
(JSC::RegExpConstructor::getBackref):
(JSC::RegExpConstructor::getLastParen):
(JSC::RegExpConstructor::getLeftContext):
(JSC::RegExpConstructor::getRightContext):
* runtime/RegExpMatchesArray.cpp:
(JSC::createRegExpMatchesArray):
(JSC::RegExpMatchesArray::RegExpMatchesArray): Deleted.
(JSC::RegExpMatchesArray::create): Deleted.
(JSC::RegExpMatchesArray::finishCreation): Deleted.
(JSC::RegExpMatchesArray::visitChildren): Deleted.
(JSC::RegExpMatchesArray::reifyAllProperties): Deleted.
(JSC::RegExpMatchesArray::reifyMatchProperty): Deleted.
(JSC::RegExpMatchesArray::leftContext): Deleted.
(JSC::RegExpMatchesArray::rightContext): Deleted.
* runtime/RegExpMatchesArray.h:
(JSC::RegExpMatchesArray::createStructure): Deleted.
(JSC::RegExpMatchesArray::reifyAllPropertiesIfNecessary): Deleted.
(JSC::RegExpMatchesArray::reifyMatchPropertyIfNecessary): Deleted.
(JSC::RegExpMatchesArray::getOwnPropertySlot): Deleted.
(JSC::RegExpMatchesArray::getOwnPropertySlotByIndex): Deleted.
(JSC::RegExpMatchesArray::put): Deleted.
(JSC::RegExpMatchesArray::putByIndex): Deleted.
(JSC::RegExpMatchesArray::deleteProperty): Deleted.
(JSC::RegExpMatchesArray::deletePropertyByIndex): Deleted.
(JSC::RegExpMatchesArray::getOwnPropertyNames): Deleted.
(JSC::RegExpMatchesArray::defineOwnProperty): Deleted.
(JSC::isRegExpMatchesArray): Deleted.
* runtime/RegExpObject.cpp:
(JSC::RegExpObject::exec):
* runtime/StringPrototype.cpp:
(JSC::stringProtoFuncMatch):

Canonical link: https://commits.webkit.org/154760.174@webkitgtk/2.6
git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@175927 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,68 @@
+2014-10-29  Andreas Kling  <akling@apple.com>
+
+        Use plain JSArray for RegExp matches instead of a lazily populated custom object.
+        <https://webkit.org/b/138191>
+
+        Reviewed by Geoffrey Garen.
+
+        We're already offering two RegExp matching APIs, one that collects subpattern
+        matches (exec), and one that simply tests for a match (test).
+        Given that, it was pretty overkill to lazily populate the resulting array of
+        matches, since the user could simply use test() if they didn't need them.
+
+        This allows the JIT to generate better code for RegExp match arrays, and also
+        enables some fast paths in the JSC runtime that check if an object isJSArray().
+
+        Looks like ~1.5% improvement on Octane/regexp according to run-jsc-benchmarks.
+
+        * jit/Repatch.cpp:
+        (JSC::tryCacheGetByID):
+        * runtime/JSArray.h:
+        (JSC::createArrayButterflyWithExactLength): Deleted.
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init):
+        * runtime/RegExpCachedResult.cpp:
+        (JSC::RegExpCachedResult::visitChildren):
+        (JSC::RegExpCachedResult::lastResult):
+        (JSC::RegExpCachedResult::leftContext):
+        (JSC::RegExpCachedResult::rightContext):
+        * runtime/RegExpCachedResult.h:
+        (JSC::RegExpCachedResult::RegExpCachedResult):
+        (JSC::RegExpCachedResult::record):
+        (JSC::RegExpCachedResult::input):
+        * runtime/RegExpConstructor.cpp:
+        (JSC::RegExpConstructor::getBackref):
+        (JSC::RegExpConstructor::getLastParen):
+        (JSC::RegExpConstructor::getLeftContext):
+        (JSC::RegExpConstructor::getRightContext):
+        * runtime/RegExpMatchesArray.cpp:
+        (JSC::createRegExpMatchesArray):
+        (JSC::RegExpMatchesArray::RegExpMatchesArray): Deleted.
+        (JSC::RegExpMatchesArray::create): Deleted.
+        (JSC::RegExpMatchesArray::finishCreation): Deleted.
+        (JSC::RegExpMatchesArray::visitChildren): Deleted.
+        (JSC::RegExpMatchesArray::reifyAllProperties): Deleted.
+        (JSC::RegExpMatchesArray::reifyMatchProperty): Deleted.
+        (JSC::RegExpMatchesArray::leftContext): Deleted.
+        (JSC::RegExpMatchesArray::rightContext): Deleted.
+        * runtime/RegExpMatchesArray.h:
+        (JSC::RegExpMatchesArray::createStructure): Deleted.
+        (JSC::RegExpMatchesArray::reifyAllPropertiesIfNecessary): Deleted.
+        (JSC::RegExpMatchesArray::reifyMatchPropertyIfNecessary): Deleted.
+        (JSC::RegExpMatchesArray::getOwnPropertySlot): Deleted.
+        (JSC::RegExpMatchesArray::getOwnPropertySlotByIndex): Deleted.
+        (JSC::RegExpMatchesArray::put): Deleted.
+        (JSC::RegExpMatchesArray::putByIndex): Deleted.
+        (JSC::RegExpMatchesArray::deleteProperty): Deleted.
+        (JSC::RegExpMatchesArray::deletePropertyByIndex): Deleted.
+        (JSC::RegExpMatchesArray::getOwnPropertyNames): Deleted.
+        (JSC::RegExpMatchesArray::defineOwnProperty): Deleted.
+        (JSC::isRegExpMatchesArray): Deleted.
+        * runtime/RegExpObject.cpp:
+        (JSC::RegExpObject::exec):
+        * runtime/StringPrototype.cpp:
+        (JSC::stringProtoFuncMatch):
+
 2014-10-27  Mark Lam  <mark.lam@apple.com>
 
         Crash when attempting to perform array iteration on a non-array with numeric keys not initialized.

Source/JavaScriptCore/jit/Repatch.cpp

@@ -638,7 +638,7 @@ static InlineCacheAction tryCacheGetByID(ExecState* exec, JSValue baseValue, con
     CodeBlock* codeBlock = exec->codeBlock();
     VM* vm = &exec->vm();
 
-    if ((isJSArray(baseValue) || isRegExpMatchesArray(baseValue) || isJSString(baseValue)) && propertyName == exec->propertyNames().length) {
+    if ((isJSArray(baseValue) || isJSString(baseValue)) && propertyName == exec->propertyNames().length) {
         GPRReg baseGPR = static_cast<GPRReg>(stubInfo.patch.baseGPR);
 #if USE(JSVALUE32_64)
         GPRReg resultTagGPR = static_cast<GPRReg>(stubInfo.patch.valueTagGPR);
@@ -647,7 +647,7 @@ static InlineCacheAction tryCacheGetByID(ExecState* exec, JSValue baseValue, con
 
         MacroAssembler stubJit;
 
-        if (isJSArray(baseValue) || isRegExpMatchesArray(baseValue)) {
+        if (isJSArray(baseValue)) {
             GPRReg scratchGPR = TempRegisterSet(stubInfo.patch.usedRegisters).getFreeGPR();
             bool needToRestoreScratch = false;
 

Source/JavaScriptCore/runtime/JSArray.h

@@ -190,18 +190,6 @@ inline Butterfly* createContiguousArrayButterfly(VM& vm, JSCell* intendedOwner,
     return result;
 }
 
-inline Butterfly* createArrayButterflyWithExactLength(VM& vm, JSCell* intendedOwner, unsigned initialLength)
-{
-    Butterfly* butterfly = Butterfly::create(
-        vm, intendedOwner, 0, 0, true, indexingHeaderForArray(initialLength, initialLength),
-        ArrayStorage::sizeFor(initialLength));
-    ArrayStorage* storage = butterfly->arrayStorage();
-    storage->m_indexBias = 0;
-    storage->m_sparseMap.clear();
-    storage->m_numValuesInVector = 0;
-    return butterfly;
-}
-
 inline Butterfly* createArrayButterfly(VM& vm, JSCell* intendedOwner, unsigned initialLength)
 {
     Butterfly* butterfly = Butterfly::create(

Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -288,8 +288,8 @@ void JSGlobalObject::init(VM& vm)
     m_originalArrayStructureForIndexingShape[SlowPutArrayStorageShape >> IndexingShapeShift].set(vm, this, JSArray::createStructure(vm, this, m_arrayPrototype.get(), ArrayWithSlowPutArrayStorage));
     for (unsigned i = 0; i < NumberOfIndexingShapes; ++i)
         m_arrayStructureForIndexingShapeDuringAllocation[i] = m_originalArrayStructureForIndexingShape[i];
-    
-    m_regExpMatchesArrayStructure.set(vm, this, RegExpMatchesArray::createStructure(vm, this, m_arrayPrototype.get()));
+
+    m_regExpMatchesArrayStructure.set(vm, this, Structure::create(vm, this, m_arrayPrototype.get(), TypeInfo(ObjectType, StructureFlags), JSArray::info(), ArrayWithSlowPutArrayStorage));
 
     RegExp* emptyRegex = RegExp::create(vm, "", NoFlags);
 

Source/JavaScriptCore/runtime/RegExpCachedResult.cpp

@@ -37,18 +37,40 @@ void RegExpCachedResult::visitChildren(SlotVisitor& visitor)
     visitor.append(&m_lastRegExp);
     visitor.append(&m_reifiedInput);
     visitor.append(&m_reifiedResult);
+    visitor.append(&m_reifiedLeftContext);
+    visitor.append(&m_reifiedRightContext);
 }
 
-RegExpMatchesArray* RegExpCachedResult::lastResult(ExecState* exec, JSObject* owner)
+JSArray* RegExpCachedResult::lastResult(ExecState* exec, JSObject* owner)
 {
-    if (m_result) {
+    if (!m_reified) {
         m_reifiedInput.set(exec->vm(), owner, m_lastInput.get());
-        m_reifiedResult.set(exec->vm(), owner, RegExpMatchesArray::create(exec, m_lastInput.get(), m_lastRegExp.get(), m_result));
-        m_result = MatchResult::failed();
+        m_reifiedResult.set(exec->vm(), owner, createRegExpMatchesArray(exec, m_lastInput.get(), m_lastRegExp.get(), m_result));
+        m_reified = true;
     }
     return m_reifiedResult.get();
 }
 
+JSString* RegExpCachedResult::leftContext(ExecState* exec, JSObject* owner)
+{
+    // Make sure we're reified.
+    lastResult(exec, owner);
+    if (!m_reifiedLeftContext)
+        m_reifiedLeftContext.set(exec->vm(), owner, m_result.start ? jsSubstring(exec, m_reifiedInput.get(), 0, m_result.start) : jsEmptyString(exec));
+    return m_reifiedLeftContext.get();
+}
+
+JSString* RegExpCachedResult::rightContext(ExecState* exec, JSObject* owner)
+{
+    // Make sure we're reified.
+    lastResult(exec, owner);
+    if (!m_reifiedRightContext) {
+        unsigned length = m_reifiedInput->length();
+        m_reifiedRightContext.set(exec->vm(), owner, m_result.end != length ? jsSubstring(exec, m_reifiedInput.get(), m_result.end, length - m_result.end) : jsEmptyString(exec));
+    }
+    return m_reifiedRightContext.get();
+}
+
 void RegExpCachedResult::setInput(ExecState* exec, JSObject* owner, JSString* input)
 {
     // Make sure we're reified, otherwise m_reifiedInput will be ignored.

Source/JavaScriptCore/runtime/RegExpCachedResult.h

@@ -30,8 +30,8 @@
 
 namespace JSC {
 
+class JSArray;
 class JSString;
-class RegExpMatchesArray;
 
 // RegExpCachedResult is used to track the cached results of the last
 // match, stores on the RegExp constructor (e.g. $&, $_, $1, $2 ...).
@@ -46,6 +46,7 @@ class RegExpCachedResult {
 public:
     RegExpCachedResult(VM& vm, JSObject* owner, RegExp* emptyRegExp)
         : m_result(0, 0)
+        , m_reified(false)
     {
         m_lastInput.set(vm, owner, jsEmptyString(&vm));
         m_lastRegExp.set(vm, owner, emptyRegExp);
@@ -55,28 +56,34 @@ class RegExpCachedResult {
     {
         m_lastRegExp.set(vm, owner, regExp);
         m_lastInput.set(vm, owner, input);
+        m_reifiedLeftContext.clear();
+        m_reifiedRightContext.clear();
         m_result = result;
+        m_reified = false;
     }
 
-    RegExpMatchesArray* lastResult(ExecState*, JSObject* owner);
+    JSArray* lastResult(ExecState*, JSObject* owner);
     void setInput(ExecState*, JSObject* owner, JSString*);
 
+    JSString* leftContext(ExecState*, JSObject* owner);
+    JSString* rightContext(ExecState*, JSObject* owner);
+
     JSString* input()
     {
-        // If m_result showas a match then we're in a lazy state, so m_lastInput
-        // is the most recent value of the input property. If not then we have
-        // reified, in which case m_reifiedInput will contain the correct value.
-        return m_result ? m_lastInput.get() : m_reifiedInput.get();
+        return m_reified ? m_reifiedInput.get() : m_lastInput.get();
     }
 
     void visitChildren(SlotVisitor&);
 
 private:
     MatchResult m_result;
+    bool m_reified;
     WriteBarrier<JSString> m_lastInput;
     WriteBarrier<RegExp> m_lastRegExp;
-    WriteBarrier<RegExpMatchesArray> m_reifiedResult;
+    WriteBarrier<JSArray> m_reifiedResult;
     WriteBarrier<JSString> m_reifiedInput;
+    WriteBarrier<JSString> m_reifiedLeftContext;
+    WriteBarrier<JSString> m_reifiedRightContext;
 };
 
 } // namespace JSC

Source/JavaScriptCore/runtime/RegExpConstructor.cpp

@@ -116,7 +116,7 @@ void RegExpConstructor::visitChildren(JSCell* cell, SlotVisitor& visitor)
 
 JSValue RegExpConstructor::getBackref(ExecState* exec, unsigned i)
 {
-    RegExpMatchesArray* array = m_cachedResult.lastResult(exec, this);
+    JSArray* array = m_cachedResult.lastResult(exec, this);
 
     if (i < array->length()) {
         JSValue result = JSValue(array).get(exec, i);
@@ -129,7 +129,7 @@ JSValue RegExpConstructor::getBackref(ExecState* exec, unsigned i)
 
 JSValue RegExpConstructor::getLastParen(ExecState* exec)
 {
-    RegExpMatchesArray* array = m_cachedResult.lastResult(exec, this);
+    JSArray* array = m_cachedResult.lastResult(exec, this);
     unsigned length = array->length();
     if (length > 1) {
         JSValue result = JSValue(array).get(exec, length - 1);
@@ -142,12 +142,12 @@ JSValue RegExpConstructor::getLastParen(ExecState* exec)
 
 JSValue RegExpConstructor::getLeftContext(ExecState* exec)
 {
-    return m_cachedResult.lastResult(exec, this)->leftContext(exec);
+    return m_cachedResult.leftContext(exec, this);
 }
 
 JSValue RegExpConstructor::getRightContext(ExecState* exec)
 {
-    return m_cachedResult.lastResult(exec, this)->rightContext(exec);
+    return m_cachedResult.rightContext(exec, this);
 }
 
 bool RegExpConstructor::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)

Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -31,93 +31,37 @@
 
 namespace JSC {
 
-const ClassInfo RegExpMatchesArray::s_info = {"Array", &JSArray::s_info, 0, CREATE_METHOD_TABLE(RegExpMatchesArray)};
-
-RegExpMatchesArray::RegExpMatchesArray(VM& vm, Butterfly* butterfly, JSGlobalObject* globalObject, JSString* input, RegExp* regExp, MatchResult result)
-    : JSArray(vm, globalObject->regExpMatchesArrayStructure(), butterfly)
-    , m_result(result)
-    , m_state(ReifiedNone)
-{
-    m_input.set(vm, this, input);
-    m_regExp.set(vm, this, regExp);
-}
-
-RegExpMatchesArray* RegExpMatchesArray::create(ExecState* exec, JSString* input, RegExp* regExp, MatchResult result)
+JSArray* createRegExpMatchesArray(ExecState* exec, JSString* input, RegExp* regExp, MatchResult result)
 {
     ASSERT(result);
     VM& vm = exec->vm();
-    Butterfly* butterfly = createArrayButterflyWithExactLength(vm, 0, regExp->numSubpatterns() + 1);
-    RegExpMatchesArray* array = new (NotNull, allocateCell<RegExpMatchesArray>(vm.heap)) RegExpMatchesArray(vm, butterfly, exec->lexicalGlobalObject(), input, regExp, result);
-    array->finishCreation(vm);
-    return array;
-}
-
-void RegExpMatchesArray::finishCreation(VM& vm)
-{
-    Base::finishCreation(vm);
-}
-
-void RegExpMatchesArray::visitChildren(JSCell* cell, SlotVisitor& visitor)
-{
-    RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(cell);
-    ASSERT_GC_OBJECT_INHERITS(thisObject, info());
-    Base::visitChildren(thisObject, visitor);
-    visitor.append(&thisObject->m_input);
-    visitor.append(&thisObject->m_regExp);
-}
+    JSArray* array = JSArray::tryCreateUninitialized(vm, exec->lexicalGlobalObject()->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1);
+    RELEASE_ASSERT(array);
 
-void RegExpMatchesArray::reifyAllProperties(ExecState* exec)
-{
-    ASSERT(m_state != ReifiedAll);
-    ASSERT(m_result);
-
     SamplingRegion samplingRegion("Reifying substring properties");
-
-    reifyMatchPropertyIfNecessary(exec);
 
-    if (unsigned numSubpatterns = m_regExp->numSubpatterns()) {
+    array->putDirectIndex(exec, 0, jsSubstring(exec, input, result.start, result.end - result.start));
+
+    if (unsigned numSubpatterns = regExp->numSubpatterns()) {
         Vector<int, 32> subpatternResults;
-        int position = m_regExp->match(exec->vm(), m_input->value(exec), m_result.start, subpatternResults);
-        ASSERT_UNUSED(position, position >= 0 && static_cast<size_t>(position) == m_result.start);
-        ASSERT(m_result.start == static_cast<size_t>(subpatternResults[0]));
-        ASSERT(m_result.end == static_cast<size_t>(subpatternResults[1]));
+        int position = regExp->match(exec->vm(), input->value(exec), result.start, subpatternResults);
+        ASSERT_UNUSED(position, position >= 0 && static_cast<size_t>(position) == result.start);
+        ASSERT(result.start == static_cast<size_t>(subpatternResults[0]));
+        ASSERT(result.end == static_cast<size_t>(subpatternResults[1]));
 
         for (unsigned i = 1; i <= numSubpatterns; ++i) {
             int start = subpatternResults[2 * i];
             if (start >= 0)
-                putDirectIndex(exec, i, jsSubstring(exec, m_input.get(), start, subpatternResults[2 * i + 1] - start));
+                array->putDirectIndex(exec, i, jsSubstring(exec, input, start, subpatternResults[2 * i + 1] - start));
             else
-                putDirectIndex(exec, i, jsUndefined());
+                array->putDirectIndex(exec, i, jsUndefined());
         }
     }
 
-    putDirect(exec->vm(), exec->propertyNames().index, jsNumber(m_result.start));
-    putDirect(exec->vm(), exec->propertyNames().input, m_input.get());
-
-    m_state = ReifiedAll;
-}
-
-void RegExpMatchesArray::reifyMatchProperty(ExecState* exec)
-{
-    ASSERT(m_state == ReifiedNone);
-    ASSERT(m_result);
-    putDirectIndex(exec, 0, jsSubstring(exec, m_input.get(), m_result.start, m_result.end - m_result.start));
-    m_state = ReifiedMatch;
-}
-
-JSString* RegExpMatchesArray::leftContext(ExecState* exec)
-{
-    if (!m_result.start)
-        return jsEmptyString(exec);
-    return jsSubstring(exec, m_input.get(), 0, m_result.start);
-}
+    array->putDirect(exec->vm(), exec->propertyNames().index, jsNumber(result.start));
+    array->putDirect(exec->vm(), exec->propertyNames().input, input);
 
-JSString* RegExpMatchesArray::rightContext(ExecState* exec)
-{
-    unsigned length = m_input->length();
-    if (m_result.end == length)
-        return jsEmptyString(exec);
-    return jsSubstring(exec, m_input.get(), m_result.end, length - m_result.end);
+    return array;
 }
 
 } // namespace JSC