Title: [104751] trunk/Source/_javascript_Core
Revision
104751
Author
msab...@apple.com
Date
2012-01-11 15:27:08 -0800 (Wed, 11 Jan 2012)

Log Message

v8-regexp spends 35% of its time allocating and copying internal regexp results data
https://bugs.webkit.org/show_bug.cgi?id=76079

Reviewed by Geoffrey Garen.

Added a new RegExpResults struct that has the input string, the number of
subexpressions and the output vector.  Changed RegExpConstructor to
include a RegExpConstructorPrivate instead of having a reference to one.
Changed RegExpMatchesArray to include a RegExpResults instead of a 
reference to a RegExpConstructorPrivate.  Created an overloaded assignment
operator to assign a RegExpConstructorPrivate to a RegExpResults.
Collectively this change is worth 24% performance improvement to v8-regexp.
        
* runtime/RegExpConstructor.cpp:
(JSC::RegExpResult::operator=):
(JSC::RegExpConstructor::RegExpConstructor):
(JSC::RegExpMatchesArray::RegExpMatchesArray):
(JSC::RegExpMatchesArray::finishCreation):
(JSC::RegExpMatchesArray::~RegExpMatchesArray):
(JSC::RegExpMatchesArray::fillArrayInstance):
(JSC::RegExpConstructor::arrayOfMatches):
(JSC::RegExpConstructor::getBackref):
(JSC::RegExpConstructor::getLastParen):
(JSC::RegExpConstructor::getLeftContext):
(JSC::RegExpConstructor::getRightContext):
(JSC::RegExpConstructor::setInput):
(JSC::RegExpConstructor::input):
(JSC::RegExpConstructor::setMultiline):
(JSC::RegExpConstructor::multiline):
* runtime/RegExpConstructor.h:
(JSC::RegExpResult::RegExpResult):
(JSC::RegExpConstructor::performMatch):
* runtime/RegExpMatchesArray.h:
(JSC::RegExpMatchesArray::create):
(JSC::RegExpMatchesArray::getOwnPropertySlot):
(JSC::RegExpMatchesArray::getOwnPropertySlotByIndex):
(JSC::RegExpMatchesArray::getOwnPropertyDescriptor):
(JSC::RegExpMatchesArray::put):
(JSC::RegExpMatchesArray::putByIndex):
(JSC::RegExpMatchesArray::deleteProperty):
(JSC::RegExpMatchesArray::deletePropertyByIndex):
(JSC::RegExpMatchesArray::getOwnPropertyNames):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (104750 => 104751)


--- trunk/Source/_javascript_Core/ChangeLog	2012-01-11 23:15:20 UTC (rev 104750)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-01-11 23:27:08 UTC (rev 104751)
@@ -1,3 +1,48 @@
+2012-01-11  Michael Saboff  <msab...@apple.com>
+
+        v8-regexp spends 35% of its time allocating and copying internal regexp results data
+        https://bugs.webkit.org/show_bug.cgi?id=76079
+
+        Reviewed by Geoffrey Garen.
+
+        Added a new RegExpResults struct that has the input string, the number of
+        subexpressions and the output vector.  Changed RegExpConstructor to
+        include a RegExpConstructorPrivate instead of having a reference to one.
+        Changed RegExpMatchesArray to include a RegExpResults instead of a 
+        reference to a RegExpConstructorPrivate.  Created an overloaded assignment
+        operator to assign a RegExpConstructorPrivate to a RegExpResults.
+        Collectively this change is worth 24% performance improvement to v8-regexp.
+        
+        * runtime/RegExpConstructor.cpp:
+        (JSC::RegExpResult::operator=):
+        (JSC::RegExpConstructor::RegExpConstructor):
+        (JSC::RegExpMatchesArray::RegExpMatchesArray):
+        (JSC::RegExpMatchesArray::finishCreation):
+        (JSC::RegExpMatchesArray::~RegExpMatchesArray):
+        (JSC::RegExpMatchesArray::fillArrayInstance):
+        (JSC::RegExpConstructor::arrayOfMatches):
+        (JSC::RegExpConstructor::getBackref):
+        (JSC::RegExpConstructor::getLastParen):
+        (JSC::RegExpConstructor::getLeftContext):
+        (JSC::RegExpConstructor::getRightContext):
+        (JSC::RegExpConstructor::setInput):
+        (JSC::RegExpConstructor::input):
+        (JSC::RegExpConstructor::setMultiline):
+        (JSC::RegExpConstructor::multiline):
+        * runtime/RegExpConstructor.h:
+        (JSC::RegExpResult::RegExpResult):
+        (JSC::RegExpConstructor::performMatch):
+        * runtime/RegExpMatchesArray.h:
+        (JSC::RegExpMatchesArray::create):
+        (JSC::RegExpMatchesArray::getOwnPropertySlot):
+        (JSC::RegExpMatchesArray::getOwnPropertySlotByIndex):
+        (JSC::RegExpMatchesArray::getOwnPropertyDescriptor):
+        (JSC::RegExpMatchesArray::put):
+        (JSC::RegExpMatchesArray::putByIndex):
+        (JSC::RegExpMatchesArray::deleteProperty):
+        (JSC::RegExpMatchesArray::deletePropertyByIndex):
+        (JSC::RegExpMatchesArray::getOwnPropertyNames):
+
 2012-01-11  Eugene Girard  <gir...@google.com>
 
         Typo in error message: Unexpected token 'defualt'

