Bug 141247 – REGRESSION (bmalloc): WebKit performance tests don't report memory stats

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

Our performance tests, use the JS call window.internals.mallocStatistics() to get information about the internal malloc stats on WebKit.

This information is collected by the tool running the performance tests and displayed on the graphs of http://perf.webkit.org

This feature was added on r123773 <http://trac.webkit.org/r123773> (Check bug 78984)

On bug 139812 I wondered why the Mac performance bots don't report any information about the malloc stats.

It turns out the cause is that bmalloc still don't implements this.
I just tested bmalloc on Linux (bug 140162) and I also get 0 bytes reported on the malloc stats of any performance test.

For example, on the link below you can see how the mac bot stopped reporting any information about malloc stats once bmalloc was made the default allocator.
Linux bots (GTK/EFL) still report this information because they are still using TCMalloc.
https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22efl%22%2C%22Parser%2Fhtml5-full-render%3AMalloc%22%5D%2C%5B%22gtk%22%2C%22Parser%2Fhtml5-full-render%3AMalloc%22%5D%2C%5B%22mac-mavericks%22%2C%22Parser%2Fhtml5-full-render%3AMalloc%22%5D%2C%5B%22mac-yosemite%22%2C%22Parser%2Fhtml5-full-render%3AMalloc%22%5D%5D&days=351

I think this is a regression, since now we lack of proper information about the memory usage/consumption and we can't identify revisions that cause huge increases on the memory usage. 

The (stub) implementation for bmalloc seems to be there: http://trac.webkit.org/browser/trunk/Source/WTF/wtf/FastMalloc.cpp?rev=179600#L339
The one for tcmalloc that was working previously: http://trac.webkit.org/browser/trunk/Source/WTF/wtf/FastMalloc.cpp?rev=179600#L4552

I'm not planning to implement this, so feel free to take it if you want.

[Radar WebKit Bug Importer]

<rdar://problem/19726151>

[Geoffrey Garen]

It's not clear if we will ever get back to the precise reporting TCMalloc had; some of that reporting relied on expensive metadata that cause throughput and/or memory use problems.

For example, TCMalloc could only do accurate reporting of committed vs uncommitted memory because it would synchronously over-commit huge swaths of memory, and synchronously commit or decommit when merging ranges, or refuse to merge ranges.

[Csaba Osztrogonác]

