Bug 15749 – RegExp/RegExpObjectImp cause needless UString creation

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Eric Seidel (no email)]

RegExp/RegExpObjectImp cause needless UString creation

performMatch/match return a UString, which is only used by very few callers (and I'm not sure those are even necessary).  Instead it just needs to return the size of the match.  if the size is non-zero, then there was a match (what most callers care about) and the callers which care about the size have that too.  There are a couple cases (replacement) where we need to actually be able to generate the match string, but that can be done using the returned offset vector and some sort of helper function.  I expect this will be a couple percentage points speedup, due to the reduce malloc pressure.

I'm too tired to write it (and busy much of tomorrow).  Anyone who feels the urge should feel free.

[Darin Adler]

Created attachment 16958 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 16958 [details]
patch

Hum... I wonder if there isn't a clean way to use a local OwnArrayPtr to make sure that any early exit cleans up the storage... meaning, only calling localPtr.release() and setting ovector at the very end of the success case.  You'd of course need a local raw ptr which pointed at either at the stack buffer (of size 3) or the newly allocated localPtr's array.  I'm not sure that would be any less complex than the current solution though.  I guess this is sorta what vector is for.

Anyway, sorry not enough brain cells for a full review.  Slightly surprised it wasn't more of a gain (since in at least some tests this was a major cause of UString creation).

[Maciej Stachowiak]

Comment on attachment 16958 [details]
patch

r=me

[Darin Adler]

(In reply to comment #2)
> I guess this is sorta what vector is for.

I toyed with multiple implementations. The first thing I tried was Vector<int>. But it was not more elegant, and in fact it was less efficient since a Vector has overhead to allow for resizing. So I went with this.

[Darin Adler]

Committed revision 27320.