Start to rework DM JSON handling.

Start to rework DM JSON handling.

DM's striking off into its own JSON world.  This gets strawman implementations
in place for writing and reading a JSON file mapping test name to hashes.

For what it's worth, I basically want to change _all_ these pieces,
  - MD5 is slow and we can replace it with something faster,
  - JSON schema needs room to grow more data,
  - it'd be nice to hash once instead of twice when reading and writing,
  - this code wants lots of refactoring,
but this gives us a starting platform to work on these bits at our leisure.

E.x. file for now:

mtklein@mtklein ~/skia (dm)> cat good/dm.json 
{
   "3x3bitmaprect_565" : "fc70d985fbfbe70e3a3c9dc626d4f5bc",
   "3x3bitmaprect_8888" : "df1591dde35907399734ea19feb76663",
   "3x3bitmaprect_gpu" : "df1591dde35907399734ea19feb76663",
   "aaclip_565" : "1862798689b838a7ab0dc0652b9ace3a",
   "aaclip_8888" : "47bb314329f0ce243f1d83fd583decb7",
   "aaclip_gpu" : "75f72412d0ef4815770202d297246e7d",
...

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/1d0f1642e8bc0fda200972926e2eaa99744e4d93

[mtklein, 2014-09-05 16:01:48]

mtklein@google.com changed reviewers:
+ jcgregorio@google.com, mtklein@google.com, stephana@google.com

[jcgregorio, 2014-09-05 17:54:21]

https://codereview.chromium.org/546873002/diff/1/dm/DMWriteTask.cpp
File dm/DMWriteTask.cpp (right):

https://codereview.chromium.org/546873002/diff/1/dm/DMWriteTask.cpp#newcode211
dm/DMWriteTask.cpp:211: root[gJsonData[i].name.c_str()] = md5Ascii;
Can we make this an array of hashes, or do you want to save that for a later CL?

[mtklein, 2014-09-05 18:27:43]

> https://codereview.chromium.org/546873002/diff/1/dm/DMWriteTask.cpp#newcode211
> dm/DMWriteTask.cpp:211: root[gJsonData[i].name.c_str()] = md5Ascii;
> Can we make this an array of hashes, or do you want to save that for a later
CL?

Yup, done, but are we ever going to use this?  Aren't we always going to be
producing one hash per test, and the server handles the checking against many
valid hashes for us?

[jcgregorio, 2014-09-05 18:31:41]

On 2014/09/05 18:27:43, mtklein wrote:
> >
https://codereview.chromium.org/546873002/diff/1/dm/DMWriteTask.cpp#newcode211
> > dm/DMWriteTask.cpp:211: root[gJsonData[i].name.c_str()] = md5Ascii;
> > Can we make this an array of hashes, or do you want to save that for a later
> CL?
> 
> Yup, done, but are we ever going to use this?  Aren't we always going to be
> producing one hash per test, and the server handles the checking against many
> valid hashes for us?

True, though we might find out we need to do the checking on the buildbot.
So it's up to you, we can go back to a single hash and only change it later
if we need to, or leave it as is.

LGTM either way.

[mtklein, 2014-09-08 14:58:03]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-09-08 14:58:46]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/546873002/40001

[commit-bot: I haz the power, 2014-09-08 15:05:22]

Committed patchset #3 (id:40001) as 1d0f1642e8bc0fda200972926e2eaa99744e4d93

[stephana, 2014-09-08 15:19:29]

On 2014/09/08 14:58:46, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/546873002/40001

I am putting the following in a design doc, but thought I bring it up in
context. 

My preference for the output JSON would be the following:

[
      {
        "key": {
          "test": "tablecolorfilter",
          "config": "565"
        },
        hash: "95c22801675e8b1e33ed88d78da86878",
        image: "relative/path/to/image.png"
      },
      {
        "key": {
          "test": "strokes_poly",
          "config": "gpu"
        },
        hash: "95c22801675e8b1e33ed88d78da86878",
        image: "relative/path/to/image.png"
      }
      ...
  ]

This is based on the the input format that perf uses
(see
https://github.com/google/skia-buildbot/blob/master/perf/go/ingester/ingester...)
and this output would be the "results" field in a bigger JSON doc (see again
perf format) 
that is uploaded by the buildbots. 

The "key" map allows us to uniquely identify this combination of (test,config)
and should 
naturally extend to SKPs and any other testing scheme we come up with. 
That we should not have DM or GM or SKPs or whatever show up in the tool itself,
but only 
the data that drive it. 

I think there is a problem with the current hashing scheme and multiple hashes
might address
this.
My understanding is that you are hashing the internal representation of the
bitmap (prior to
writing it to a PNG), because there is information loss in the conversion to
PNG. 
IMHO, the problem with that approach is that we have no way of verifying that
hash later, 
because the internal bitmap is gone and cannot be reconstructed from PNG. 
We could add another hash to the above entry for each image, to distinguish
between 
internal hash and PNG hash. But the PNG hash should be used to address the PNG
image.

[mtklein, 2014-09-08 15:28:19]

On 2014/09/08 15:19:29, stephana wrote:
> On 2014/09/08 14:58:46, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> > 
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/546873002/40001
> 
> I am putting the following in a design doc, but thought I bring it up in
> context. 
> 
> My preference for the output JSON would be the following:
> 
> [
>       {
>         "key": {
>           "test": "tablecolorfilter",
>           "config": "565"
>         },
>         hash: "95c22801675e8b1e33ed88d78da86878",
>         image: "relative/path/to/image.png"
>       },
>       {
>         "key": {
>           "test": "strokes_poly",
>           "config": "gpu"
>         },
>         hash: "95c22801675e8b1e33ed88d78da86878",
>         image: "relative/path/to/image.png"
>       }
>       ...
>   ]
> 
> This is based on the the input format that perf uses
> (see
>
https://github.com/google/skia-buildbot/blob/master/perf/go/ingester/ingester...)
> and this output would be the "results" field in a bigger JSON doc (see again
> perf format) 
> that is uploaded by the buildbots. 
> 
> The "key" map allows us to uniquely identify this combination of (test,config)
> and should 
> naturally extend to SKPs and any other testing scheme we come up with. 
> That we should not have DM or GM or SKPs or whatever show up in the tool
itself,
> but only 
> the data that drive it. 
> 
> I think there is a problem with the current hashing scheme and multiple hashes
> might address
> this.
> My understanding is that you are hashing the internal representation of the
> bitmap (prior to
> writing it to a PNG), because there is information loss in the conversion to
> PNG. 
> IMHO, the problem with that approach is that we have no way of verifying that
> hash later, 
> because the internal bitmap is gone and cannot be reconstructed from PNG. 
> We could add another hash to the above entry for each image, to distinguish
> between 
> internal hash and PNG hash. But the PNG hash should be used to address the PNG
> image.

SGTM.  Joe and I were thinking of adding a --hashAsName flag (or similar) that'd
dump out the .pngs in a flat directory with names based on the hash of the
bitmap.  That way we don't need to associate the pngs and hashes after the fact;
they're already together.  But totally agreed on splitting up the key data.