Bug 99309 - Web Inspector: convert manual size calculation of different WebKit things into MemoryInstrumentation.
Summary: Web Inspector: convert manual size calculation of different WebKit things int...
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: Ilya Tikhonovsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-15 04:49 PDT by Ilya Tikhonovsky
Modified: 2012-10-15 10:35 PDT (History)
11 users (show)

See Also:


Attachments
Patch (42.91 KB, patch)
2012-10-15 05:20 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
Patch (42.86 KB, patch)
2012-10-15 05:26 PDT, Ilya Tikhonovsky
buildbot: commit-queue-
Details | Formatted Diff | Diff
[please ignore] (6.11 KB, text/plain)
2012-10-15 10:33 PDT, Hans Muller
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2012-10-15 04:49:35 PDT
MemoryInstrumentation is our primary method for WebCore internal classes.
But due to historical reasons sizes for some objects  were calculated manually.
I think we could fix this.
Comment 1 Ilya Tikhonovsky 2012-10-15 05:20:14 PDT
Created attachment 168684 [details]
Patch
Comment 2 Ilya Tikhonovsky 2012-10-15 05:26:04 PDT
Created attachment 168685 [details]
Patch
Comment 3 Build Bot 2012-10-15 05:58:13 PDT
Comment on attachment 168685 [details]
Patch

Attachment 168685 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14300666
Comment 4 Early Warning System Bot 2012-10-15 06:06:01 PDT
Comment on attachment 168685 [details]
Patch

Attachment 168685 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14289634
Comment 5 Yury Semikhatsky 2012-10-15 06:07:00 PDT
Comment on attachment 168685 [details]
Patch

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

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:156
> +static String nodeName(Node* node)

No need to make it static as it is already in anonymous namespace.
Comment 6 Alexei Filippov 2012-10-15 06:08:01 PDT
Comment on attachment 168685 [details]
Patch

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

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:-479
> -                    unusedSize -= child.children[j].size;

Why do you need this *.Unused record? What's wrong with calculating it? What's if *.Unused is not present? It seems that unusedSize will be zero, e.g. for JSHeap.
Comment 7 Build Bot 2012-10-15 06:08:49 PDT
Comment on attachment 168685 [details]
Patch

Attachment 168685 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14290676
Comment 8 Early Warning System Bot 2012-10-15 06:13:41 PDT
Comment on attachment 168685 [details]
Patch

Attachment 168685 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14288730
Comment 9 Gyuyoung Kim 2012-10-15 06:15:23 PDT
Comment on attachment 168685 [details]
Patch

Attachment 168685 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14292740
Comment 10 Ilya Tikhonovsky 2012-10-15 06:52:01 PDT
Committed r131299: <http://trac.webkit.org/changeset/131299>
Comment 11 Ilya Tikhonovsky 2012-10-15 07:09:55 PDT
(In reply to comment #6)
> (From update of attachment 168685 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168685&action=review
> 
> > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:-479
> > -                    unusedSize -= child.children[j].size;
> 
> Why do you need this *.Unused record? What's wrong with calculating it? What's if *.Unused is not present? It seems that unusedSize will be zero, e.g. for JSHeap.

This patch flips the code from manual block building to the automatic builder.
The builder builds parent MemoryBlocks on the fly and calculates their sizes.
As a result when I use automatic builder code for JSHeap I need to add two blocks, JSHeap.Used and JSHeap.Unused. The builder makes parent JSHeap block automatically and assigns the right size to it. Latter, on the front end size, we use JSHeap size as the allocated size and JSHeap.Unused as unused size for JSHeap.
Comment 12 Hans Muller 2012-10-15 10:33:00 PDT
Created attachment 168737 [details]
[please ignore]

Added FloatRect::extend() to simplify writing loops that accumulate the bounding box for a sequence of vertices.  For example:

FloatRect box(vertices[0], FloatSize(0,0));
for (unsigned i = 1; i < vertices.length; i++)
    box.extend(vertices[i]);

ExclusionShape and ExclusionPolygon now use FloatRect::extend().
Comment 13 Hans Muller 2012-10-15 10:35:18 PDT
(In reply to comment #12)
> Created an attachment (id=168737) [details]

My apologies for accidentally attaching this patch/comment to the wrong bug.