Bug 76079 – v8-regexp spends 35% of its time allocating and copying internal regexp results data

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

35% of v8-regexp processing is spent allocating and copying match results data.  RegExpConstructor currently takes a pointer to a RegExpConstructorPrivate object that is malloced.  When a RegExpMatchesArray is created, it too mallocs a RegExpConstructorPrivate object and copies the data from the RegExpConstructor instance of the same class.  The RegExpConstructorPrivate has more attributes than needed for RegExpMatchesArray uses.  The two mallocs and some of the copying can be eliminated.

[Michael Saboff]

<rdar://problem/10665597>

[Michael Saboff]

Created attachment 122052 [details]
Patch for review

[Michael Saboff]

Actually the performance benefit is 24%.  I will change the ChangeLog.

[Geoffrey Garen]

Comment on attachment 122052 [details]
Patch for review

View in context: https://bugs.webkit.org/attachment.cgi?id=122052&action=review

r=me

> Source/JavaScriptCore/runtime/RegExpConstructor.cpp:100
> +RegExpResult& RegExpResult::operator=(const RegExpConstructorPrivate& rhs)

Should this be inlined?

> Source/JavaScriptCore/runtime/RegExpConstructor.cpp:143
>  RegExpMatchesArray::~RegExpMatchesArray()

You can remove this entirely -- that will make Mark's work a little easier.

> Source/JavaScriptCore/runtime/RegExpMatchesArray.h:120
> +        RegExpResult privateData;

Minor nit: I would call this variable "regExpResult". The class can have an arbitrary amount of private data -- what's unique about this data is that it's a regular expression result.

> Source/JavaScriptCore/runtime/RegExpMatchesArray.h:121
> +        bool m_filledArrayInstance;

Minor nit: I prefer the style of prefacing boolean predicates with "did" -- "m_didFillArrayInstance". Otherwise, it's ambiguous whether "filledArrayInstance" is a boolean predicate or a noun.

[Geoffrey Garen]

Correction: "m_regExpResult".

[Michael Saboff]

Committed r104751: <http://trac.webkit.org/changeset/104751>