75050 2011-12-21 15:50:09 -0800 REGRESSION(r103349): 7.5% peak memory use increase on Windows 2012-01-03 05:21:01 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P1 Normal --- 1 tony reed caryclark enne epoger gbillock ggaren loislo reed rniwa sail sam oldest_to_newest 525751 0 tony 2011-12-21 15:50:09 -0800 http://build.chromium.org/f/chromium/perf/vista-release-webkit-latest/moz/report.html?history=150&rev=115372&graph=vm_peak_r Looks like the regression range is http://trac.webkit.org/log/trunk/Source?rev=103365&stop_rev=103354&verbose=on . None of the changes seems to obviously be the cause. I'm OOO right now, but thought I should file a bug so someone can take a look. 525855 1 rniwa 2011-12-21 17:13:49 -0800 Suspicious revisions: Merge ScrollAnimatorChromiumMac.mm back to ScrollAnimatorMac http://trac.webkit.org/changeset/103354 Tightened up Vector<T>::append http://trac.webkit.org/changeset/103356 [Coverity] Fix leak in V8HTMLDocument http://trac.webkit.org/changeset/103358 Change adoptPtr(new ...) to ...::create in Page.cpp http://trac.webkit.org/changeset/103365 525871 2 rniwa 2011-12-21 17:25:39 -0800 Looking at Chromium changes, WebKit roll r115310 (Chromium revision) definitely regressed the memory usage on Windows bot: http://build.chromium.org/f/chromium/perf/vista-release-dual-core/moz/report.html?history=150&rev=-1&graph=vm_peak_r http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk/src&range=115306:115310 525872 3 rniwa 2011-12-21 17:26:06 -0800 All regressions are P1. 525891 4 rniwa 2011-12-21 17:50:04 -0800 Apparently http://trac.webkit.org/changeset/103354 is specific to Mac so we can cross that out. It seems very unlikely that either http://trac.webkit.org/changeset/103358 or http://trac.webkit.org/changeset/103365 can cause Windows-specific memory regression. Given that I think http://trac.webkit.org/changeset/103356/trunk/Source/JavaScriptCore/wtf/Vector.h is the culprit. I don't see why it can cause memory use increase unless the compiler is generating bad code but then I wouldn't be surprised if that were the case since the comment being removed says it used to cause a compilation error. 526196 5 loislo 2011-12-22 05:08:22 -0800 I confirm that the problem is in webkit. I'm bisecting it. Looks like it is somewhere between 103344 and 103355 The suspicious revision is 103353 sizeof(RenderStyle) is 64 instead of 56 on Windows (x86) https://bugs.webkit.org/show_bug.cgi?id=74876 Reviewed by Ryosuke Niwa. Move bit fields into a new class and use unsigned for all types so we align at 4 byte bounds. Also move the initializers into the header file (has the side benefit of not needing to duplicate the initializers in 3 places). Enable the compile assert on Windows. 526228 6 loislo 2011-12-22 06:51:32 -0800 (In reply to comment #5) > I confirm that the problem is in webkit. > I'm bisecting it. Looks like it is somewhere between 103344 and 103355 > > The suspicious revision is 103353 > > sizeof(RenderStyle) is 64 instead of 56 on Windows (x86) > https://bugs.webkit.org/show_bug.cgi?id=74876 > Reviewed by Ryosuke Niwa. > Move bit fields into a new class and use unsigned for all types so we > align at 4 byte bounds. Also move the initializers into the header > file (has the side benefit of not needing to duplicate the initializers > in 3 places). > Enable the compile assert on Windows. Actually it is 103349. enable USE_SKIA_TEXT by default, replacing GDI for all text drawing Revert it? 526241 7 reed 2011-12-22 07:19:52 -0800 I expect this is related to the large font-cache setting we have (in skia/skia.gyp). This gives us a big perf win on page-cyclers (faster than GDI) but will take more ram (which can be purged). 526242 8 reed 2011-12-22 07:21:22 -0800 The max cache size can be reduce (either in the gyp, or via a runtime API), or we can live with a larger peak, since that is the tradeoff for faster performance. I can go either way. 526265 9 epoger 2011-12-22 08:22:11 -0800 Here are more graphs, showing the simultaneous performance improvement and peak-memory-consumption regression as of Chromium r115310 (Webkit DEPS roll): http://build.chromium.org/f/chromium/perf/vista-release-dual-core/intl1/report.html?history=150&rev=115500&graph=times http://build.chromium.org/f/chromium/perf/vista-release-dual-core/intl1/report.html?history=150&rev=115500&graph=vm_peak_r 526270 10 epoger 2011-12-22 08:35:00 -0800 Are there any bots that went red as a result of this peak-memory-consumption regression? I don't see any on the Chromium side... 526274 11 reed 2011-12-22 08:40:45 -0800 We will locally reproduce these page cyclers, and play with the font-cache size to mitigate the ram usage -vs- performance, but for the moment I recommend the code stays as is. - faster text drawing needs ram unfortunately - we will continue to improve our balance-point for ram-vs-speed as we gather in-the-field numbers of real font usage (not just for these cyclers) - we will also implement font-cache-shrinking for tabs that go into the background 527404 12 tony 2011-12-27 10:25:05 -0800 Since this is an intentional trade off, seems like we can close this bug if we file bugs for the two follow up action items mentioned in comment #11. 528735 13 reed 2012-01-03 05:21:01 -0800 http://code.google.com/p/chromium/issues/detail?id=108979 http://code.google.com/p/chromium/issues/detail?id=108980 Closing, to track the follow-on initiatives via the above bugs.