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+

Description Maciej Stachowiak 2010-01-24 11:40:43 PST
Content with heavily nested residual style is so slow, it seems like a hang
Comment 1 Maciej Stachowiak 2010-01-24 11:47:53 PST
Created attachment 47298 [details]
Patch
Comment 2 Maciej Stachowiak 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.
Comment 3 Eric Seidel (no email) 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
Comment 4 WebKit Review Bot 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
Comment 5 mitz 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
Comment 6 WebKit Review Bot 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
Comment 7 Alexey Proskuryakov 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.
Comment 8 WebKit Review Bot 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
Comment 9 Maciej Stachowiak 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
Comment 10 Alexey Proskuryakov 2010-01-24 18:32:46 PST
D'oh!
Comment 11 Maciej Stachowiak 2010-01-24 20:11:54 PST
Committed r53790: <http://trac.webkit.org/changeset/53790>