Bug 158157 – regExpProtoFuncSplitFast should OOM before it swaps

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

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 279970 [details]
stuff

I haven't tried compiling it yet.

[Filip Pizlo]

Created attachment 279980 [details]
the patch

[Mark Lam]

Comment on attachment 279980 [details]
the patch

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

r=me

> Source/JavaScriptCore/ChangeLog:15
> +        This is an enormous speed-up on some jsfunfuzz tests: they got from running for a long

/got/go/.

> Source/JavaScriptCore/runtime/RegExpPrototype.cpp:599
> +    unsigned maxSizeForDirectPath = 100000;

Maybe make this a common constant at the top of this file since it appears to be used for more than 1 function?

> Source/JavaScriptCore/runtime/RegExpPrototype.cpp:671
> +    

Please remove blank space.

[Filip Pizlo]

(In reply to comment #3)
> Comment on attachment 279980 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279980&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/ChangeLog:15
> > +        This is an enormous speed-up on some jsfunfuzz tests: they got from running for a long
> 
> /got/go/.

OK!

> 
> > Source/JavaScriptCore/runtime/RegExpPrototype.cpp:599
> > +    unsigned maxSizeForDirectPath = 100000;
> 
> Maybe make this a common constant at the top of this file since it appears
> to be used for more than 1 function?

It appears only once in this file!  It also appears in RegExpObject.h.  I figure that having two copies of the constant is slightly less evil than the alternatives that I can think of:

- Introduce a new header for the constant.
- Introduce the constant into some random header.

> 
> > Source/JavaScriptCore/runtime/RegExpPrototype.cpp:671
> > +    
> 
> Please remove blank space.

OK!

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/201467

[Filip Pizlo]

Committed.