Bug 81742 - RegExpMatchesArray should not copy the ovector
Summary: RegExpMatchesArray should not copy the ovector
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2012-03-20 22:19 PDT by Gavin Barraclough
Modified: 2012-03-24 12:09 PDT (History)
1 user (show)

See Also:

Fix (35.76 KB, patch)
2012-03-20 22:21 PDT, Gavin Barraclough
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2012-03-20 22:19:40 PDT
Currently, all RegExpMatchesArray object contain Vector<int, 32>, used to hold any sub-pattern results.  This makes allocation/construction/destruction of these objects more expensive.  Instead, just store the main match, and recreate the sub-pattern ranges only if necessary (these are often only used for grouping, and the results never accessed).  If the main match (index 0) of the RegExpMatchesArray is accessed, reify that value alone.
Comment 1 Gavin Barraclough 2012-03-20 22:21:08 PDT
Created attachment 132972 [details]
Comment 2 WebKit Review Bot 2012-03-20 22:23:40 PDT
Attachment 132972 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/runtime/RegExpObject.h:29:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/RegExpObject.h:120:  The parameter name "string" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 9 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Michael Saboff 2012-03-21 11:00:04 PDT
Comment on attachment 132972 [details]


Please fix the style issues.

What is the performance implication of this change?
Comment 4 Gavin Barraclough 2012-03-21 14:04:28 PDT
Fixed in r111603.
Comment 5 Oliver Hunt 2012-03-24 12:09:58 PDT
Comment on attachment 132972 [details]

Clearing review flags as comments imply this has landed.