Bug 105261 - Web Inspector: show cached images under MemoryCache -> Images section
Summary: Web Inspector: show cached images under MemoryCache -> Images section
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:
Blocks:
 
Reported: 2012-12-18 00:00 PST by Yury Semikhatsky
Modified: 2012-12-18 03:47 PST (History)
14 users (show)

See Also:


Attachments
Patch (22.54 KB, patch)
2012-12-18 00:03 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (23.80 KB, patch)
2012-12-18 02:26 PST, Yury Semikhatsky
no flags 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-12-18 00:00:08 PST
Native memory profiler should show specific CachedImages with their sizes under Memory cache resources: Image section so that user can figure out which images make up the cache.
Comment 1 Yury Semikhatsky 2012-12-18 00:03:55 PST
Created attachment 179891 [details]
Patch
Comment 2 Alexei Filippov 2012-12-18 01:10:09 PST
Comment on attachment 179891 [details]
Patch

lgtm
Comment 3 Alexei Filippov 2012-12-18 02:08:50 PST
Comment on attachment 179891 [details]
Patch

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

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:272
> +    _addChildrenFromGraph: function(memoryBlock)

Missing function annotations here and below.

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:294
> +        for (var i = 0; i < edges.length; i++) {
> +            var target = edges[i].target();
> +            if (target.className() === "CachedImage") {

Perhaps a helper iterator function forAllChildrenOfClass?

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:302
> +                if (image.className() === "BitmapImage") {

What about other classes? Later?

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:439
> +        return this.targetsOfAllEdges(edgeName)[0];

Pure functional code doesn't work well in JS. ;-)
This one doesn't seem too effective. Add a TODO/FIXME?
Comment 4 Yury Semikhatsky 2012-12-18 02:26:06 PST
(In reply to comment #3)
> (From update of attachment 179891 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179891&action=review
> 
> > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:272
> > +    _addChildrenFromGraph: function(memoryBlock)
> 
> Missing function annotations here and below.
> 
Fixed.

> > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:294
> > +        for (var i = 0; i < edges.length; i++) {
> > +            var target = edges[i].target();
> > +            if (target.className() === "CachedImage") {
> 
> Perhaps a helper iterator function forAllChildrenOfClass?
> 
I'd change that when we have another client for the code to better understand which API we need.

> > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:302
> > +                if (image.className() === "BitmapImage") {
> 
> What about other classes? Later?
> 
Yes, it is a first step. Show just referenced pixel buffer which we know may be quite big. If you feel that we should show some other objects please speak up.

> > Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:439
> > +        return this.targetsOfAllEdges(edgeName)[0];
> 
> Pure functional code doesn't work well in JS. ;-)
> This one doesn't seem too effective. Add a TODO/FIXME?
We can optimize this code when we find it inefficient for the current use case it seems to be good enough.
Comment 5 Yury Semikhatsky 2012-12-18 02:26:49 PST
Created attachment 179910 [details]
Patch
Comment 6 WebKit Review Bot 2012-12-18 03:47:11 PST
Comment on attachment 179910 [details]
Patch

Clearing flags on attachment: 179910

Committed r138005: <http://trac.webkit.org/changeset/138005>
Comment 7 WebKit Review Bot 2012-12-18 03:47:15 PST
All reviewed patches have been landed.  Closing bug.