Bug 91204

There are also bug 80444 and bug 86636 opened about the subject, but they focus more on a web apps usage. This change is useful to serve performance tests (bug 78984)

Created attachment 152191 [details] Patch

Comment on attachment 152191 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152191&action=review > Source/WebCore/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=78984 You need to update the title and a the bug number.

Created attachment 152192 [details] Patch

Comment on attachment 152192 [details] Patch Attachment 152192 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13232288 New failing tests: fast/dom/Window/window-properties-performance.html

Created attachment 152290 [details] Archive of layout-test-results from gce-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

Comment on attachment 152192 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152192&action=review > Source/WebCore/page/MemoryInfo.idl:43 > + readonly attribute unsigned long reservedFastMalloc; > + readonly attribute unsigned long committedFastMalloc; > + readonly attribute unsigned long freeListFastMalloc; "FastMalloc" is an internal concept to WebKit and should not be exposed to the web platform.

Well, FastMalloc is in as much an internal level as the JS Heap is. And JS Heap stats are also available in console.memory under ENABLE(INSPECTOR). Why would the JS heap allowed to be exposed there and not FastMalloc?

> Well, FastMalloc is in as much an internal level as the JS Heap is. > And JS Heap stats are also available in console.memory under ENABLE(INSPECTOR). > Why would the JS heap allowed to be exposed there and not FastMalloc? Firefox and other browsers have a JS heap. None of the other browsers have a "fast malloc" concept.

It's not critical, but it would have made perftests' memory measure method easier. We can access to these numbers through DRT.

If you're planning to use this internally in the WebKit project, consider exposing this information via Internals.idl. That way we won't leak this API to the web.

firefox also has mozalloc/jemalloc. But how is it relevant this console.memory is AFAIK a WebKit-only API? Why would exposing the heap stats be ok and not fastmalloc stats? Also, this gets exposed only when enabled via frame->settings()->memoryInfoEnabled()

We should probably expose it via internals.

Intially, I submitted this patch to have a way in my apps to get more statistics about memory usage. It was considered useful for bug 78984, but this was not my main goal. What's the harm in adding FastMalloc stats together with JS heap stats if they are protected by memoryInfoEnabled settings?

> Intially, I submitted this patch to have a way in my apps to get more statistics about memory usage. If that's your goal, one approach is to start by proposing the API to a standards body. I suspect the first piece of feedback you'll get is that "fast malloc" is a WebKit-specific concept and therefore not appropriate to expose to the web platform.

I agree with Adam here. I don't we should be exposing this information to the Web. It's not appropriate.

Adam: then console.memory.*JSHeap* got some preferential treatment by not needing to go through a standards body review :) Even console.memory is WebKit-specific. Ryosuke: that's not always exposed to web, since it needs WebKitMemoryInfoEnabled to be set explicitely beforehand. I guessed it could have been useful as well to expose these statistics together with the JS heap statistics, to be used in the inspector/web timing/Performance. Anyway, I will rewrite the patch to expose these statistics in window.internals Thanks for your input Adam and Ryosuke. I will open a new bug.

I have opened Bug 91274 - Add FastMalloc statistics in window.internals

> Adam: then console.memory.*JSHeap* got some preferential treatment by not needing to go through a standards body review :) Yes. I'm not super happy about that, and it's a debt we'll need to pay at some point. :-/ > Anyway, I will rewrite the patch to expose these statistics in window.internals Thanks. I do think that's a better approach in this case.

Maybe JSHeap stats could be migrated to window.internals as well then. Thanks again for your review.