Bug 110872

Summary: REGRESSION(r144019): 8% perf regression in chromium-win7 on intl1 page cycler
Product: WebKit Reporter: Glenn Adams <glenn>
Component: Layout and RenderingAssignee: Glenn Adams <glenn>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, jamesr, ojan, simonjam, tmpsantos, tonyg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Windows 7   
Bug Depends on:    
Bug Blocks: 89235    

Description Glenn Adams 2013-02-26 05:09:52 PST
this appears to be limited to windows platform only; please do not rollout r144019 unless absolutely necessary, as i'd like to have a little time to fix it with a follow-on patch that addresses this win specific problem
Comment 2 Glenn Adams 2013-02-26 05:14:36 PST
note that there is no corresponding memory regression:

http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/intl1/report.html?history=40&rev=184623&graph=commit_charge
Comment 3 James Robinson 2013-02-26 09:18:50 PST
(In reply to comment #0)
> this appears to be limited to windows platform only; please do not rollout r144019 unless absolutely necessary, as i'd like to have a little time to fix it with a follow-on patch that addresses this win specific problem

In Chromium we have a strict no regressions policy for all performance issues.  Leaving regressions in the tree masks other problems.
Comment 4 Tony Gentilcore 2013-02-26 09:31:27 PST
(In reply to comment #3)
> (In reply to comment #0)
> > this appears to be limited to windows platform only; please do not rollout r144019 unless absolutely necessary, as i'd like to have a little time to fix it with a follow-on patch that addresses this win specific problem
> 
> In Chromium we have a strict no regressions policy for all performance issues.  Leaving regressions in the tree masks other problems.

+1

If we are reasonably confident that this is the source of the regression, we should roll out and fix off the tree.
Comment 5 Glenn Adams 2013-02-26 09:37:20 PST
-1

how is this different than, say, turning qt-5.0-wk2 red? does doing so require roll out? where are the chromium intl2 page cycler tests? how are WK devs supposed to test/fix this specific chromium-win bug, even if they have win build support?
Comment 6 James Robinson 2013-02-26 10:38:56 PST
The page cycler data itself can't be shared due to copyright, but there's nothing particularly special about it - it's just a snapshot of a number of pages with lots of different types of complex text.  Try running on pages like bn.wikipedia.org, ml.wikipedia.org, msn.co.il and see if you can reproduce the issue there.  With a regression this large it's unlikely to be due to just one site.
Comment 7 James Robinson 2013-02-26 10:40:10 PST
(In reply to comment #6)
> The page cycler data itself can't be shared due to copyright, but there's nothing particularly special about it - it's just a snapshot of a number of pages with lots of different types of complex text.  Try running on pages like bn.wikipedia.org, ml.wikipedia.org, msn.co.il

Sorry - these are from our intl2 set (but maybe they'll be useful anyway).  The wikipedia homepages in various languages are good general purpose tests for different scripts.
Comment 8 Ojan Vafai 2013-02-26 10:40:53 PST
(In reply to comment #5)
> how is this different than, say, turning qt-5.0-wk2 red? does doing so require roll out? where are the chromium intl2 page cycler tests? how are WK devs supposed to test/fix this specific chromium-win bug, even if they have win build support?

It has nothing to do with Chromium vs. other ports. It's about performance tests versus other types of failures. If a layout test fails and is fixed later (even if it's fixed weeks later), it's easy to see that the final fix for it completely fixed the regression. With performance tests it's considerably harder given all the patches that go in that change performance on the same bot and the difficulty of tracking performance over long periods of time.

This has been a semi-official WebKit policy for a long time and we rarely make exceptions. I don't see why this would be an exceptional case.

My suggestion is to rollout and recommit this code behind a #define. That way, you can keep from having a ton of code churn from the rollouts and difficulty getting reviews while you try to resolve the performance problem.
Comment 9 Glenn Adams 2013-02-26 15:25:12 PST
rolled out r144019 in <http://trac.webkit.org/changeset/144073>
Comment 10 Thiago Marcos P. Santos 2013-02-27 05:37:40 PST
(In reply to comment #0)
> this appears to be limited to windows platform only; please do not rollout r144019 unless absolutely necessary, as i'd like to have a little time to fix it with a follow-on patch that addresses this win specific problem

The performance regression was very visible on the Linux bots.