RESOLVED WONTFIX 45749
Extend YARR JIT with beginning character look-up optimization
https://bugs.webkit.org/show_bug.cgi?id=45749
Summary Extend YARR JIT with beginning character look-up optimization
Peter Varga
Reported 2010-09-14 07:35:29 PDT
Search the start index of the first potential match before the pattern matching process. Use the vector of possible beginning characters which are collected by YARR's parser.
Attachments
proposed patch (4.21 KB, patch)
2010-09-14 07:39 PDT, Peter Varga
no flags
proposed patch v2 (4.30 KB, patch)
2010-09-20 04:12 PDT, Peter Varga
no flags
proposed patch v3 (4.09 KB, patch)
2010-10-21 08:20 PDT, Peter Varga
eric: review-
proposed patch v3 (4.09 KB, patch)
2010-12-06 02:35 PST, Peter Varga
no flags
Peter Varga
Comment 1 2010-09-14 07:39:47 PDT
Created attachment 67548 [details] proposed patch Performance results: ref mod regexp-dna: 1.060x as fast 21.1ms +/- 1.1% 19.9ms +/- 1.1% v8-regexp: 1.011x as slow 350.9ms +/- 0.2% 354.7ms +/- 0.1%
Early Warning System Bot
Comment 2 2010-09-14 07:54:49 PDT
WebKit Review Bot
Comment 3 2010-09-14 08:00:33 PDT
Csaba Osztrogonác
Comment 4 2010-09-14 08:06:00 PDT
(In reply to comment #2) > Attachment 67548 [details] did not build on qt: > Build output: http://queues.webkit.org/results/3998008 It will build on Qt after https://bugs.webkit.org/show_bug.cgi?id=45748 fixed.
WebKit Review Bot
Comment 5 2010-09-14 08:48:34 PDT
Geoffrey Garen
Comment 6 2010-09-14 13:53:00 PDT
Do you know why v8-regexp got slower?
Peter Varga
Comment 7 2010-09-15 07:19:01 PDT
(In reply to comment #6) > Do you know why v8-regexp got slower? I made some other measurements: - I increased the number of iterations in the v8-regexp benchmark. The ratio of the difference between the two results didn't increase. - I just measured with the parser's patch (https://bugs.webkit.org/show_bug.cgi?id=45748). I noticed that the slow-down still remains. - On another machine the v8 benchmark's results were different. v8-regexp was faster than the reference but other tests' (which don't contain regular expressions) performance results were different. I think it is caused by deviation. It looks like the slow-down with v8-regexp is caused by the beginning character collecting in the parser. I'm working on this problem.
Geoffrey Garen
Comment 8 2010-09-15 10:32:52 PDT
Any idea why this would slow down the JIT but not the interpreter? Your interpreter results show a solid improvement on v8-regexp. It's surprising that the JIT wouldn't improve as well.
Peter Varga
Comment 9 2010-09-15 11:34:28 PDT
(In reply to comment #8) > Any idea why this would slow down the JIT but not the interpreter? Your interpreter results show a solid improvement on v8-regexp. It's surprising that the JIT wouldn't improve as well. I think this optimization haven't enough possibilities to make faster the pattern matching in case of v8-regexp test. Therefore the overhead of the character collecting is greater than the profit of the optimization in case of JIT. However this optimization is more efficient in case of interpreter and the overhead of character collecting is same in both case, therefore the overhead should be negligible when the interpreter does the pattern matching.
Gavin Barraclough
Comment 10 2010-09-17 18:12:19 PDT
Hi Peter, I think Michael is going to take a look over your patch, since he's been working in this area at the minute. He has a patch on bugs.webkit.org/show_bug.cgi?id=45787 that I imagine could conflict with this, which is at commit-queue+ at the minute. You may need to update your patches once this has landed to resolve any merge issues. cheers, G.
Peter Varga
Comment 11 2010-09-20 04:12:42 PDT
Created attachment 68070 [details] proposed patch v2 updated patch for top of trunk.
Early Warning System Bot
Comment 12 2010-09-20 04:19:41 PDT
WebKit Review Bot
Comment 13 2010-09-20 04:52:45 PDT
WebKit Review Bot
Comment 14 2010-09-20 05:08:11 PDT
Peter Varga
Comment 15 2010-09-20 09:20:10 PDT
The new performance results without the collecting of character classes: ref mod regexp-dna: 1.055x as fast 21.1ms +/- 1.1% 20.0ms +/- 0.0% v8-regexp: ?? 316.6ms +/- 0.6% 317.6ms +/- 0.2%
Peter Varga
Comment 16 2010-10-21 08:20:56 PDT
Created attachment 71439 [details] proposed patch v3 update patch.
Early Warning System Bot
Comment 17 2010-11-16 13:08:29 PST
Build Bot
Comment 18 2010-11-16 15:18:22 PST
Eric Seidel (no email)
Comment 19 2010-12-03 13:30:15 PST
Comment on attachment 71439 [details] proposed patch v3 r- for qt and win failures.
Peter Varga
Comment 20 2010-12-06 02:35:41 PST
Created attachment 75659 [details] proposed patch v3 I upload the previous patch again. It has failed on the last ews check because a depedency wasn't landed but now it seems ok.
Eric Seidel (no email)
Comment 21 2011-05-23 11:00:10 PDT
Oliver, Gavin: This patch has been up for review for 6 months. it's tiny. Please r- it or r+ it. I will r- this next week if it still remains undecided.
Gavin Barraclough
Comment 22 2011-05-23 14:22:49 PDT
Comment on attachment 75659 [details] proposed patch v3 Clearing review flag pending 61306
Gavin Barraclough
Comment 23 2011-05-24 13:02:02 PDT
Marking this won't fix based on resolution to r61306.
Note You need to log in before you can comment on or make changes to this bug.