Bug 38988 – Avoid to store the beginning of the match on the stack in Yarr JIT

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

[Peter Varga]

The current JIT solution stores and changes the start character position of
the whole pattern's match on the stack.
To store the beginning of the match directly in the output array can be a better solution
because this value doesn't need to be available by the use of the stack pointer.

[Peter Varga]

Created attachment 55838 [details]
proposed patch

[Eric Seidel (no email)]

Ping?  Any YARR JIT reviewers care able to review this simple patch?

[Geoffrey Garen]

Peter, can you post performance test results? For this patch, I would test SunSpider and v8-regexp.

[Geoffrey Garen]

Comment on attachment 55838 [details]
proposed patch

Code looks good, but I'm going to say r- for now, waiting on performance numbers.

[Peter Varga]

Comment on attachment 55838 [details]
proposed patch

We don't need to do this allocation on stack, so this patch solves the removed TODO. It saves sizeof(int) bytes from stack per matching.
In fact, this patch doesn't even have effect on performance, it improves the memory usage. 
Btw, the performance results are:

               ref                mod
regexp-dna: 22.4ms +/- 1.6%    22.4ms +/- 1.6%
v8-regexp:  445.9ms +/- 2.6%   444.5ms +/- 2.6%

I set to r? again.

[Geoffrey Garen]

Comment on attachment 55838 [details]
proposed patch

r=me

[Peter Varga]

Comment on attachment 55838 [details]
proposed patch

Thanks Geoffrey.
cq removed to land it manually.

[Andras Becsi]

Comment on attachment 55838 [details]
proposed patch

Sending        JavaScriptCore/ChangeLog
Sending        JavaScriptCore/yarr/RegexJIT.cpp
Transmitting file data ..
Committed revision 61244.

Clearing flags.

(In reply to comment #7)
> (From update of attachment 55838 [details])
> Thanks Geoffrey.
> cq removed to land it manually.

[Andras Becsi]

All reviewed patches landed.
Closing bug.