Improve performance of GeometryMapper cache.

Improve performance of GeometryMapper cache.

1. Store clips and transforms in vectors attached to the clip and transform nodes.
This avoids all hash map lookups. Invalidation uses a global cache generation id
(currently we don't support anything other than global invalidation of the
GeometryMapper caches).

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2729243002
Cr-Commit-Position: refs/heads/master@{#455571}
Committed: https://chromium.googlesource.com/chromium/src/+/ee72c4b5d2fd1e4882bddca66e0f89fe8784a7fc

[chrishtr, 2017-03-03 19:41:03]

Description was changed from

==========
none


none


none

BUG=
==========

to

==========
none


none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-03-03 19:41:48]

Description was changed from

==========
none


none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Simplify GeometryMapper cache.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-03-06 01:42:21]

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

[commit-bot: I haz the power, 2017-03-06 01:42:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-06 03:23:21]

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

[commit-bot: I haz the power, 2017-03-06 03:23:22]

Dry run: This issue passed the CQ dry run.

[chrishtr, 2017-03-06 19:03:31]

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

[commit-bot: I haz the power, 2017-03-06 19:03:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chrishtr, 2017-03-06 19:25:25]

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

[commit-bot: I haz the power, 2017-03-06 19:26:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chrishtr, 2017-03-06 20:47:11]

Description was changed from

==========
Simplify GeometryMapper cache.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms under a hash map keyed only by the descendant
node.
Values are stored as a vector of ancestor key and clip or transform.

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2017-03-06 22:50:50]

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

[commit-bot: I haz the power, 2017-03-06 22:50:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[chrishtr, 2017-03-07 04:46:32]

chrishtr@chromium.org changed reviewers:
+ wangxianzhu@chromium.org

[chrishtr, 2017-03-07 04:46:33]

It looks like GeometryMapper for PaintLayerClipper use cases caused about a 4-6%
regression
in caching-disabled rasterize_and_record_micro:

https://00e9e64bac09afa7b05834c173021e1b95943a00dfaa5859f3-apidata.googleuser...


This patch gets some of it back:

https://00e9e64bac925009d17303f6925b4d769def018c10433cbb0b-apidata.googleuser...

[Xianzhu, 2017-03-07 16:50:26]

On 2017/03/07 04:46:33, chrishtr wrote:
> It looks like GeometryMapper for PaintLayerClipper use cases caused about a
4-6%
> regression
> in caching-disabled rasterize_and_record_micro:
> 
>
https://00e9e64bac09afa7b05834c173021e1b95943a00dfaa5859f3-apidata.googleuser...
> 
> 
> This patch gets some of it back:
> 
>
https://00e9e64bac925009d17303f6925b4d769def018c10433cbb0b-apidata.googleuser...

The googleusercontent links were temporary. Can you paste the page containing
the original links?

[Xianzhu, 2017-03-07 16:55:25]

Description was changed from

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms under a hash map keyed only by the descendant
node.
Values are stored as a vector of ancestor key and clip or transform.

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms under a hash map keyed only by the descendant
node.
Values are stored as a vector of ancestor key and clip or transform.

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=691614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-03-07 16:58:46]

lgtm

I updated title of crbug.com/692614 to "SlimmingPaintInvalidation performance
regression because of cost of GeometryMapper" and added BUG=692614 to this CL
because I think the cost of GeometryMapper is one of the root causes of the bug.
Some of other landed and planned optimizations are common to spinvalidation and
non-spinvalidation.

https://codereview.chromium.org/2729243002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(right):

https://codereview.chromium.org/2729243002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:348: //
--enable-prefer-compositing-to-lcd-text) for details.
Nit: join the above two lines.

[chrishtr, 2017-03-07 19:09:01]

https://codereview.chromium.org/2729243002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(right):

https://codereview.chromium.org/2729243002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:348: //
--enable-prefer-compositing-to-lcd-text) for details.
On 2017/03/07 at 16:58:46, Xianzhu wrote:
> Nit: join the above two lines.

Done.

[chrishtr, 2017-03-07 19:10:27]

