Print gpu/cache stats from DM.

Print gpu/cache stats from DM.

[bsalomon, 2015-02-01 01:34:47]

bsalomon@google.com changed reviewers:
+ mtklein@google.com, robertphillips@google.com

[bsalomon, 2015-02-01 01:34:47]

Moves GPUStats from GrContext to GrGpu, stats defns to GrTest.cpp, adds
functions to append cache/gpu stats to a string, and adds mechanism for per-sink
verbose logging that appears after the test name is printed in done().

[mtklein, 2015-02-02 16:23:17]

I have also uploaded my suggestions in the form of a CL:

https://codereview.chromium.org/894013002

https://codereview.chromium.org/886233004/diff/40001/dm/DM.cpp
File dm/DM.cpp (right):

https://codereview.chromium.org/886233004/diff/40001/dm/DM.cpp#newcode54
dm/DM.cpp:54: SkDebugf("%s(%4dMB %5d) %s\t%s %s %s  ", FLAGS_verbose ? "\n" :
kSkOverwriteLine
I think you'll have a better time if we log everything from one SkDebugf call. 
On most platforms, SkDebugf is synchronized so that single calls print out
atomically.   But there'll definitely be no guarantee that your two SkDebugf
here will print sequentially.

https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.cpp#newcode148
dm/DMSrcSink.cpp:148: DEFINE_bool(gpuStats, false, "Print GPU stats after each
gpu sink runs? Requires verbose.");
Seems like --gpuStats is plenty enough to control this.  Why don't we just
ignore --verbose?

https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.h
File dm/DMSrcSink.h (right):

https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.h#newcode52
dm/DMSrcSink.h:52: mutable SkString fVerboseLog;
I don't think this is going to work.  Storing mutable state on Sinks means
they're very not reentrant.

I think the safe thing to do is write it to this API:
  Error SK_WARN_UNUSED_RESULT Draw(const Src&, SkBitmap*, SkWStream*, SkString*
log);
Then scope the log SkString to Task::Run() inside DM.

https://codereview.chromium.org/886233004/diff/40001/src/gpu/GrTest.cpp
File src/gpu/GrTest.cpp (right):

https://codereview.chromium.org/886233004/diff/40001/src/gpu/GrTest.cpp#newco...
src/gpu/GrTest.cpp:105: out->appendf("Budget: %d items %d bytes\n", fMaxCount,
fMaxBytes);
Had some trouble with %d and size_t in these appendf lines.  I figure casting
them all to int would never go wrong here?

[bsalomon, 2015-02-02 19:44:59]

https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.h
File dm/DMSrcSink.h (right):

https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.h#newcode52
dm/DMSrcSink.h:52: mutable SkString fVerboseLog;
On 2015/02/02 16:23:17, mtklein wrote:
> I don't think this is going to work.  Storing mutable state on Sinks means
> they're very not reentrant.
> 
> I think the safe thing to do is write it to this API:
>   Error SK_WARN_UNUSED_RESULT Draw(const Src&, SkBitmap*, SkWStream*,
SkString*
> log);
> Then scope the log SkString to Task::Run() inside DM.

Ah... forgot that the sinks weren't instanced per test. TBH I'll probably
abandon this for now. I was able to use it to find my bug. Maybe when I'm back
in NC I'll spend the time to clean up.

[bsalomon, 2015-02-03 01:31:13]

On 2015/02/02 16:23:17, mtklein wrote:
> I have also uploaded my suggestions in the form of a CL:
> 
> https://codereview.chromium.org/894013002
> 

Somehow I missed this the first time around... plane fatigue... looking at that
cl


> https://codereview.chromium.org/886233004/diff/40001/dm/DM.cpp
> File dm/DM.cpp (right):
> 
> https://codereview.chromium.org/886233004/diff/40001/dm/DM.cpp#newcode54
> dm/DM.cpp:54: SkDebugf("%s(%4dMB %5d) %s\t%s %s %s  ", FLAGS_verbose ? "\n" :
> kSkOverwriteLine
> I think you'll have a better time if we log everything from one SkDebugf call.

> On most platforms, SkDebugf is synchronized so that single calls print out
> atomically.   But there'll definitely be no guarantee that your two SkDebugf
> here will print sequentially.
> 
> https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.cpp
> File dm/DMSrcSink.cpp (right):
> 
>
https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.cpp#newcode148
> dm/DMSrcSink.cpp:148: DEFINE_bool(gpuStats, false, "Print GPU stats after each
> gpu sink runs? Requires verbose.");
> Seems like --gpuStats is plenty enough to control this.  Why don't we just
> ignore --verbose?
> 
> https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.h
> File dm/DMSrcSink.h (right):
> 
> https://codereview.chromium.org/886233004/diff/40001/dm/DMSrcSink.h#newcode52
> dm/DMSrcSink.h:52: mutable SkString fVerboseLog;
> I don't think this is going to work.  Storing mutable state on Sinks means
> they're very not reentrant.
> 
> I think the safe thing to do is write it to this API:
>   Error SK_WARN_UNUSED_RESULT Draw(const Src&, SkBitmap*, SkWStream*,
SkString*
> log);
> Then scope the log SkString to Task::Run() inside DM.
> 
> https://codereview.chromium.org/886233004/diff/40001/src/gpu/GrTest.cpp
> File src/gpu/GrTest.cpp (right):
> 
>
https://codereview.chromium.org/886233004/diff/40001/src/gpu/GrTest.cpp#newco...
> src/gpu/GrTest.cpp:105: out->appendf("Budget: %d items %d bytes\n", fMaxCount,
> fMaxBytes);
> Had some trouble with %d and size_t in these appendf lines.  I figure casting
> them all to int would never go wrong here?