Bug 133876

Summary: REGRESSION(r169703): Memory usage (JSHeap) on all performance tests got ~50% worse
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: ap, barraclough, dbates, ggaren, jer.noble, jonlee, kling, mhahnenberg, oliver, rniwa, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: https://perf.webkit.org/#mode=charts&chartList=[[%22efl%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22gtk%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22mac-lion%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22mac-mavericks%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22mac-mountainlion%22%2C%22Animation%2Fballs%3AJSHeap%22]]

Description Carlos Alberto Lopez Perez 2014-06-13 13:07:02 PDT
Some of the revisions between [r169699-169710] made a significant performance regression on Animation/balls:JSHeap

Mac-MountainLion: 33.98% worse
Mac-Mavericks:    14.18% worse
EFL:               9.52% worse
GTK:              56.66% worse



WebKit: http://trac.webkit.org/log/?rev=169710&stop_rev=169699&verbose=on


The graphs are here: https://perf.webkit.org/#mode=charts&chartList=[[%22efl%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22gtk%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22mac-lion%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22mac-mavericks%22%2C%22Animation%2Fballs%3AJSHeap%22]%2C[%22mac-mountainlion%22%2C%22Animation%2Fballs%3AJSHeap%22]]
Comment 3 Carlos Alberto Lopez Perez 2014-06-18 03:48:17 PDT
I have bisected the interval and got r169703 as the revision that caused the regression.

You can check here the bisection I did: http://people.igalia.com/clopez/wk133876/PerformanceTestsResults.html
Click on memory at the top left.

I only tested with one Performance test (Bindings/document-implementation.html) to do it faster, but all the other tests would give the same result related to JSHeap memory usage according to perf.webkit.org
Comment 4 Geoffrey Garen 2014-06-18 10:53:21 PDT
It looks like the absolute size of the regression is 60kB - 100kB. Is that right? If so, I think those high percentages just indicate that the test doesn't use much JS memory -- not that something major has changed.

In a test that does use a lot of JS memory, I would expect the memory impact of <http://trac.webkit.org/changeset/169703> to be much lower, percentage-wise, since one prototype is typically used by many client objects, making prototype memory a small fraction of heap memory.

I think we can accept this delta.
Comment 5 Andreas Kling 2014-06-18 10:55:53 PDT
(In reply to comment #4)
> It looks like the absolute size of the regression is 60kB - 100kB. Is that right? If so, I think those high percentages just indicate that the test doesn't use much JS memory -- not that something major has changed.
> 
> In a test that does use a lot of JS memory, I would expect the memory impact of <http://trac.webkit.org/changeset/169703> to be much lower, percentage-wise, since one prototype is typically used by many client objects, making prototype memory a small fraction of heap memory.
> 
> I think we can accept this delta.

Indeed. There is clearly a space/time tradeoff here, and I think 100kB is well within the acceptable range.