RESOLVED FIXED 98625
http://www.robohornet.org/tests/svgresize.html spends all its time in StyleResolver::sweepMatchedPropertiesCache 
https://bugs.webkit.org/show_bug.cgi?id=98625
Summary http://www.robohornet.org/tests/svgresize.html spends all its time in StyleRe...
Eric Seidel (no email)
Reported 2012-10-07 15:19:00 PDT
http://www.robohornet.org/tests/svgresize.html spends all (90%) of its time in StyleResolver::sweepMatchedPropertiesCache It must be hitting some pathological case?
Attachments
GC the matched properties cache on a timer instead of every 100 additions. (4.58 KB, patch)
2012-10-15 11:33 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2012-10-07 15:21:15 PDT
Does the cache grow without bound?
Antti Koivisto
Comment 2 2012-10-10 04:32:05 PDT
Maybe the sweep needs be done by a short timer instead of being synchronous.
Andreas Kling
Comment 3 2012-10-15 11:33:58 PDT
Created attachment 168748 [details] GC the matched properties cache on a timer instead of every 100 additions.
Eric Seidel (no email)
Comment 4 2012-10-15 11:43:59 PDT
Comment on attachment 168748 [details] GC the matched properties cache on a timer instead of every 100 additions. Should we be capping the size of this cache?
Eric Seidel (no email)
Comment 5 2012-10-15 11:46:53 PDT
Comment on attachment 168748 [details] GC the matched properties cache on a timer instead of every 100 additions. I now see that I asked the same question earlier in this bug. :) Anyway, the change itself looks fine. I still have some mild concern about the seemingly unbounded nature of this cache. :)
Eric Seidel (no email)
Comment 6 2012-10-15 11:47:40 PDT
Perhaps we should add svgresize or similar to PerformanceTests to catch this?
Andreas Kling
Comment 7 2012-10-15 11:50:07 PDT
(In reply to comment #4) > (From update of attachment 168748 [details]) > Should we be capping the size of this cache? That sounds like the sane thing to do. Thoughts, Antti? (In reply to comment #6) > Perhaps we should add svgresize or similar to PerformanceTests to catch this? Could we import the test suite wholesale? If so, that would be the best thing to do. It might have a negative impact on cycle times (though that'll certainly motivate us to make the tests run faster!)
Eric Seidel (no email)
Comment 8 2012-10-15 11:51:50 PDT
(In reply to comment #7) > (In reply to comment #6) > > Perhaps we should add svgresize or similar to PerformanceTests to catch this? > > Could we import the test suite wholesale? If so, that would be the best thing to do. It might have a negative impact on cycle times (though that'll certainly motivate us to make the tests run faster!) Bug 97507 is about just that. I think the current prevailing view is that the suite is in-flux and thus not worth importing. shrug.
WebKit Review Bot
Comment 9 2012-10-15 17:36:41 PDT
Comment on attachment 168748 [details] GC the matched properties cache on a timer instead of every 100 additions. Clearing flags on attachment: 168748 Committed r131388: <http://trac.webkit.org/changeset/131388>
WebKit Review Bot
Comment 10 2012-10-15 17:36:45 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 11 2012-10-16 17:05:29 PDT
Good times. :) Now the test spends 50% of time in StyleResolver::canShareStyleWithElement (which has shown up hot in other samples before).
Note You need to log in before you can comment on or make changes to this bug.