(In reply to comment #2)
> It's not clear if we will ever get back to the precise reporting TCMalloc
> had; some of that reporting relied on expensive metadata that cause
> throughput and/or memory use problems.
> 
> For example, TCMalloc could only do accurate reporting of committed vs
> uncommitted memory because it would synchronously over-commit huge swaths of
> memory, and synchronously commit or decommit when merging ranges, or refuse
> to merge ranges.

If it isn't possible to get back memory statistics, how can we avoid serious
memory regressions in the future? How do you measure memory consumption on OSX
since switching to bmalloc? Are you going to upstream your method to be able
run memory measurements on the performance bots again?

[Csaba Osztrogonác]

I found bug136592 now, so it is a known issue long time ago.

[Carlos Alberto Lopez Perez]

As a workaround (until a better solution is found) I'm wondering if on Linux we could just use the information from the Kernel regarding memory usage. It should be easy to get both the resident and virtual memory usage from /proc/$pid/stat (rss and vsize fields) and report it.

WDYT?

[Sam Weinig]

(In reply to comment #3)
> (In reply to comment #2)
> > It's not clear if we will ever get back to the precise reporting TCMalloc
> > had; some of that reporting relied on expensive metadata that cause
> > throughput and/or memory use problems.
> > 
> > For example, TCMalloc could only do accurate reporting of committed vs
> > uncommitted memory because it would synchronously over-commit huge swaths of
> > memory, and synchronously commit or decommit when merging ranges, or refuse
> > to merge ranges.
> 
> If it isn't possible to get back memory statistics, how can we avoid serious
> memory regressions in the future? How do you measure memory consumption on
> OSX
> since switching to bmalloc? Are you going to upstream your method to be able
> run memory measurements on the performance bots again?

We have never really relied on malloc statistics for our memory testing.  On OS X we use the `footprint` tool, though in the past, we have used tools like `heap` and `vmmap`.  These tools give better indications of memory use, as they include all of the allocators on the system.

[Carlos Alberto Lopez Perez]

Created attachment 256108 [details]
Patch

[Darin Adler]

Geoff, are you going to review this?

[Carlos Alberto Lopez Perez]

BTW, I have checked the output of the win EWS bot and the failure it gives looks totally unrelated with this bug, also it seems that it has built fine Source/WTF/wtf/FastMalloc.cpp that is the only file that this patch touches.


  python: can't open file '/home/buildbot/WebKit/WebKitBuild/Release/include/private/JavaScriptCore/cssmin.py': [Errno 2] No such file or directory
  make: *** No rule to make target '/home/buildbot/WebKit/WebKitBuild/Release/include/private/JavaScriptCore/JSInputs.json', needed by 'WebReplayInputs.h'.  Stop.
  make: *** Waiting for unfinished jobs....
  /home/buildbot/WebKit/Source/WebCore/DerivedSources.make:893: recipe for target 'XMLViewerCSS.h' failed
  make: *** [XMLViewerCSS.h] Error 2


So I deduce that the win EWS is in bad status, checking https://webkit-queues.appspot.com/queue-status/win-ews seems to confirm that as is failing for every patch or not being able to build without patch.

[Csaba Osztrogonác]

(In reply to comment #9)
> BTW, I have checked the output of the win EWS bot and the failure it gives
> looks totally unrelated with this bug, also it seems that it has built fine
> Source/WTF/wtf/FastMalloc.cpp that is the only file that this patch touches.
> 
> 
>   python: can't open file
> '/home/buildbot/WebKit/WebKitBuild/Release/include/private/JavaScriptCore/
> cssmin.py': [Errno 2] No such file or directory
>   make: *** No rule to make target
> '/home/buildbot/WebKit/WebKitBuild/Release/include/private/JavaScriptCore/
> JSInputs.json', needed by 'WebReplayInputs.h'.  Stop.
>   make: *** Waiting for unfinished jobs....
>   /home/buildbot/WebKit/Source/WebCore/DerivedSources.make:893: recipe for
> target 'XMLViewerCSS.h' failed
>   make: *** [XMLViewerCSS.h] Error 2
> 
> 
> So I deduce that the win EWS is in bad status, checking
> https://webkit-queues.appspot.com/queue-status/win-ews seems to confirm that
> as is failing for every patch or not being able to build without patch.

Clean build is broken on Windows, it isn't an EWS issue. I already reported it
in bug146628 and pinged Apple Windows port maintainers too in email. They know
the problem, maybe they can fix it next week.

[Zoltan Horvath]

By using TCmalloc's statistics, we intended to log memory allocated by new. Since there is no immediate solution to get stats from bmalloc, I think it's fine to save the max rss for mac. I'm not sure linux kernel will provide the same result for maxrss though.

[Carlos Alberto Lopez Perez]

(In reply to comment #11)
> By using TCmalloc's statistics, we intended to log memory allocated by new.
> Since there is no immediate solution to get stats from bmalloc, I think it's
> fine to save the max rss for mac. I'm not sure linux kernel will provide the
> same result for maxrss though.

What are your concerns regarding the Linux kernel? On the tests I did, the results I got with this patch where the ones I was expecting.

[Zoltan Horvath]

(In reply to comment #12)
> (In reply to comment #11)
> > By using TCmalloc's statistics, we intended to log memory allocated by new.
> > Since there is no immediate solution to get stats from bmalloc, I think it's
> > fine to save the max rss for mac. I'm not sure linux kernel will provide the
> > same result for maxrss though.
> 
> What are your concerns regarding the Linux kernel? On the tests I did, the
> results I got with this patch where the ones I was expecting.

Good to hear that! Just wanted to be sure it provides the numbers that you want to see.

[Carlos Alberto Lopez Perez]

(In reply to comment #8)
> Geoff, are you going to review this?

Not sure about Geoff... but is there any other reviewer willing to review this patch? It has been waiting here for review for almost 3 weeks already.

I think that this patch is easy to review and brings back a missing functionality (since bmalloc) that is very useful (at least for me) to track down regressions in memory usage at http://perf.webkit.org

Thanks.

[Geoffrey Garen]

Comment on attachment 256108 [details]
Patch

These days, bmalloc has the metadata you need to produce numbers for reservedVMBytes and committedVMBytes.

It's not so great to report overall memory usage like this, since that counts non-malloc memory.

Still, I guess it's not actively harmful, and it will get *some* number back up on the bots, so I'll say r+.

[Carlos Alberto Lopez Perez]

(In reply to comment #15)
> Comment on attachment 256108 [details]
> Patch
> 
> These days, bmalloc has the metadata you need to produce numbers for
> reservedVMBytes and committedVMBytes.
> 
> It's not so great to report overall memory usage like this, since that
> counts non-malloc memory.
> 
> Still, I guess it's not actively harmful, and it will get *some* number back
> up on the bots, so I'll say r+.

I agree with you that there should be a better way to report the memory usage than this, but in the meanwhile at least this get us some stats back.

Thanks for the r+ :)

[Carlos Alberto Lopez Perez]

Comment on attachment 256108 [details]
Patch

Clearing flags on attachment: 256108

Committed r187389: <http://trac.webkit.org/changeset/187389>

[Carlos Alberto Lopez Perez]

All reviewed patches have been landed.  Closing bug.