Description was changed from

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms under a hash map keyed only by the descendant
node.
Values are stored as a vector of ancestor key and clip or transform.

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=691614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms under a hash map keyed only by the descendant
node.
Values are stored as a vector of ancestor key and clip or transform.

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-03-07 19:15:58]

Test of turning off GeometryMapper for paint:

https://storage.cloud.google.com/cluster-telemetry/tasks/chromium_perf_runs/c...

-4.196% record_time_caching_disabled

Test of first patchset of this CL (just clips):

https://storage.cloud.google.com/cluster-telemetry/tasks/chromium_perf_runs/c...

-0.685% record_time_caching_disabled

Second patchset (includes transform):

https://storage.cloud.google.com/cluster-telemetry/tasks/chromium_perf_runs/c...

-0.477% record_time_caching_disabled

Third patchset (more improvements):

https://storage.cloud.google.com/cluster-telemetry/tasks/chromium_perf_runs/c...

-0.743% record_time_caching_disabled



These numbers are not as high as I want. Will look further.

[chrishtr, 2017-03-07 22:43:04]

Description was changed from

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms under a hash map keyed only by the descendant
node.
Values are stored as a vector of ancestor key and clip or transform.

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms in vectors attached to the clip and transform
nodes.
This avoids all hash map lookups. Invalidation uses a global cache generation id
(currently we don't support anything other than global invalidation of the
GeometryMapper caches).

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-03-07 23:05:35]

PTAL. I moved the caches onto the clip and transform nodes, thereby removing all
uses of
hash maps. This improves performance significantly on
https://bugs.chromium.org/p/chromium/issues/detail?id=692614&desc=2#c24

[chrishtr, 2017-03-07 23:11:22]

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

[commit-bot: I haz the power, 2017-03-07 23:11:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xianzhu, 2017-03-07 23:19:06]

lgtm.

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.cpp
(right):

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.cpp:91:
CHECK(getCachedClip(clipAndTransform));
Should this be DCHECK?

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.cpp
(right):

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.cpp:106:
TransformCacheEntry(ancestorTransform, toAncestor));
Should we check similar to ClipPaintPropertyNode::setCachedClip()?

[chrishtr, 2017-03-07 23:21:25]

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.cpp
(right):

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.cpp:91:
CHECK(getCachedClip(clipAndTransform));
On 2017/03/07 at 23:19:06, Xianzhu wrote:
> Should this be DCHECK?

Oh good catch!

Actually deleted this, it was during debugging.

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.cpp
(right):

https://codereview.chromium.org/2729243002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.cpp:106:
TransformCacheEntry(ancestorTransform, toAncestor));
On 2017/03/07 at 23:19:06, Xianzhu wrote:
> Should we check similar to ClipPaintPropertyNode::setCachedClip()?

ditto

[pdr., 2017-03-07 23:22:11]

pdr@chromium.org changed reviewers:
+ pdr@chromium.org

[pdr., 2017-03-07 23:22:13]

https://codereview.chromium.org/2729243002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h
(right):

https://codereview.chromium.org/2729243002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h:141:
std::unique_ptr<Vector<ClipCacheEntry>> m_clipCache;
We're exposing a lot of geometry mapper's caching internals here. WDYT of
encapsulating this in a separate class?

I'm thinking something like:
std::unique_ptr<GeometryMapperClipCache> m_clipCache;
with ClipPaintPropertyNode only having setClipCache and getClipCache. Then
GeometryMapperClipCache could encapsulate all of these details.

[pdr., 2017-03-07 23:25:43]

https://codereview.chromium.org/2729243002/diff/360001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h
(right):

https://codereview.chromium.org/2729243002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h:133: 
nit: extra newline

https://codereview.chromium.org/2729243002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h:143:
const FloatClipRect m_infiniteClip;
I don't quite understand this. What is it used for?

[chrishtr, 2017-03-08 00:35:45]

https://codereview.chromium.org/2729243002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h
(right):

