cc: Make full-pile invalidations cheap.

cc: Make full-pile invalidations cheap.

A PicturePile can be really huge. Normally invalidations should be
relatively small anyways. But some times we have to SetNeedsDisplay()
to invalidate the whole layer. We can make these cases fast by only
iterating through the hash_map to find the recorded tiles that exist
and invalidate them.

This takes the record time for a 20 million squared layer from over a
minute to under a millisecond.

R=enne, vmpstr
BUG=420944
Committed: https://crrev.com/3813b916c2dfb95440b1501db7ef5a880b8a49f0
Cr-Commit-Position: refs/heads/master@{#299520}

[danakj, 2014-10-11 00:18:50]

wdyt of these tests?

[danakj, 2014-10-11 00:21:53]

In particular, these tests just time out on my z620 in release without the patch
(ie > 45 seconds). I figured 5 seconds would be stable enough to say "you're
doing it wrong" without hitting it by accident though.

[enne (OOO), 2014-10-11 00:26:34]

https://codereview.chromium.org/643363002/diff/1/cc/resources/picture_pile.cc
File cc/resources/picture_pile.cc (right):

https://codereview.chromium.org/643363002/diff/1/cc/resources/picture_pile.cc...
cc/resources/picture_pile.cc:386: updated = it.second.Invalidate(frame_number)
|| updated;
Oh, right.  I forgot can't just drop tiles because we need invalidation history.
 Yet another reason to move that up into Blink eventually.

https://codereview.chromium.org/643363002/diff/1/cc/resources/picture_pile_un...
File cc/resources/picture_pile_unittest.cc (right):

https://codereview.chromium.org/643363002/diff/1/cc/resources/picture_pile_un...
cc/resources/picture_pile_unittest.cc:424: EXPECT_LT(length.InSeconds(), 5);
Are there correctness tests you can do here too, like making sure recordings are
rerecorded, or the grid cells in PicturePile are invalidated?  That's my biggest
worry--that any optimization for invalidating everything does something
incorrectly.

I have to admit I'm not super keen on the "this test passes if it doesn't take
that long" strategy.  It's like a performance test baked into a unit test.  I'd
almost just say don't test the speed at all if you have local measurements that
support what you're doing.  Maybe other folks feel differently.

[danakj, 2014-10-11 00:30:22]

https://codereview.chromium.org/643363002/diff/1/cc/resources/picture_pile_un...
File cc/resources/picture_pile_unittest.cc (right):

https://codereview.chromium.org/643363002/diff/1/cc/resources/picture_pile_un...
cc/resources/picture_pile_unittest.cc:424: EXPECT_LT(length.InSeconds(), 5);
On 2014/10/11 00:26:33, enne wrote:
> Are there correctness tests you can do here too, like making sure recordings
are
> rerecorded, or the grid cells in PicturePile are invalidated?  That's my
biggest
> worry--that any optimization for invalidating everything does something
> incorrectly.


Hmm ya I guess that'd be good. I did break some LTH tests when I had a copy in
that loop so that's something, but making sure it even does something would be
good.

> I have to admit I'm not super keen on the "this test passes if it doesn't take
> that long" strategy.  It's like a performance test baked into a unit test. 
I'd
> almost just say don't test the speed at all if you have local measurements
that
> support what you're doing.  Maybe other folks feel differently.

OTOH there are a bunch of cases here, and I totally missed this last one at
first (it's the || in the if statement). It'd be easy to miss it again in a
refactor.

[danakj, 2014-10-11 01:21:45]

PTAL Added a correctness test.

[danakj, 2014-10-11 01:22:54]

On 2014/10/11 00:26:34, enne wrote:
> I have to admit I'm not super keen on the "this test passes if it doesn't take
> that long" strategy.  It's like a performance test baked into a unit test. 
I'd
> almost just say don't test the speed at all if you have local measurements
that
> support what you're doing.  Maybe other folks feel differently.

One more comment about this. While they are measuring time, I don't think they
are a perf test. Perf tests give a "how fast did it go" and you try to drive it
down. This is a "can you handle this case correctly?" where correctly is
"without falling off a cliff".

[enne (OOO), 2014-10-13 21:19:16]

lgtm

[danakj, 2014-10-14 17:46:42]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-10-14 17:48:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/643363002/340001

[commit-bot: I haz the power, 2014-10-14 19:08:51]

Committed patchset #4 (id:340001)

[commit-bot: I haz the power, 2014-10-14 19:10:51]

Patchset 4 (id:??) landed as
https://crrev.com/3813b916c2dfb95440b1501db7ef5a880b8a49f0
Cr-Commit-Position: refs/heads/master@{#299520}