Modified: trunk/Source/_javascript_Core/runtime/RegExpConstructor.cpp (104750 => 104751)


--- trunk/Source/_javascript_Core/runtime/RegExpConstructor.cpp	2012-01-11 23:15:20 UTC (rev 104750)
+++ trunk/Source/_javascript_Core/runtime/RegExpConstructor.cpp	2012-01-11 23:27:08 UTC (rev 104751)
@@ -97,9 +97,18 @@
 @end
 */
 
+RegExpResult& RegExpResult::operator=(const RegExpConstructorPrivate& rhs)
+{
+    this->input = rhs.input;
+    this->ovector = rhs.lastOvector();
+    this->lastNumSubPatterns = rhs.lastNumSubPatterns;
+    
+    return *this;            
+}
+
+
 RegExpConstructor::RegExpConstructor(JSGlobalObject* globalObject, Structure* structure)
     : InternalFunction(globalObject, structure)
-    , d(adoptPtr(new RegExpConstructorPrivate))
 {
 }
 
@@ -122,29 +131,16 @@
 
 RegExpMatchesArray::RegExpMatchesArray(ExecState* exec)
     : JSArray(exec->globalData(), exec->lexicalGlobalObject()->regExpMatchesArrayStructure())
+    , m_didFillArrayInstance(false)
 {
 }
 
-void RegExpMatchesArray::finishCreation(JSGlobalData& globalData, RegExpConstructorPrivate* data)
+void RegExpMatchesArray::finishCreation(JSGlobalData& globalData, const RegExpConstructorPrivate& data)
 {
-    Base::finishCreation(globalData, data->lastNumSubPatterns + 1);
-    RegExpConstructorPrivate* d = new RegExpConstructorPrivate;
-    d->input = data->lastInput;
-    d->lastInput = data->lastInput;
-    d->lastNumSubPatterns = data->lastNumSubPatterns;
-    unsigned offsetVectorSize = (data->lastNumSubPatterns + 1) * 2; // only copying the result part of the vector
-    d->lastOvector().resize(offsetVectorSize);
-    memcpy(d->lastOvector().data(), data->lastOvector().data(), offsetVectorSize * sizeof(int));
-    // d->multiline is not needed, and remains uninitialized
-
-    setSubclassData(d);
+    Base::finishCreation(globalData, data.lastNumSubPatterns + 1);
+    m_regExpResult = data;
 }
 
