Bug 146006 - [JSC] Pre-bake final Structure for RegExp matches arrays.
Summary: [JSC] Pre-bake final Structure for RegExp matches arrays.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 312.x
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: Performance
Depends on:
Blocks:
 
Reported: 2015-06-15 22:18 PDT by Andreas Kling
Modified: 2015-06-16 11:38 PDT (History)
4 users (show)

See Also:


Attachments
Patch (9.98 KB, patch)
2015-06-16 03:07 PDT, Andreas Kling
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2015-06-15 22:18:46 PDT
RegExp matches arrays always have the "input" and "index" fields. We can save time by pre-baking a Structure with these fields already added instead of starting from scratch every time.
Comment 1 Andreas Kling 2015-06-16 03:07:49 PDT
Created attachment 254947 [details]
Patch

regexp         x2      34.85803+-0.22637    ^    31.52553+-0.38239       ^ definitely 1.1057x faster

:-)
Comment 2 Darin Adler 2015-06-16 10:48:55 PDT
Comment on attachment 254947 [details]
Patch

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

So many pointers that seem like they should be references! JavaScriptCore sometimes seems like it’s on a different continent from the rest of WebKit.

> Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp:91
> +    RELEASE_ASSERT(offset == indexPropertyOffset);

Why RELEASE_ASSERT instead of ASSERT exactly? I guess because this code is run only once and it would be really bad if this was untrue?
Comment 3 Andreas Kling 2015-06-16 11:12:28 PDT
(In reply to comment #2)
> Comment on attachment 254947 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254947&action=review
> 
> So many pointers that seem like they should be references! JavaScriptCore
> sometimes seems like it’s on a different continent from the rest of WebKit.

Truth.

> > Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp:91
> > +    RELEASE_ASSERT(offset == indexPropertyOffset);
> 
> Why RELEASE_ASSERT instead of ASSERT exactly? I guess because this code is
> run only once and it would be really bad if this was untrue?

That was my thinking. It seemed more important when I was really sleepy, a regular assertion will do fine.
Comment 4 Andreas Kling 2015-06-16 11:38:51 PDT
Committed r185597: <http://trac.webkit.org/changeset/185597>