141247 2015-02-04 07:35:27 -0800 REGRESSION (bmalloc): WebKit performance tests don't report memory stats 2015-07-25 07:47:57 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P1 Normal --- 141459 1 clopez clopez benjamin cdumez cgarcia cmarcelo commit-queue darin dbates ggaren gyuyoung.kim kling kvserr mitica msaboff olivier.blin ossy rniwa sam webkit-bug-importer zan zoltan oldest_to_newest 1066798 0 clopez 2015-02-04 07:35:27 -0800 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. 1067028 1 webkit-bug-importer 2015-02-04 20:22:30 -0800 <rdar://problem/19726151> 1068786 2 ggaren 2015-02-12 16:22:36 -0800 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. 1068890 3 ossy 2015-02-13 05:06:41 -0800 (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? 1068898 4 ossy 2015-02-13 06:19:39 -0800 I found bug136592 now, so it is a known issue long time ago. 1068923 5 clopez 2015-02-13 08:43:05 -0800 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? 1068928 6 sam 2015-02-13 09:30:01 -0800 (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. 1106729 7 256108 clopez 2015-07-03 10:28:05 -0700 Created attachment 256108 Patch 1107844 8 darin 2015-07-08 12:35:23 -0700 Geoff, are you going to review this? 1108204 9 clopez 2015-07-09 08:53:53 -0700 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. 1108206 10 ossy 2015-07-09 09:13:07 -0700 (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. 1109487 11 zoltan 2015-07-14 15:15:26 -0700 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. 1109848 12 clopez 2015-07-16 03:06:31 -0700 (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. 1110185 13 zoltan 2015-07-17 13:18:45 -0700 (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. 1111103 14 clopez 2015-07-21 16:13:45 -0700 (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. 1112123 15 256108 ggaren 2015-07-24 15:45:39 -0700 Comment on attachment 256108 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+. 1112212 16 clopez 2015-07-25 07:46:48 -0700 (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+ :) 1112213 17 256108 clopez 2015-07-25 07:47:45 -0700 Comment on attachment 256108 Patch Clearing flags on attachment: 256108 Committed r187389: <http://trac.webkit.org/changeset/187389> 1112214 18 clopez 2015-07-25 07:47:57 -0700 All reviewed patches have been landed. Closing bug. 256108 2015-07-03 10:28:05 -0700 2015-07-25 07:47:45 -0700 Patch bug-141247-20150703192710.patch text/plain 2453 clopez U3VidmVyc2lvbiBSZXZpc2lvbjogMTg2MjI0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDRiZjVjZjdlOGIyYmJlNzVlYzFjM2Uy NzUwZWY4Nzg2MzczYTRkZDEuLmQxY2E5OGZlYThmYjUzY2FhNzAxZDhmOTRhNzRiMTIwNGU0OTY4 YTMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjEgQEAKKzIwMTUtMDctMDMgIENhcmxvcyBBbGJlcnRvIExvcGV6 IFBlcmV6ICA8Y2xvcGV6QGlnYWxpYS5jb20+CisKKyAgICAgICAgUkVHUkVTU0lPTiAoYm1hbGxv Yyk6IFdlYktpdCBwZXJmb3JtYW5jZSB0ZXN0cyBkb24ndCByZXBvcnQgbWVtb3J5IHN0YXRzLgor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQxMjQ3CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTWVhbndoaWxl IGEgYmV0dGVyIHdheSBvZiBnZXR0aW5nIG1lbW9yeSBzdGF0cyB3aXRoIGJtYWxsb2MgaXMgbm90 IGZvdW5kCisgICAgICAgIChzZWUgYnVnIDEzNjU5MiksIHdlIGNhbiByZXBvcnQgYXMgbWVtb3J5 IHN0YXRzIHRoZSByZXNpZGVudCBzZXQgc2l6ZQorICAgICAgICBpbmZvcm1hdGlvbiB0aGF0IHRo ZSBvcGVyYXRpbmcgc3lzdGVtIHByb3ZpZGVzIHRvIHVzLgorCisgICAgICAgIFRoaXMgYXQgbGVh c3Qgc2hvdWxkIGJlIGdvb2QgZW5vdWdoIHRvIGdldCBiYWNrIHRoZSBtZW1vcnkgc3RhdHMgb24g dGhlCisgICAgICAgIHBlcmZvcm1hbmNlIHRlc3RzIGFuZCBiZWluZyBhYmxlIHRvIHRyYWNrIGRv d24gbWVtb3J5IHVzYWdlIHJlZ3Jlc3Npb25zCisgICAgICAgIGF0IGh0dHBzOi8vcGVyZi53ZWJr aXQub3JnCisKKyAgICAgICAgKiB3dGYvRmFzdE1hbGxvYy5jcHA6CisgICAgICAgIChXVEY6OmZh c3RNYWxsb2NTdGF0aXN0aWNzKTogUmVwb3J0IG1heHJzcyBkYXRhIGFzIGNvbW1pdHRlZFZNQnl0 ZXMuCisKIDIwMTUtMDctMDEgIEFsZXggQ2hyaXN0ZW5zZW4gIDxhY2hyaXN0ZW5zZW5Ad2Via2l0 Lm9yZz4KIAogICAgICAgICBSZS1lbmFibGUgV2ViR0wgb24gV2luQ2Fpcm8KZGlmZiAtLWdpdCBh L1NvdXJjZS9XVEYvd3RmL0Zhc3RNYWxsb2MuY3BwIGIvU291cmNlL1dURi93dGYvRmFzdE1hbGxv Yy5jcHAKaW5kZXggZTU1MjZlODExNTQ5MjkxOGZlYWJiMzJjZDk1MDFlMTU3MGNkNzFiYy4uYzRj MWQxOGM0YjA2ODIwYzUxMWQ4ZTBiNDc2MDUyM2RmZjU3MzE0NiAxMDA2NDQKLS0tIGEvU291cmNl L1dURi93dGYvRmFzdE1hbGxvYy5jcHAKKysrIGIvU291cmNlL1dURi93dGYvRmFzdE1hbGxvYy5j cHAKQEAgLTM3LDYgKzM3LDcgQEAKICNpbmNsdWRlIDx3aW5kb3dzLmg+CiAjZWxzZQogI2luY2x1 ZGUgPHB0aHJlYWQuaD4KKyNpbmNsdWRlIDxzeXMvcmVzb3VyY2UuaD4KICNlbmRpZgogCiAjaWYg T1MoREFSV0lOKQpAQCAtMjYxLDggKzI2MiwyNyBAQCB2b2lkIHJlbGVhc2VGYXN0TWFsbG9jRnJl ZU1lbW9yeSgpCiAKIEZhc3RNYWxsb2NTdGF0aXN0aWNzIGZhc3RNYWxsb2NTdGF0aXN0aWNzKCkK IHsKLSAgICAvLyBGSVhNRTogVGhpcyBpcyBpbmNvcnJlY3Q7IG5lZWRzIGFuIGltcGxlbWVudGF0 aW9uIG9yIHRvIGJlIHJlbW92ZWQuCi0gICAgRmFzdE1hbGxvY1N0YXRpc3RpY3Mgc3RhdGlzdGlj cyA9IHsgMCwgMCwgMCB9OworCisgICAgLy8gRklYTUU6IENhbiBibWFsbG9jIGl0c2VsZiByZXBv cnQgdGhlIHN0YXRzIGluc3RlYWQgb2YgcmVseWluZyBvbiB0aGUgT1M/CisgICAgRmFzdE1hbGxv Y1N0YXRpc3RpY3Mgc3RhdGlzdGljczsKKyAgICBzdGF0aXN0aWNzLmZyZWVMaXN0Qnl0ZXMgPSAw OworICAgIHN0YXRpc3RpY3MucmVzZXJ2ZWRWTUJ5dGVzID0gMDsKKworI2lmIE9TKFdJTkRPV1Mp CisgICAgUFJPQ0VTU19NRU1PUllfQ09VTlRFUlMgcmVzb3VyY2VVc2FnZTsKKyAgICBHZXRQcm9j ZXNzTWVtb3J5SW5mbyhHZXRDdXJyZW50UHJvY2VzcygpLCAmcmVzb3VyY2VVc2FnZSwgc2l6ZW9m KHJlc291cmNlVXNhZ2UpKTsKKyAgICBzdGF0aXN0aWNzLmNvbW1pdHRlZFZNQnl0ZXMgPSByZXNv dXJjZVVzYWdlLlBlYWtXb3JraW5nU2V0U2l6ZTsKKyNlbHNlCisgICAgc3RydWN0IHJ1c2FnZSBy ZXNvdXJjZVVzYWdlOworICAgIGdldHJ1c2FnZShSVVNBR0VfU0VMRiwgJnJlc291cmNlVXNhZ2Up OworCisjaWYgT1MoREFSV0lOKQorICAgIHN0YXRpc3RpY3MuY29tbWl0dGVkVk1CeXRlcyA9IHJl c291cmNlVXNhZ2UucnVfbWF4cnNzOworI2Vsc2UKKyAgICBzdGF0aXN0aWNzLmNvbW1pdHRlZFZN Qnl0ZXMgPSByZXNvdXJjZVVzYWdlLnJ1X21heHJzcyAqIDEwMjQ7CisjZW5kaWYgLy8gT1MoREFS V0lOKQorCisjZW5kaWYgLy8gT1MoV0lORE9XUykKICAgICByZXR1cm4gc3RhdGlzdGljczsKIH0K IAo=