-RegExpMatchesArray::~RegExpMatchesArray()
-{
-    delete static_cast<RegExpConstructorPrivate*>(subclassData());
-}
-
 void RegExpMatchesArray::destroy(JSCell* cell)
 {
     jsCast<RegExpMatchesArray*>(cell)->RegExpMatchesArray::~RegExpMatchesArray();
@@ -152,65 +148,61 @@
 
 void RegExpMatchesArray::fillArrayInstance(ExecState* exec)
 {
-    RegExpConstructorPrivate* d = static_cast<RegExpConstructorPrivate*>(subclassData());
-    ASSERT(d);
+    unsigned lastNumSubpatterns = m_regExpResult.lastNumSubPatterns;
 
-    unsigned lastNumSubpatterns = d->lastNumSubPatterns;
-
     for (unsigned i = 0; i <= lastNumSubpatterns; ++i) {
-        int start = d->lastOvector()[2 * i];
+        int start = m_regExpResult.ovector[2 * i];
         if (start >= 0)
-            JSArray::putByIndex(this, exec, i, jsSubstring(exec, d->lastInput, start, d->lastOvector()[2 * i + 1] - start));
+            JSArray::putByIndex(this, exec, i, jsSubstring(exec, m_regExpResult.input, start, m_regExpResult.ovector[2 * i + 1] - start));
         else
             JSArray::putByIndex(this, exec, i, jsUndefined());
     }
 
     PutPropertySlot slot;
-    JSArray::put(this, exec, exec->propertyNames().index, jsNumber(d->lastOvector()[0]), slot);
-    JSArray::put(this, exec, exec->propertyNames().input, jsString(exec, d->input), slot);
+    JSArray::put(this, exec, exec->propertyNames().index, jsNumber(m_regExpResult.ovector[0]), slot);
+    JSArray::put(this, exec, exec->propertyNames().input, jsString(exec, m_regExpResult.input), slot);
 
-    delete d;
-    setSubclassData(0);
+    m_didFillArrayInstance = true;
 }
 
 JSObject* RegExpConstructor::arrayOfMatches(ExecState* exec) const
 {
-    return RegExpMatchesArray::create(exec, d.get());
+    return RegExpMatchesArray::create(exec, d);
 }
 
 JSValue RegExpConstructor::getBackref(ExecState* exec, unsigned i) const
 {
-    if (!d->lastOvector().isEmpty() && i <= d->lastNumSubPatterns) {
-        int start = d->lastOvector()[2 * i];
+    if (!d.lastOvector().isEmpty() && i <= d.lastNumSubPatterns) {
+        int start = d.lastOvector()[2 * i];
         if (start >= 0)
-            return jsSubstring(exec, d->lastInput, start, d->lastOvector()[2 * i + 1] - start);
+            return jsSubstring(exec, d.lastInput, start, d.lastOvector()[2 * i + 1] - start);
     }
     return jsEmptyString(exec);
 }
 
 JSValue RegExpConstructor::getLastParen(ExecState* exec) const
 {
-    unsigned i = d->lastNumSubPatterns;
+    unsigned i = d.lastNumSubPatterns;
     if (i > 0) {
-        ASSERT(!d->lastOvector().isEmpty());
-        int start = d->lastOvector()[2 * i];
+        ASSERT(!d.lastOvector().isEmpty());
+        int start = d.lastOvector()[2 * i];
         if (start >= 0)
-            return jsSubstring(exec, d->lastInput, start, d->lastOvector()[2 * i + 1] - start);
+            return jsSubstring(exec, d.lastInput, start, d.lastOvector()[2 * i + 1] - start);
     }
     return jsEmptyString(exec);
 }
 
 JSValue RegExpConstructor::getLeftContext(ExecState* exec) const
 {
-    if (!d->lastOvector().isEmpty())
-        return jsSubstring(exec, d->lastInput, 0, d->lastOvector()[0]);
+    if (!d.lastOvector().isEmpty())
+        return jsSubstring(exec, d.lastInput, 0, d.lastOvector()[0]);
     return jsEmptyString(exec);
 }
 
 JSValue RegExpConstructor::getRightContext(ExecState* exec) const
 {
-    if (!d->lastOvector().isEmpty())
-        return jsSubstring(exec, d->lastInput, d->lastOvector()[1], d->lastInput.length() - d->lastOvector()[1]);
+    if (!d.lastOvector().isEmpty())
+        return jsSubstring(exec, d.lastInput, d.lastOvector()[1], d.lastInput.length() - d.lastOvector()[1]);
     return jsEmptyString(exec);
 }
     
@@ -377,24 +369,24 @@
 
 void RegExpConstructor::setInput(const UString& input)
 {
-    d->input = input;
+    d.input = input;
 }
 
 const UString& RegExpConstructor::input() const
 {
     // Can detect a distinct initial state that is invisible to _javascript_, by checking for null
     // state (since jsString turns null strings to empty strings).
-    return d->input;
+    return d.input;
 }
 
 void RegExpConstructor::setMultiline(bool multiline)
 {
-    d->multiline = multiline;
+    d.multiline = multiline;
 }
 
 bool RegExpConstructor::multiline() const
 {
-    return d->multiline;
+    return d.multiline;
 }
 
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/runtime/RegExpConstructor.h (104750 => 104751)


--- trunk/Source/_javascript_Core/runtime/RegExpConstructor.h	2012-01-11 23:15:20 UTC (rev 104750)
+++ trunk/Source/_javascript_Core/runtime/RegExpConstructor.h	2012-01-11 23:27:08 UTC (rev 104751)
@@ -55,6 +55,21 @@
         unsigned lastOvectorIndex : 1;
     };
 
+    struct RegExpResult {
+        WTF_MAKE_FAST_ALLOCATED;
+    public:
+        RegExpResult()
+        : lastNumSubPatterns(0)
+        {
+        }
+        
+        RegExpResult& operator=(const RegExpConstructorPrivate&);
+        
+        UString input;
+        unsigned lastNumSubPatterns;
+        Vector<int, 32> ovector;
+    };
+    
     class RegExpConstructor : public InternalFunction {
     public:
         typedef InternalFunction Base;
@@ -102,7 +117,7 @@
         static ConstructType getConstructData(JSCell*, ConstructData&);
         static CallType getCallData(JSCell*, CallData&);
 
-        OwnPtr<RegExpConstructorPrivate> d;
+        RegExpConstructorPrivate d;
     };
 
     RegExpConstructor* asRegExpConstructor(JSValue);
@@ -122,20 +137,20 @@
     */
     ALWAYS_INLINE void RegExpConstructor::performMatch(JSGlobalData& globalData, RegExp* r, const UString& s, int startOffset, int& position, int& length, int** ovector)
     {
-        position = r->match(globalData, s, startOffset, &d->tempOvector());
+        position = r->match(globalData, s, startOffset, &d.tempOvector());
 
         if (ovector)
-            *ovector = d->tempOvector().data();
+            *ovector = d.tempOvector().data();
 
         if (position != -1) {
-            ASSERT(!d->tempOvector().isEmpty());
+            ASSERT(!d.tempOvector().isEmpty());
 
-            length = d->tempOvector()[1] - d->tempOvector()[0];
+            length = d.tempOvector()[1] - d.tempOvector()[0];
 
-            d->input = s;
-            d->lastInput = s;
-            d->changeLastOvector();
-            d->lastNumSubPatterns = r->numSubpatterns();
+            d.input = s;
+            d.lastInput = s;
+            d.changeLastOvector();
+            d.lastNumSubPatterns = r->numSubpatterns();
         }
     }
 

Modified: trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.h (104750 => 104751)


--- trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.h	2012-01-11 23:15:20 UTC (rev 104750)
+++ trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.h	2012-01-11 23:27:08 UTC (rev 104751)
@@ -31,30 +31,29 @@
     public:
         typedef JSArray Base;
 
-        static RegExpMatchesArray* create(ExecState* exec, RegExpConstructorPrivate* ctorPrivate)
+        static RegExpMatchesArray* create(ExecState* exec, const RegExpConstructorPrivate& ctorPrivate)
         {
             RegExpMatchesArray* regExp = new (NotNull, allocateCell<RegExpMatchesArray>(*exec->heap())) RegExpMatchesArray(exec);
             regExp->finishCreation(exec->globalData(), ctorPrivate);
             return regExp;
         }
-        ~RegExpMatchesArray();
         static void destroy(JSCell*);
 
         static JS_EXPORTDATA const ClassInfo s_info;
-        
+
         static Structure* createStructure(JSGlobalData& globalData, JSGlobalObject* globalObject, JSValue prototype)
         {
             return Structure::create(globalData, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), &s_info);
         }
