Bug 99457 - Web Inspector: NMI provide data for mixing with tcmalloc heap dumps
Summary: Web Inspector: NMI provide data for mixing with tcmalloc heap dumps
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-16 05:22 PDT by Ilya Tikhonovsky
Modified: 2012-10-19 00:45 PDT (History)
14 users (show)

See Also:


Attachments
Patch (23.71 KB, patch)
2012-10-16 05:58 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
Patch (22.65 KB, patch)
2012-10-17 09:08 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
Patch (20.38 KB, patch)
2012-10-17 09:18 PDT, Ilya Tikhonovsky
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2012-10-16 05:22:36 PDT
Early we used tcmalloc heap dumps for check what classes have to be instrumented first.
We instrumented a lot of classes and it became difficult to use heap dumps for this task.
We need a way to see the classes that aren't instrumented yet or their objects were not reached during snapshot.
We could use heap dump but need to mix tcmalloc data with the data about already counted objects.
Comment 1 Ilya Tikhonovsky 2012-10-16 05:58:59 PDT
Created attachment 168932 [details]
Patch
Comment 2 Rik Cabanier 2012-10-16 09:39:04 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 3 Yury Semikhatsky 2012-10-17 08:09:48 PDT
Comment on attachment 168932 [details]
Patch

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

> Source/WebKit/chromium/public/WebDevToolsAgentClient.h:66
> +        virtual size_t visitObject(const void* ptr) = 0;

Please use separate interface for this.
Comment 4 Ilya Tikhonovsky 2012-10-17 09:08:49 PDT
Created attachment 169193 [details]
Patch
Comment 5 Ilya Tikhonovsky 2012-10-17 09:18:57 PDT
Created attachment 169194 [details]
Patch
Comment 6 Ilya Tikhonovsky 2012-10-17 09:20:48 PDT
(In reply to comment #3)
> (From update of attachment 168932 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168932&action=review
> 
> > Source/WebKit/chromium/public/WebDevToolsAgentClient.h:66
> > +        virtual size_t visitObject(const void* ptr) = 0;
> 
> Please use separate interface for this.

comments addressed
Comment 7 Yury Semikhatsky 2012-10-17 09:35:48 PDT
Comment on attachment 169194 [details]
Patch

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

> Source/WebKit/chromium/public/WebDevToolsAgentClient.h:78
> +    virtual void dumpUncountedObjects(const ObjectInfoProvider*) { }

dumpAllocatedObjects?
Comment 8 Ilya Tikhonovsky 2012-10-17 11:26:39 PDT
(In reply to comment #7)
> (From update of attachment 169194 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169194&action=review
> 
> > Source/WebKit/chromium/public/WebDevToolsAgentClient.h:78
> > +    virtual void dumpUncountedObjects(const ObjectInfoProvider*) { }
> 
> dumpAllocatedObjects?

I am sure that we do not want to print all the allocated objects.
Thus, the current name better suits for this purpose.
Comment 9 Yury Semikhatsky 2012-10-18 01:18:43 PDT
Comment on attachment 169194 [details]
Patch

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

> Source/WebKit/chromium/public/WebDevToolsAgentClient.h:72
> +    class ObjectInfoProvider {

Maybe rename ObjectInfoProvider to InstrumentedObjects?
Comment 10 Vsevolod Vlasov 2012-10-18 01:21:41 PDT
Comment on attachment 169194 [details]
Patch

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

>>> Source/WebKit/chromium/public/WebDevToolsAgentClient.h:78
>>> +    virtual void dumpUncountedObjects(const ObjectInfoProvider*) { }
>> 
>> dumpAllocatedObjects?
> 
> I am sure that we do not want to print all the allocated objects.
> Thus, the current name better suits for this purpose.

dumpUncountedAllocatedObjects then?
Comment 11 Vsevolod Vlasov 2012-10-18 01:23:01 PDT
Comment on attachment 169194 [details]
Patch

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

>> Source/WebKit/chromium/public/WebDevToolsAgentClient.h:72
>> +    class ObjectInfoProvider {
> 
> Maybe rename ObjectInfoProvider to InstrumentedObjects?

AllocatedObjectSizeProvider?
Comment 12 Ilya Tikhonovsky 2012-10-19 00:45:32 PDT
Committed r131869: <http://trac.webkit.org/changeset/131869>