Bug 43933 - Improved Table Hit Testing and Painting Performance
Summary: Improved Table Hit Testing and Painting Performance
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-12 14:22 PDT by Fady Samuel
Modified: 2010-08-18 10:53 PDT (History)
9 users (show)

See Also:


Attachments
Table hit testing graph (131.35 KB, image/png)
2010-08-12 14:22 PDT, Fady Samuel
no flags Details
Benchmark script (2.25 KB, text/html)
2010-08-12 14:24 PDT, Fady Samuel
no flags Details
Patch For Faster Table Hit Testing (60.72 KB, patch)
2010-08-12 14:32 PDT, Fady Samuel
hyatt: review-
hyatt: commit-queue-
Details | Formatted Diff | Diff
128*128 table benchmark (2.25 KB, text/html)
2010-08-12 15:50 PDT, Fady Samuel
no flags Details
Revised Patch (106.11 KB, patch)
2010-08-17 11:37 PDT, Fady Samuel
no flags Details | Formatted Diff | Diff
Revised Patch (Fixes merge conflict) (106.49 KB, patch)
2010-08-17 12:47 PDT, Fady Samuel
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fady Samuel 2010-08-12 14:22:51 PDT
Created attachment 64260 [details]
Table hit testing graph

Table hit testing performance in webkit is relatively poor especially when compared to Firefox. It is done currently by visiting every single cell in the table  until a cell is found that is interested in the hit. This can perform horribly as the table size gets larger. I have attached a log-log graph showing before and after performance of table hit testing as well as a comparison with firefox.
Comment 1 Fady Samuel 2010-08-12 14:24:25 PDT
Created attachment 64261 [details]
Benchmark script
Comment 2 Fady Samuel 2010-08-12 14:32:15 PDT
Created attachment 64263 [details]
Patch For Faster Table Hit Testing
Comment 3 Fady Samuel 2010-08-12 14:36:12 PDT
As a real-world demo, this site's performance is SIGNIFICANTLY improved with this patch:

http://tekkub.net/
Comment 4 Fady Samuel 2010-08-12 15:50:37 PDT
Created attachment 64277 [details]
128*128 table benchmark
Comment 5 Dave Hyatt 2010-08-16 13:02:53 PDT
Comment on attachment 64263 [details]
Patch For Faster Table Hit Testing

Minusing.  Comments delivered on IRC.  The code can incorrectly paint a little too much.  Need some precise boundary detection and a simple repaint test etc.
Comment 6 Fady Samuel 2010-08-17 11:37:56 PDT
Created attachment 64612 [details]
Revised Patch

Fixed as per dhyatt's comments.
Comment 7 Fady Samuel 2010-08-17 12:47:28 PDT
Created attachment 64621 [details]
Revised Patch (Fixes merge conflict)
Comment 8 Dave Hyatt 2010-08-17 13:11:09 PDT
Comment on attachment 64621 [details]
Revised Patch (Fixes merge conflict)

r=me
Comment 9 WebKit Commit Bot 2010-08-17 13:33:16 PDT
Comment on attachment 64621 [details]
Revised Patch (Fixes merge conflict)

Rejecting patch 64621 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'David Hyatt', u'--force']" exit_code: 255
Last 500 characters of output:
ng file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/table/simple_paint.html
patching file LayoutTests/platform/chromium-linux/fast/table/simple_paint-expected.checksum
literal is only avaliable with the XS version at /Library/Perl/5.8.8/Compress/Zlib.pm line 9
BEGIN failed--compilation aborted at /Library/Perl/5.8.8/Compress/Zlib.pm line 9.
Compilation failed in require at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/VCSUtils.pm line 1662.

Full output: http://queues.webkit.org/results/3736294
Comment 10 James Robinson 2010-08-17 13:57:36 PDT
Comment on attachment 64621 [details]
Revised Patch (Fixes merge conflict)

Clearing flags on attachment: 64621

Committed r65539: <http://trac.webkit.org/changeset/65539>
Comment 11 James Robinson 2010-08-17 13:57:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 James Robinson 2010-08-17 13:58:37 PDT
Landed by hand.  Eric, why did the commit queue barf on this?
Comment 13 Eric Seidel (no email) 2010-08-17 17:19:01 PDT
We thought we had fixed this.  I just need to move the commit-queue to SnowLeopard to work around this and the CoreVideo media/compositing crash/timeouts.
Comment 14 Eric Seidel (no email) 2010-08-18 10:52:16 PDT
This appears to be missing Mac results. :(
Comment 15 Fady Samuel 2010-08-18 10:53:00 PDT
(In reply to comment #14)
> This appears to be missing Mac results. :(

Yup working on it.