https://codereview.chromium.org/2729243002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h:141:
std::unique_ptr<Vector<ClipCacheEntry>> m_clipCache;
On 2017/03/07 at 23:22:13, pdr. wrote:
> We're exposing a lot of geometry mapper's caching internals here. WDYT of
encapsulating this in a separate class?

It's not all that exposed, since it's private to this class..

> 
> I'm thinking something like:
> std::unique_ptr<GeometryMapperClipCache> m_clipCache;
> with ClipPaintPropertyNode only having setClipCache and getClipCache. Then
GeometryMapperClipCache could encapsulate all of these details.

With a design like that, everything could be removed from this class and moved
into that
class..done.

[chrishtr, 2017-03-08 00:35:49]

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

[commit-bot: I haz the power, 2017-03-08 00:36:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-03-08 02:34:55]

Looks super awesome. I like how the logic is all in its own file now. Looking
forward to the perf results.

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h (right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h:180: const
FloatClipRect m_infiniteClip;
I still don't understand this. Is it needed?

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.cpp
(right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.cpp:47:
#if DCHECK_IS_ON
DCHECK_IS_ON()

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.h
(right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.h:58:
std::unique_ptr<Vector<ClipCacheEntry>> m_clipCache;
Because GeometryMapperClipCache is lazily allocated, do we need to use a unique
putter? I think we can just store Vector<ClipCacheEntry> directly:
Vector<ClipCacheEntry> m_clipCache;

Similarly for m_transformCache.

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperTransformCache.cpp
(right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperTransformCache.cpp:43:
#if DCHECK_IS_ON
DCHECK_IS_ON()

[chrishtr, 2017-03-08 02:59:28]

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h (right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h:180: const
FloatClipRect m_infiniteClip;
On 2017/03/08 at 02:34:55, pdr. wrote:
> I still don't understand this. Is it needed?

Yes. Since localToAncestorClipRectInternal now returns a reference instead of
a value, there needs to always be a non-local object to take the reference of.
This variable is the return value when there is no cached clip to return
(e.g. for mapping a clip to itself, or if it fails to find an ancestor).

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.cpp
(right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.cpp:47:
#if DCHECK_IS_ON
On 2017/03/08 at 02:34:55, pdr. wrote:
> DCHECK_IS_ON()

Done.

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.h
(right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperClipCache.h:58:
std::unique_ptr<Vector<ClipCacheEntry>> m_clipCache;
On 2017/03/08 at 02:34:55, pdr. wrote:
> Because GeometryMapperClipCache is lazily allocated, do we need to use a
unique putter? I think we can just store Vector<ClipCacheEntry> directly:
> Vector<ClipCacheEntry> m_clipCache;
> 
> Similarly for m_transformCache.

Done.

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperTransformCache.cpp
(right):

https://codereview.chromium.org/2729243002/diff/450001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapperTransformCache.cpp:43:
#if DCHECK_IS_ON
On 2017/03/08 at 02:34:55, pdr. wrote:
> DCHECK_IS_ON()

Done.

[chrishtr, 2017-03-08 03:00:05]

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

[commit-bot: I haz the power, 2017-03-08 03:00:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-03-08 03:05:13]

thanks, LGTM

[chrishtr, 2017-03-08 03:17:17]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-03-08 03:17:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org, pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2729243002/#ps490001
(title: "none")

[commit-bot: I haz the power, 2017-03-08 03:17:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 04:04:43]

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

[commit-bot: I haz the power, 2017-03-08 04:04:44]

Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[chrishtr, 2017-03-08 04:57:49]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2017-03-08 04:57:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 05:08:35]

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

[commit-bot: I haz the power, 2017-03-08 05:08:37]

Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[chrishtr, 2017-03-08 17:21:30]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2017-03-08 17:22:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chrishtr, 2017-03-08 17:54:09]

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

[commit-bot: I haz the power, 2017-03-08 17:54:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 18:28:56]

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

[commit-bot: I haz the power, 2017-03-08 18:28:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chrishtr, 2017-03-08 18:40:28]

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

[commit-bot: I haz the power, 2017-03-08 18:41:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 18:50:54]

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

[commit-bot: I haz the power, 2017-03-08 18:50:55]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chrishtr, 2017-03-08 18:53:54]

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

[commit-bot: I haz the power, 2017-03-08 18:54:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chrishtr, 2017-03-08 19:21:18]

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

[commit-bot: I haz the power, 2017-03-08 19:22:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chrishtr, 2017-03-08 19:40:01]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-03-08 19:40:02]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2729243002/#ps670001
(title: "none")

[commit-bot: I haz the power, 2017-03-08 19:40:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 19:52:43]

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

[commit-bot: I haz the power, 2017-03-08 19:52:44]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chrishtr, 2017-03-08 20:04:00]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-03-08 20:04:00]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2729243002/#ps690001
(title: "none")

[commit-bot: I haz the power, 2017-03-08 20:04:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 22:52:09]

CQ is committing da patch.

Bot data: {"patchset_id": 690001, "attempt_start_ts": 1489003440413040,
"parent_rev": "10f4c63878b42aac67c41e5a69aebf7cc6438fe3", "commit_rev":
"ee72c4b5d2fd1e4882bddca66e0f89fe8784a7fc"}

[commit-bot: I haz the power, 2017-03-08 22:52:49]

Description was changed from

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms in vectors attached to the clip and transform
nodes.
This avoids all hash map lookups. Invalidation uses a global cache generation id
(currently we don't support anything other than global invalidation of the
GeometryMapper caches).

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms in vectors attached to the clip and transform
nodes.
This avoids all hash map lookups. Invalidation uses a global cache generation id
(currently we don't support anything other than global invalidation of the
GeometryMapper caches).

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2729243002
Cr-Commit-Position: refs/heads/master@{#455571}
Committed:
https://chromium.googlesource.com/chromium/src/+/ee72c4b5d2fd1e4882bddca66e0f...
==========

[commit-bot: I haz the power, 2017-03-08 22:52:52]

Committed patchset #39 (id:690001) as
https://chromium.googlesource.com/chromium/src/+/ee72c4b5d2fd1e4882bddca66e0f...

[Michael Achenbach, 2017-03-09 07:25:52]

machenbach@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Achenbach, 2017-03-09 07:25:53]

Note this breaks msan layout tests:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty...

[Michael Achenbach, 2017-03-09 07:26:26]

Description was changed from

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms in vectors attached to the clip and transform
nodes.
This avoids all hash map lookups. Invalidation uses a global cache generation id
(currently we don't support anything other than global invalidation of the
GeometryMapper caches).

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2729243002
Cr-Commit-Position: refs/heads/master@{#455571}
Committed:
https://chromium.googlesource.com/chromium/src/+/ee72c4b5d2fd1e4882bddca66e0f...
==========

to

==========
Improve performance of GeometryMapper cache.

1. Store clips and transforms in vectors attached to the clip and transform
nodes.
This avoids all hash map lookups. Invalidation uses a global cache generation id
(currently we don't support anything other than global invalidation of the
GeometryMapper caches).

2. Return clips and transforms by reference, to avoid copying.

#1 is expected to be a good optimization because we almost always compute clips
and transforms relative to the same ancestor, so the vectors are short and hash
lookups are reduced.

BUG=692614
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2729243002
Cr-Commit-Position: refs/heads/master@{#455571}
Committed:
https://chromium.googlesource.com/chromium/src/+/ee72c4b5d2fd1e4882bddca66e0f...
==========

[Michael Achenbach, 2017-03-09 07:26:26]

machenbach@chromium.org changed reviewers:
- machenbach@chromium.org

[Michael Achenbach, 2017-03-09 09:12:47]

A revert of this CL (patchset #39 id:690001) has been created in
https://codereview.chromium.org/2738113003/ by machenbach@chromium.org.

The reason for reverting is: Breaks msan:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty....

[Michael Achenbach, 2017-03-09 09:23:32]

Also breaks chromeos msan and lets the bot time out:
https://build.chromium.org/p/chromium.memory.full/builders/Linux%20ChromeOS%2...

The swarming shard output of the timed-out shards shows failures from this CL.