Bug 38988 - Avoid to store the beginning of the match on the stack in Yarr JIT
Summary: Avoid to store the beginning of the match on the stack in Yarr JIT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-12 06:22 PDT by Peter Varga
Modified: 2010-06-16 01:54 PDT (History)
5 users (show)

See Also:


Attachments
proposed patch (4.27 KB, patch)
2010-05-12 06:25 PDT, Peter Varga
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Varga 2010-05-12 06:22:05 PDT
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.
Comment 1 Peter Varga 2010-05-12 06:25:47 PDT
Created attachment 55838 [details]
proposed patch
Comment 2 Eric Seidel (no email) 2010-06-12 21:01:54 PDT
Ping?  Any YARR JIT reviewers care able to review this simple patch?
Comment 3 Geoffrey Garen 2010-06-14 11:50:15 PDT
Peter, can you post performance test results? For this patch, I would test SunSpider and v8-regexp.
Comment 4 Geoffrey Garen 2010-06-14 11:50:46 PDT
Comment on attachment 55838 [details]
proposed patch

Code looks good, but I'm going to say r- for now, waiting on performance numbers.
Comment 5 Peter Varga 2010-06-15 09:31:15 PDT
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.
Comment 6 Geoffrey Garen 2010-06-15 09:47:32 PDT
Comment on attachment 55838 [details]
proposed patch

r=me
Comment 7 Peter Varga 2010-06-16 01:42:53 PDT
Comment on attachment 55838 [details]
proposed patch

Thanks Geoffrey.
cq removed to land it manually.
Comment 8 Andras Becsi 2010-06-16 01:53:17 PDT
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.
Comment 9 Andras Becsi 2010-06-16 01:54:49 PDT
All reviewed patches landed.
Closing bug.