[tracing] Add support for skia caches to dump memory stats

[tracing] Add support for skia caches to dump memory stats

Dump the memory statistics of resource cache and glyph cache using the
SkTraceMemoryDump interface.

BUG=chromium:503168
Committed: https://skia.googlesource.com/skia/+/33c594c9611529ba72474ec52586be65f3e2d76f

[ssid, 2015-08-26 14:55:38]

ssid@chromium.org changed reviewers:
+ reed@google.com

[ssid, 2015-08-26 14:55:38]

PTAL, Thanks.

This will be followed by chrome side crrev.com/1319473003

[reed1, 2015-08-26 18:46:24]

Hmmm, seems like the .h to change is SkGraphics, since that is already public

SkGraphics::DumpMemoryTrace(...)

?

[ssid, 2015-08-27 13:28:38]

Made changes, PTAL. Thanks

[reed1, 2015-08-27 13:59:41]

This is fine. i have some nits:

1.  (sometimes we say DumpMemoryStatistics and sometimes DumpMemoryTrace)

2. I think all of the new code could live in a separate file (e.g.
SkGraphcs_TraceMemory.cpp or something). 

I mention #2 partly because all of the new code is explicitly in support of the
(now single) public API, and other clients may not need/want it.

All of that said, I can perform (or experiment w/ it) such a reorg after the
initial version lands.

SO ... if the name in SkGraphics.h is the one you want (DumpMemoryStatistics or
DumpMemoryTrace), we can land this so we can start to test it.

lgtm

[ssid, 2015-08-27 14:07:45]

On 2015/08/27 13:59:41, reed1 wrote:
> This is fine. i have some nits:
> 
> 1.  (sometimes we say DumpMemoryStatistics and sometimes DumpMemoryTrace)
>
Ah sorry, I thought you wanted different names in public one, since you
mentioned in the comment. I prefer DumpMemoryStatistics.

> 2. I think all of the new code could live in a separate file (e.g.
> SkGraphcs_TraceMemory.cpp or something). 
> 
> I mention #2 partly because all of the new code is explicitly in support of
the
> (now single) public API, and other clients may not need/want it.
> 

Yes, that sounds good. Would you like me to write a follow up CL which moves
this code into a separate file?
Or you prefer to do it yourself?

> All of that said, I can perform (or experiment w/ it) such a reorg after the
> initial version lands.
> 
> SO ... if the name in SkGraphics.h is the one you want (DumpMemoryStatistics
or
> DumpMemoryTrace), we can land this so we can start to test it.
> 
> lgtm

[ssid, 2015-08-27 14:27:33]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-27 14:27:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313793004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313793004/100001

[commit-bot: I haz the power, 2015-08-27 14:29:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-27 14:29:45]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)

[reed1, 2015-08-27 14:32:13]

I'll try the move (in case I don't like it :)

[ssid, 2015-08-27 14:38:36]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-27 14:40:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313793004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313793004/120001

[commit-bot: I haz the power, 2015-08-27 14:46:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-27 14:46:17]

Dry run: This issue passed the CQ dry run.

[ssid, 2015-08-27 15:04:43]

The CQ bit was checked by ssid@chromium.org

[ssid, 2015-08-27 15:04:43]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1313793004/#ps120001
(title: "Fixing SampleApp.")

[commit-bot: I haz the power, 2015-08-27 15:04:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313793004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313793004/120001

[reed1, 2015-08-27 15:05:13]

Not sure why you deleted the TestDump code I had put into SampleApp. It was #if
0 at the moment, but i think we can leave it in.

[reed1, 2015-08-27 15:05:37]

i.e. I don't think you need to have removed the TestDump from resourcecache
either.

[ssid, 2015-08-27 15:09:21]

The CQ bit was unchecked by ssid@chromium.org

[ssid, 2015-08-27 15:18:31]

On 2015/08/27 15:05:37, reed1 wrote:
> i.e. I don't think you need to have removed the TestDump from resourcecache
> either.

I remember you mentioning in your previous CL that the sample app code is just a
place holder till we have support for taking dumps using UI. So, I thought this
was the time to remove it. That is the reason I replaced the functions as well.
I got back the functions and restored the SampleApp.

[ssid, 2015-08-27 15:19:55]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-27 15:20:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313793004/80002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313793004/80002

[commit-bot: I haz the power, 2015-08-27 15:23:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-27 15:23:33]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[ssid, 2015-08-27 15:37:22]

On 2015/08/27 15:18:31, ssid wrote:
> On 2015/08/27 15:05:37, reed1 wrote:
> > i.e. I don't think you need to have removed the TestDump from resourcecache
> > either.
> 
> I remember you mentioning in your previous CL that the sample app code is just
a
> place holder till we have support for taking dumps using UI. So, I thought
this
> was the time to remove it. That is the reason I replaced the functions as
well.
> I got back the functions and restored the SampleApp.

Are you also suggesting that I should add tests for this code? Do you think it
is fine now?

[reed1, 2015-08-27 15:49:00]

lgtm

[ssid, 2015-08-27 15:50:08]

On 2015/08/27 15:49:00, reed1 wrote:
> lgtm

Thanks for your patience.

[ssid, 2015-08-27 15:50:22]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-27 15:50:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313793004/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313793004/150001

[commit-bot: I haz the power, 2015-08-27 15:55:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-27 15:55:56]

Dry run: This issue passed the CQ dry run.

[ssid, 2015-08-27 16:23:22]

The CQ bit was checked by ssid@chromium.org

[commit-bot: I haz the power, 2015-08-27 16:23:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313793004/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313793004/150001

[commit-bot: I haz the power, 2015-08-27 16:23:59]

Committed patchset #9 (id:150001) as
https://skia.googlesource.com/skia/+/33c594c9611529ba72474ec52586be65f3e2d76f