Bug 78984 - Performance tests should measure memory usage
Summary: Performance tests should measure memory usage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 90080 90858 91204 91274 93958 98956 99288 99899 100029
Blocks: 77037
  Show dependency treegraph
 
Reported: 2012-02-18 17:43 PST by Ryosuke Niwa
Modified: 2013-01-24 20:08 PST (History)
16 users (show)

See Also:


Attachments
Patch (3.57 KB, patch)
2012-07-12 10:13 PDT, Olivier Blin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-08 (305.87 KB, application/zip)
2012-07-12 11:02 PDT, WebKit Review Bot
no flags Details
Patch (3.53 KB, patch)
2012-07-13 01:37 PDT, Olivier Blin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-02-18 17:43:43 PST
Right now, performance tests only measure time performance. We also need to measure memory performance as well.
Comment 1 Zoltan Horvath 2012-06-22 01:19:23 PDT
Have we decided already what and how to measure?
On webkit.sed.hu we used to measure maximum resident set size of JSC benchmarks.

We have to decide either to use kernel level data or some WebKit internal measure system. The prior is already given for us, since the different systems support memory consumption monitoring somehow. 

* What to measure? (assume we use kernel level information)
 - maximum RSS [maxRSS](peak)
 - average memory consumption during the run [avgRSS]

* How to measure? (external tools case)
 - Linux: ps, /proc/*/smaps,stat,statm (polling) we can compute avgRSS, maxRSS
 - MAC: ps, sysctl, time we can compute avgRSS, maxRSS
 - Win: psapi, we can compute avgRSS, maxRSS
Comment 2 Ryosuke Niwa 2012-06-22 09:13:27 PDT
The last time I had talked with Antti, we decided that FastMalloc's statistics will be useful but I'm open to other options.
Comment 3 Zoltan Horvath 2012-06-23 04:15:01 PDT
FastMalloc statistics is suitable to measure the usage of the heap. Is it enough for us? Ofcourse, beyond question FastMallocStatistics is available on every platform.
Comment 4 Hajime Morrita 2012-06-24 18:09:37 PDT
(In reply to comment #3)
> FastMalloc statistics is suitable to measure the usage of the heap. Is it enough for us? Ofcourse, beyond question FastMallocStatistics is available on every platform.

Some system-originated objects won't able to be tracked, off-screen images, for example, isn't under FastMalloc's supervision. System resources like window objects are also out of the sight.

System stats like /proc on the other hand lacks some application specific 
details like number of nodes. So we can attack from the both side.
How about to use this bug as a tracking bug and 
have sub-bugs for each approach?
Comment 5 Zoltan Horvath 2012-06-25 04:48:50 PDT
(In reply to comment #4)
> System stats like /proc on the other hand lacks some application specific 
> details like number of nodes. So we can attack from the both side.
> How about to use this bug as a tracking bug and 
> have sub-bugs for each approach?

Right! It's okay for me.

I'm going to investigate on the topic which numbers can be the most useful ones.
Comment 6 Zoltan Horvath 2012-06-26 00:28:20 PDT
(In reply to comment #4)

> System stats like /proc on the other hand lacks some application specific 
> details like number of nodes. 

What do you mean on number of nodes?

struct FastMallocStatistics {
    size_t reservedVMBytes;
    size_t committedVMBytes;
    size_t freeListBytes;
};

FastMallocStatistics can provide only these numbers. Ideally, using smaps would be one good option (because in that case we can split the RSS into library usage), but in that case we would need /proc available on mac (it's possible) and on win (not possible) also.
Comment 7 Ryosuke Niwa 2012-06-26 00:36:30 PDT
(In reply to comment #6)
> (In reply to comment #4)
> 
> > System stats like /proc on the other hand lacks some application specific 
> > details like number of nodes. 
> 
> What do you mean on number of nodes?

Inspector keeps track of that information and it's exposed via internals.
Comment 8 Ryosuke Niwa 2012-06-27 12:47:53 PDT
btw, inspector folks added some wrapper memory instrumentation code: http://trac.webkit.org/changeset/121022
Comment 9 Olivier Blin 2012-07-12 09:11:25 PDT
There is also a console.memory object that already provides JS heap statistics.
I have local patches adding FastMalloc stats (reservedFastMalloc, committedFastMalloc, freeListFastMalloc) in this object.
Is this something that could be merged?
Comment 10 Ryosuke Niwa 2012-07-12 09:53:29 PDT
(In reply to comment #9)
> There is also a console.memory object that already provides JS heap statistics.
> I have local patches adding FastMalloc stats (reservedFastMalloc, committedFastMalloc, freeListFastMalloc) in this object.
> Is this something that could be merged?

That sounds very useful. Ideally though, we add those methods on V8 as well so that we don't have to have two different tests for JSC and V8.
Comment 11 Olivier Blin 2012-07-12 10:13:29 PDT
Created attachment 151984 [details]
Patch
Comment 12 WebKit Review Bot 2012-07-12 10:17:10 PDT
Attachment 151984 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:10:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 WebKit Review Bot 2012-07-12 11:02:22 PDT
Comment on attachment 151984 [details]
Patch

Attachment 151984 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13203797

New failing tests:
fast/dom/Window/window-properties-performance.html
Comment 14 WebKit Review Bot 2012-07-12 11:02:27 PDT
Created attachment 152003 [details]
Archive of layout-test-results from gce-cr-linux-08

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 15 Zoltan Horvath 2012-07-12 14:09:33 PDT
Nice! This will be the easiest way to measure the memory of the perftests. For pageloadtests we will need to be a little tricky and return these values through DRT/WRT. Please fix the style issues then we can check it in, then I'll handle the script side of it.
Comment 16 Olivier Blin 2012-07-13 01:37:29 PDT
Created attachment 152187 [details]
Patch
Comment 17 Zoltan Horvath 2012-07-13 01:45:17 PDT
(In reply to comment #16)
> Created an attachment (id=152187) [details]
> Patch

The patch is okay, but the current title is a bit misleading, since it only adds fastmallocstatistics to console.memory. 

It would be better to open a new bug and make this depends on it. The new bug's title would be the description (Add FastMalloc statistics in console.memory). 
...we can leave this bug as a master tracker for the memory consumption measurement, since we need additional changes on the tests-system and the tests side as well.

Sorry to didn't write this earlier!
Comment 18 Olivier Blin 2012-07-13 01:46:25 PDT
Indeed, that's better.
I'll open a new bug for that.
Comment 19 Zoltan Horvath 2012-07-13 02:30:48 PDT
Comment on attachment 152187 [details]
Patch

Remove r? and make it obsolete since Olivier opened a new bug for the patch, bug #91204.
Comment 20 Ryosuke Niwa 2012-12-05 00:41:18 PST
We're already recording Malloc & JSHeap data.