Bug 43610

Summary: Set the shouldLazyAttach flag to true in the tree builder's adoption agency algorithm
Product: WebKit Reporter: James Robinson <jamesr>
Component: New BugsAssignee: James Robinson <jamesr>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, hyatt, mrobinson, ossy, tonyg, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch none

James Robinson
Reported 2010-08-05 23:19:53 PDT
Set the shouldLazyAttach flag to true in the tree builder's adoption agency algorithm
Attachments
Patch (9.50 KB, patch)
2010-08-05 23:22 PDT, James Robinson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
James Robinson
Comment 1 2010-08-05 23:22:48 PDT
Created attachment 63696 [details] Patch
James Robinson
Comment 2 2010-08-05 23:23:28 PDT
I don't fully understand why the render tree output is different. Both the before and after seem sane to me.
Adam Barth
Comment 3 2010-08-05 23:30:17 PDT
Comment on attachment 63696 [details] Patch I'd like to see the dom diffs on these tests. They're easy to generate with a dump-as-markup script tag.
James Robinson
Comment 4 2010-08-05 23:38:13 PDT
WebCore/benchmarks/parser/html-parser.html says: before: avg 3172.45 stdev 23.66743543352342 after: avg 3172.5 stdev 26.693632199459106 This makes sense - I would not expect the HTML5 spec to have many misnested tags requiring the adoption agency.
James Robinson
Comment 5 2010-08-05 23:45:52 PDT
Running the tests as dump-as-markup shows no changes with or without the lazy attach flag set. It's render tree only diffs.
Adam Barth
Comment 6 2010-08-05 23:49:22 PDT
Comment on attachment 63696 [details] Patch Awesome. LGTM.
James Robinson
Comment 7 2010-08-06 00:09:48 PDT
Thanks for the review. Will land this tomorrow so I can take care of any platform-specific baselines that need updating.
Eric Seidel (no email)
Comment 8 2010-08-06 10:39:29 PDT
We desperately need to move to the world of full lazy attach.
Eric Seidel (no email)
Comment 9 2010-08-06 10:40:16 PDT
I think we had skipped the hang test on gtk before because it was too slow. Now it might be fast enough!
James Robinson
Comment 10 2010-08-08 12:30:20 PDT
Committed r64954: <http://trac.webkit.org/changeset/64954>
James Robinson
Comment 11 2010-08-08 12:31:24 PDT
Landed, watchin' the bots now to see if anything needs rebaselines that I couldn't generate myself. I (perhaps optimistically) unskipped residual-style-hang on Gtk to see if it runs OK now - if it's too slow I'll just skip it again.
WebKit Review Bot
Comment 12 2010-08-08 12:43:59 PDT
http://trac.webkit.org/changeset/64954 might have broken Qt Windows 32-bit Debug
James Robinson
Comment 13 2010-08-08 13:45:06 PDT
Comment on attachment 63696 [details] Patch Clearing flags (was abarth r+)
Csaba Osztrogonác
Comment 14 2010-08-08 16:16:29 PDT
(In reply to comment #12) > http://trac.webkit.org/changeset/64954 might have broken Qt Windows 32-bit Debug Sorry, it was false positive alarm. I fixed the bot.
Note You need to log in before you can comment on or make changes to this bug.