Bug 87263 - Web Inspector: add a command to InspectorMemoryAgent for getting process memory break down
Summary: Web Inspector: add a command to InspectorMemoryAgent for getting process memo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on: 87387
Blocks: 87262
  Show dependency treegraph
 
Reported: 2012-05-23 07:30 PDT by Yury Semikhatsky
Modified: 2012-05-25 02:29 PDT (History)
17 users (show)

See Also:


Attachments
Patch (9.64 KB, patch)
2012-05-23 07:38 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (9.65 KB, patch)
2012-05-23 08:11 PDT, Yury Semikhatsky
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2012-05-23 07:30:08 PDT
We need a command that would return us total size of the memory allocated by the inspected process and its breakdown by the components.
Comment 1 Yury Semikhatsky 2012-05-23 07:38:37 PDT
Created attachment 143568 [details]
Patch
Comment 2 Yury Semikhatsky 2012-05-23 07:39:07 PDT
(In reply to comment #1)
> Created an attachment (id=143568) [details]
> Patch

Chromium part of the change: http://codereview.chromium.org/9669039
Comment 3 WebKit Review Bot 2012-05-23 07:41:24 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 4 Ilya Tikhonovsky 2012-05-23 07:52:09 PDT
Comment on attachment 143568 [details]
Patch

lgtm
Comment 5 Alexei Filippov 2012-05-23 07:53:06 PDT
Comment on attachment 143568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=143568&action=review

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:322
> +    RefPtr<MemoryBlock> jsHeap = MemoryBlock::create().setName("allcated JS heap").setSize(totalJSHeapSize);

nit: Allocated
Comment 6 Early Warning System Bot 2012-05-23 07:54:06 PDT
Comment on attachment 143568 [details]
Patch

Attachment 143568 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12775246
Comment 7 Early Warning System Bot 2012-05-23 08:01:45 PDT
Comment on attachment 143568 [details]
Patch

Attachment 143568 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12773223
Comment 8 Build Bot 2012-05-23 08:06:44 PDT
Comment on attachment 143568 [details]
Patch

Attachment 143568 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12785080
Comment 9 Yury Semikhatsky 2012-05-23 08:11:04 PDT
(In reply to comment #5)
> (From update of attachment 143568 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143568&action=review
> 
> > Source/WebCore/inspector/InspectorMemoryAgent.cpp:322
> > +    RefPtr<MemoryBlock> jsHeap = MemoryBlock::create().setName("allcated JS heap").setSize(totalJSHeapSize);
> 
> nit: Allocated

Fixed.
Comment 10 Yury Semikhatsky 2012-05-23 08:11:26 PDT
Created attachment 143572 [details]
Patch
Comment 11 Pavel Feldman 2012-05-24 04:40:33 PDT
Comment on attachment 143572 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=143572&action=review

> Source/WebCore/inspector/Inspector.json:95
> +                    { "name": "size", "type": "integer", "description": "Size of the block in bytes" },

You can't report size for some of the nodes. Should this be optional?

> Source/WebCore/inspector/Inspector.json:96
> +                    { "name": "name", "type": "string", "description": "User-friendly name describing the component that allocated this block" },

We don't have a way to localize backend. Should this be key names instead?
Comment 12 Yury Semikhatsky 2012-05-24 05:35:00 PDT
(In reply to comment #11)
> (From update of attachment 143572 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143572&action=review
> 
> > Source/WebCore/inspector/Inspector.json:95
> > +                    { "name": "size", "type": "integer", "description": "Size of the block in bytes" },
> 
> You can't report size for some of the nodes. Should this be optional?
> 
Done.

> > Source/WebCore/inspector/Inspector.json:96
> > +                    { "name": "name", "type": "string", "description": "User-friendly name describing the component that allocated this block" },
> 
> We don't have a way to localize backend. Should this be key names instead?
Done.
Comment 13 Yury Semikhatsky 2012-05-24 06:01:12 PDT
Committed r118357: <http://trac.webkit.org/changeset/118357>
Comment 14 WebKit Review Bot 2012-05-24 07:23:26 PDT
Re-opened since this is blocked by 87387