WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
91204
Add FastMalloc statistics in console.memory
https://bugs.webkit.org/show_bug.cgi?id=91204
Summary
Add FastMalloc statistics in console.memory
Olivier Blin
Reported
2012-07-13 02:12:59 PDT
In addition to JS heap statistics, the console.memory object could also provide FastMalloc statistics (reservedFastMalloc, committedFastMalloc, freeListFastMalloc).
Attachments
Patch
(3.53 KB, patch)
2012-07-13 02:21 PDT
,
Olivier Blin
no flags
Details
Formatted Diff
Diff
Patch
(3.48 KB, patch)
2012-07-13 02:30 PDT
,
Olivier Blin
abarth
: review-
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from gce-cr-linux-03
(416.66 KB, application/zip)
2012-07-13 10:25 PDT
,
WebKit Review Bot
no flags
Details
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Olivier Blin
Comment 1
2012-07-13 02:17:25 PDT
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
)
Olivier Blin
Comment 2
2012-07-13 02:21:02 PDT
Created
attachment 152191
[details]
Patch
Zoltan Horvath
Comment 3
2012-07-13 02:25:37 PDT
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.
Olivier Blin
Comment 4
2012-07-13 02:30:03 PDT
Created
attachment 152192
[details]
Patch
WebKit Review Bot
Comment 5
2012-07-13 10:25:41 PDT
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
WebKit Review Bot
Comment 6
2012-07-13 10:25:45 PDT
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
Adam Barth
Comment 7
2012-07-13 10:26:44 PDT
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.
Olivier Blin
Comment 8
2012-07-13 10:32:55 PDT
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?
Adam Barth
Comment 9
2012-07-13 10:41:33 PDT
> 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.
Zoltan Horvath
Comment 10
2012-07-13 10:48:28 PDT
It's not critical, but it would have made perftests' memory measure method easier. We can access to these numbers through DRT.
Adam Barth
Comment 11
2012-07-13 10:54:09 PDT
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.
Olivier Blin
Comment 12
2012-07-13 11:51:24 PDT
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()
Ryosuke Niwa
Comment 13
2012-07-13 11:58:29 PDT
We should probably expose it via internals.
Olivier Blin
Comment 14
2012-07-13 12:31:02 PDT
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?
Adam Barth
Comment 15
2012-07-13 12:39:44 PDT
> 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.
Ryosuke Niwa
Comment 16
2012-07-13 13:09:55 PDT
I agree with Adam here. I don't we should be exposing this information to the Web. It's not appropriate.
Olivier Blin
Comment 17
2012-07-13 13:25:50 PDT
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.
Olivier Blin
Comment 18
2012-07-13 13:31:39 PDT
I have opened
Bug 91274
- Add FastMalloc statistics in window.internals
Adam Barth
Comment 19
2012-07-13 13:57:06 PDT
> 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.
Olivier Blin
Comment 20
2012-07-13 14:11:26 PDT
Maybe JSHeap stats could be migrated to window.internals as well then. Thanks again for your review.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug