Bug 34059

Summary: Content with heavily nested residual style is so slow, it seems like a hang
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dglazkov, eric, gustavo, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch mitz: review+

Maciej Stachowiak
Reported 2010-01-24 11:40:43 PST
Content with heavily nested residual style is so slow, it seems like a hang
Attachments
Patch (2.30 KB, patch)
2010-01-24 11:47 PST, Maciej Stachowiak 		mitz: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Maciej Stachowiak
Comment 1 2010-01-24 11:47:53 PST
Created attachment 47298 [details] Patch
Maciej Stachowiak
Comment 2 2010-01-24 11:51:59 PST
I wasn't sure whether to provide a test case for this. The only test case I can think of would be annoyingly slow when it passes, and an outright hang when it fails. This means it would probably be on the edge of timing out on the bots, which is not so great.
Eric Seidel (no email)
Comment 3 2010-01-24 11:52:58 PST
Attachment 47298 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/208763
WebKit Review Bot
Comment 4 2010-01-24 11:59:50 PST
Attachment 47298 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/208767
mitz
Comment 5 2010-01-24 12:28:03 PST
Comment on attachment 47298 [details] Patch > + while (!finished && (iterationCount++ < cResidualStyleIterationLimititerationLimit)) { Typo: cResidualStyleIterationLimititerationLimit and redundant parentheses, but otherwise r=me
WebKit Review Bot
Comment 6 2010-01-24 12:28:19 PST
Attachment 47298 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/209685
Alexey Proskuryakov
Comment 7 2010-01-24 12:31:53 PST
Comment on attachment 47298 [details] Patch > + Limit the number of iterations of the main loop to 5. <...> > + while (!finished && (iterationCount++ < cResidualStyleIterationLimititerationLimit)) { I think this actually limits the count to 6, not 5. It would be nice to attach a test case to this Bugzilla bug, so that people could verify that it didn't regress - if it's not possible to make an automated test, of course.
WebKit Review Bot
Comment 8 2010-01-24 12:32:24 PST
Attachment 47298 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/208787
Maciej Stachowiak
Comment 9 2010-01-24 18:08:04 PST
(In reply to comment #7) > (From update of attachment 47298 [details]) > > + Limit the number of iterations of the main loop to 5. > <...> > > + while (!finished && (iterationCount++ < cResidualStyleIterationLimititerationLimit)) { > > I think this actually limits the count to 6, not 5. I think it's 5. The value being compared on iterations through the loop will be: 0 1 2 3 4 5 ==> this breaks out of the loop before it executes. > > It would be nice to attach a test case to this Bugzilla bug, so that people > could verify that it didn't regress - if it's not possible to make an automated > test, of course. I'll see if I can make an automated test that hangs on release but reliably runs fast in debug. Regards, Maciej
Alexey Proskuryakov
Comment 10 2010-01-24 18:32:46 PST
D'oh!
Maciej Stachowiak
Comment 11 2010-01-24 20:11:54 PST
Committed r53790: <http://trac.webkit.org/changeset/53790>
Note You need to log in before you can comment on or make changes to this bug.