Bug 47906 – The number of recursive match calls isn't limited in YARR Interpreter

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

[Peter Varga]

The YARR Interpreter fails on the fast/regex/slow.html layout test 
because the number of recursive match calls is unbounded.

[Peter Varga]

Created attachment 71172 [details]
proposed patch

[Andras Becsi]

(In reply to comment #1)
> Created an attachment (id=71172) [details]
> proposed patch

If I'm correct this should fix the fast/regex/slow.html time out of bug 46719. Does this patch affect the performance somehow, especially because of the newly introduced checks? Would be great to see some perf results, too.
Great work, btw!

[Peter Varga]

(In reply to comment #2)
> If I'm correct this should fix the fast/regex/slow.html time out of bug 46719. Does this patch affect the performance somehow, especially because of the newly introduced checks? Would be great to see some perf results, too.
> Great work, btw!

Performance results:
                                       ref                mod
regexp-dna:   1.008x as fast   564.7ms +/- 0.2%   560.3ms +/- 0.3%
v8-regexp:    1.016x as fast  3497.1ms +/- 0.3%  3441.3ms +/- 0.3%

[Peter Varga]

Created attachment 71181 [details]
proposed patch v2

Fix to free allocated memory in case of failure.

[Gavin Barraclough]

Comment on attachment 71181 [details]
proposed patch v2

Hmmm, I'm pondering.  So, I don't think this is the perfect solution, but it's probably the best solution right now.

I think we want to move away as much as possible from arbitrary limits like this, and instead be using the timeout approach used by the JS language VM.  This presently requires polling, but we may be able to make this more asynchronous.  But this is likely a much more involved change, and best handled separately – just reintroducing the fixed limit for now is probably sufficient, and a sensible step.  Let me cogitate a little more.  By the way, 1000000 is the same limit as PCRE I assume?

[Peter Varga]

(In reply to comment #5)
> (From update of attachment 71181 [details])
> By the way, 1000000 is the same limit as PCRE I assume?

Yes, it's same. The limit and the logic of check are from pcre.

[Peter Varga]

ping

[WebKit Commit Bot]

Comment on attachment 71181 [details]
proposed patch v2

Clearing flags on attachment: 71181

Committed r72140: <http://trac.webkit.org/changeset/72140>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.