Bug 146006

Summary: [JSC] Pre-bake final Structure for RegExp matches arrays.
Product: WebKit Reporter: Andreas Kling <kling>
Component: JavaScriptCoreAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, kling
Priority: P2 Keywords: Performance
Version: 312.x   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

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>