-        
+
     protected:
-        void finishCreation(JSGlobalData&, RegExpConstructorPrivate* data);
+        void finishCreation(JSGlobalData&, const RegExpConstructorPrivate& data);
 
     private:
         static bool getOwnPropertySlot(JSCell* cell, ExecState* exec, const Identifier& propertyName, PropertySlot& slot)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(cell);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             return JSArray::getOwnPropertySlot(thisObject, exec, propertyName, slot);
         }
@@ -62,7 +61,7 @@
         static bool getOwnPropertySlotByIndex(JSCell* cell, ExecState* exec, unsigned propertyName, PropertySlot& slot)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(cell);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             return JSArray::getOwnPropertySlotByIndex(thisObject, exec, propertyName, slot);
         }
@@ -70,7 +69,7 @@
         static bool getOwnPropertyDescriptor(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(object);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             return JSArray::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
         }
@@ -78,7 +77,7 @@
         static void put(JSCell* cell, ExecState* exec, const Identifier& propertyName, JSValue v, PutPropertySlot& slot)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(cell);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             JSArray::put(thisObject, exec, propertyName, v, slot);
         }
@@ -86,7 +85,7 @@
         static void putByIndex(JSCell* cell, ExecState* exec, unsigned propertyName, JSValue v)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(cell);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             JSArray::putByIndex(thisObject, exec, propertyName, v);
         }
@@ -94,7 +93,7 @@
         static bool deleteProperty(JSCell* cell, ExecState* exec, const Identifier& propertyName)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(cell);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             return JSArray::deleteProperty(thisObject, exec, propertyName);
         }
@@ -102,7 +101,7 @@
         static bool deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned propertyName)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(cell);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             return JSArray::deletePropertyByIndex(thisObject, exec, propertyName);
         }
@@ -110,12 +109,15 @@
         static void getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& arr, EnumerationMode mode = ExcludeDontEnumProperties)
         {
             RegExpMatchesArray* thisObject = jsCast<RegExpMatchesArray*>(object);
-            if (thisObject->subclassData())
+            if (!thisObject->m_didFillArrayInstance)
                 thisObject->fillArrayInstance(exec);
             JSArray::getOwnPropertyNames(thisObject, exec, arr, mode);
         }
 
         void fillArrayInstance(ExecState*);
+
+        RegExpResult m_regExpResult;
+        bool m_didFillArrayInstance;
 };
 
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to