cc mojo: Use ArrayDataViews in RenderPasses

cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of
18% on an HP Z620. Deserialization performance improves
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews
in parent StructTraits, we had to do extra work at
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization.
StructTraits assume a preallocated type and populated
fields in that type.

Alternatively we could have deserialized DrawQuads into
unique_ptrs, but cc required that we allocate DrawQuads
inline within a buffer in order to minimize the cost of
allocations (there can be hundreds of DrawQuads in a
CompositorFrame).

2. Previously, SharedQuadStates were serialized
independently of DrawQuads. However, in in-memory form,
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in
the SharedQuadStateList. After the QuadList and SharedQuadStateList were
serialized,  shared_quad_state_references updated the raw
pointers in the DrawQuads in the quad list to refer to the
SharedQuadStates. This required revisiting all deserialized
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0QPe8xFAhYMUAXaxoEqgg/pubhtml

BUG=624459
TBR=danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/edd40fdc29170e97ab54578fdc5096fc564c0ccb
Cr-Commit-Position: refs/heads/master@{#408809}

[Fady Samuel, 2016-07-22 20:37:54]

Description was changed from

==========
WIP Experiment: Use ArrayDataViews in RenderPasses

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

to

==========
WIP Experiment: Use ArrayDataViews in RenderPasses

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-22 20:45:26]

fsamuel@chromium.org changed reviewers:
+ yzshen@chromium.org

[Fady Samuel, 2016-07-22 20:45:27]

Hi Yuzhu,

This CL is not yet ready for formal review but I thought I'd pass it along to
you to inspect. Correctness tests pass and perf tests are significantly
improved. There's still lots of room for improvement though. I haven't profiled
yet.

[Fady Samuel, 2016-07-24 17:21:53]

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

[commit-bot: I haz the power, 2016-07-24 17:22:04]

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, 2016-07-24 17:26:55]

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

[commit-bot: I haz the power, 2016-07-24 17:26:55]

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

[Fady Samuel, 2016-07-25 21:20:16]

Description was changed from

==========
WIP Experiment: Use ArrayDataViews in RenderPasses

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 18% on an HP Z620.
Deserialization performance improves between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews in
parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. StructTraits assume a
preallocated type and populated fields in that type. 
Alternatively we could have deserialized DrawQuads into unique_ptrs, but cc
required that we allocate DrawQuads inline within a buffer in order
to minimize the cost of allocations (there can be hundreds
of DrawQuads in a CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form,
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were serialized,
 shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:20:36]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 18% on an HP Z620.
Deserialization performance improves between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews in
parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. StructTraits assume a
preallocated type and populated fields in that type. 
Alternatively we could have deserialized DrawQuads into unique_ptrs, but cc
required that we allocate DrawQuads inline within a buffer in order
to minimize the cost of allocations (there can be hundreds
of DrawQuads in a CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form,
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were serialized,
 shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. StructTraits assume a
preallocated type and populated fields in that type. 
Alternatively we could have deserialized DrawQuads into unique_ptrs, but cc
required that we allocate DrawQuads inline within a buffer in order
to minimize the cost of allocations (there can be hundreds
of DrawQuads in a CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form,
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were serialized,
 shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:21:11]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. StructTraits assume a
preallocated type and populated fields in that type. 
Alternatively we could have deserialized DrawQuads into unique_ptrs, but cc
required that we allocate DrawQuads inline within a buffer in order
to minimize the cost of allocations (there can be hundreds
of DrawQuads in a CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form,
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were serialized,
 shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were serialized,
 shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:21:30]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were serialized,
 shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:21:48]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw pointers in
the DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw pointers in the
DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:22:13]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in the 
SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw pointers in the
DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw pointers in the
DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:22:52]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw pointers in the
DrawQuads in the quad list to refer to the SharedQuadStates. This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the SharedQuadStates.
This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:23:19]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the SharedQuadStates.
This
required revisiting all deserialized DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-25 21:23:32]

PTAL Yuzhu!

[yzshen1, 2016-07-26 17:56:03]

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads.mojom
File cc/ipc/quads.mojom (right):

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads.mojom#newc...
cc/ipc/quads.mojom:131: // Index into the containing pass' shared quad state
array which has state
Shall we remove this comment?

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:474: struct StructTraits<cc::mojom::DrawQuad,
cc::DrawQuad> {
Is this StructTraits still needed after we introduce the one above?

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:549: static void AdvanceIterator(QuadListArray&
input, ConstIterator& iterator) {
This API change seems pretty weird. |input| is non-const. :/

I have an idea: define a new iterator:

CustomConstIterator {
  const cc::DrawQuad* last_quad;
  cc::QuadList::ConstIterator;
};

(Because you only use this ArrayTraits for reading, those non-const
GetBegin/AdvanceIterator/... are not needed, I believe)


That way we don't need to introduce |input|.

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:559: static DrawQuadWithSharedQuadState dq;
This is pretty hacky. It won't work, for example, in multi-threaded environment.
Comparing with this, I would rather we support GetValue() returning a value (in
addition to returning a ref).

Have you considered the following alternatives:
(1) In the SetUpContext of RenderPass, const_cast the input so that we can
modify the contents, and then set |shared_quad_state| to null for quads as
needed. And in TearDownContext we restore those values. That simplifies the code
quite a bit.

(2) Or actually setup a real DrawQuadWithSharedQuadState array (and cache it as
context of the owning struct).

I wonder how the performance of those alternatives is.

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:583: struct StructTraits<cc::mojom::QuadList,
cc::QuadList> {
Is it possible that we remove cc::mojom::QuadList? (That means we change the
cc::mojom::RenderPass::quad_list's type to array<DrawQuad>.)

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/shared_quad_stat...
File cc/ipc/shared_quad_state_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/shared_quad_stat...
cc/ipc/shared_quad_state_struct_traits.h:13: struct OptSharedQuadState {
Could you use base::Optional<cc::SharedQuadState>? It will automatically use the
StructTraits<cc::mojom::SharedQuadState, cc::SharedQuadState>, so you don't need
to duplicate the code.

[yzshen1, 2016-07-26 18:00:44]

Btw, please consider adding perf tests with quad/render pass numbers that are
more typical in real-world scenarios.
That would help us to better understand the problem. WDYT?

[Fady Samuel, 2016-07-27 23:52:23]

PTAL Yuzhu! Net negative LOC! w00t!

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads.mojom
File cc/ipc/quads.mojom (right):

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads.mojom#newc...
cc/ipc/quads.mojom:131: // Index into the containing pass' shared quad state
array which has state
On 2016/07/26 17:56:03, yzshen1 wrote:
> Shall we remove this comment?

Done.

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:474: struct StructTraits<cc::mojom::DrawQuad,
cc::DrawQuad> {
On 2016/07/26 17:56:03, yzshen1 wrote:
> Is this StructTraits still needed after we introduce the one above?

No, I added a TODO to move over tests.

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:549: static void AdvanceIterator(QuadListArray&
input, ConstIterator& iterator) {
On 2016/07/26 17:56:03, yzshen1 wrote:
> This API change seems pretty weird. |input| is non-const. :/
> 
> I have an idea: define a new iterator:
> 
> CustomConstIterator {
>   const cc::DrawQuad* last_quad;
>   cc::QuadList::ConstIterator;
> };
> 
> (Because you only use this ArrayTraits for reading, those non-const
> GetBegin/AdvanceIterator/... are not needed, I believe)
> 
> 
> That way we don't need to introduce |input|.

Done.

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:559: static DrawQuadWithSharedQuadState dq;
On 2016/07/26 17:56:03, yzshen1 wrote:
> This is pretty hacky. It won't work, for example, in multi-threaded
environment.
> Comparing with this, I would rather we support GetValue() returning a value
(in
> addition to returning a ref).
> 
> Have you considered the following alternatives:
> (1) In the SetUpContext of RenderPass, const_cast the input so that we can
> modify the contents, and then set |shared_quad_state| to null for quads as
> needed. And in TearDownContext we restore those values. That simplifies the
code
> quite a bit.
> 
> (2) Or actually setup a real DrawQuadWithSharedQuadState array (and cache it
as
> context of the owning struct).
> 
> I wonder how the performance of those alternatives is. 
> 

I added support for returning Element by value.

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:583: struct StructTraits<cc::mojom::QuadList,
cc::QuadList> {
On 2016/07/26 17:56:03, yzshen1 wrote:
> Is it possible that we remove cc::mojom::QuadList? (That means we change the
> cc::mojom::RenderPass::quad_list's type to array<DrawQuad>.)

Done.

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/shared_quad_stat...
File cc/ipc/shared_quad_state_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/shared_quad_stat...
cc/ipc/shared_quad_state_struct_traits.h:13: struct OptSharedQuadState {
On 2016/07/26 17:56:03, yzshen1 wrote:
> Could you use base::Optional<cc::SharedQuadState>? It will automatically use
the
> StructTraits<cc::mojom::SharedQuadState, cc::SharedQuadState>, so you don't
need
> to duplicate the code.

I don't think I can. I don't want to create another instance of SharedQuadState.
I'm passing around a SharedQuadState pointer that is owned elsewhere.

[Fady Samuel, 2016-07-28 12:05:09]

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

[commit-bot: I haz the power, 2016-07-28 12:05:21]

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, 2016-07-28 12:05:22]

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

[commit-bot: I haz the power, 2016-07-28 12:05:23]

Dry run: Your CL was about to rely on recently removed CQ feature(s):
 * master.tryserver.blink builder linux_blink_rel was renamed to
linux_precise_blink_rel (http://crbug.com/590036)

[Fady Samuel, 2016-07-28 12:17:00]

PTAL Yuzhu! It turned out I could delete even more code!

[Fady Samuel, 2016-07-28 13:17:14]

I've added a couple more serialization/deserialization that are perhaps more
representation of real CompositorFrame sizes:
https://codereview.chromium.org/2190943002/

[Fady Samuel, 2016-07-28 13:17:34]

On 2016/07/28 13:17:14, Fady Samuel wrote:
> I've added a couple more serialization/deserialization that are perhaps more
> representation of real CompositorFrame sizes:
> https://codereview.chromium.org/2190943002/

I'll update the spreadsheet when I get in the office. I'm heading out in a bit.

[yzshen1, 2016-07-28 18:25:19]

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/shared_quad_stat...
File cc/ipc/shared_quad_state_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/20001/cc/ipc/shared_quad_stat...
cc/ipc/shared_quad_state_struct_traits.h:13: struct OptSharedQuadState {
On 2016/07/27 23:52:23, Fady Samuel wrote:
> On 2016/07/26 17:56:03, yzshen1 wrote:
> > Could you use base::Optional<cc::SharedQuadState>? It will automatically use
> the
> > StructTraits<cc::mojom::SharedQuadState, cc::SharedQuadState>, so you don't
> need
> > to duplicate the code.
> 
> I don't think I can. I don't want to create another instance of
SharedQuadState.
> I'm passing around a SharedQuadState pointer that is owned elsewhere.

It is still possible if I let T* automatically use T's StructTraits (like what I
have done for base::Optional<T>). But that is a separate improvement that I can
consider later.

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:479: struct QuadListArray {
I think we could remove this struct by changing the following ArrayTraits to
something like ArrayTraits<QuadList*> or ArrayTraits<QuadList>, right?

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:486: struct QuadListConstIterator {
This looks so much better than the previous code! :D

nit: you could directly define this as ConstIterator in the ArrayTraits below,
instead of defining it out-of-line and then adding an alias.

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:511: dq.quad =
const_cast<cc::DrawQuad*>(*iterator.it);
Why do we need a non-const pointer here?

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass.typ...
File cc/ipc/render_pass.typemap (right):

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass.typ...
cc/ipc/render_pass.typemap:22: type_mappings = [
"cc.mojom.RenderPass=std::unique_ptr<cc::RenderPass>" ]
nit: you probably want to add attributes: 
"cc.mojom.RenderPass=std::unique_ptr<cc::RenderPass>[move_only,nullable_is_same_type]"

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass_str...
File cc/ipc/render_pass_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass_str...
cc/ipc/render_pass_struct_traits.h:46:
const_cast<cc::QuadList*>(&input->quad_list), nullptr};
why do we need a non-const pointer?

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/lib/array_serialization.h (right):

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:309:
Serialize<Element>(input->GetNext(), &output->at(i), context);
We need to do
typename UserTypeIterator::GetNextResult next = input->GetNext();

here too.

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:365: typename
UserTypeIterator::GetNextResult next = input->GetNext();
Please also update the documentation of array_traits.h about the new feature.

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:461: size +=
PrepareToSerialize<Element>(input->GetNext(), false, context);
Please also do
typename UserTypeIterator::GetNextResult next = input->GetNext();

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:474:
Serialize<Element>(input->GetNext(), buf, &result, true, context);
typename UserTypeIterator::GetNextResult next = input->GetNext();

[Fady Samuel, 2016-07-28 19:53:32]

PTAL Yuzhu!

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:479: struct QuadListArray {
On 2016/07/28 18:25:19, yzshen1 wrote:
> I think we could remove this struct by changing the following ArrayTraits to
> something like ArrayTraits<QuadList*> or ArrayTraits<QuadList>, right?

Done.

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:486: struct QuadListConstIterator {
On 2016/07/28 18:25:19, yzshen1 wrote:
> This looks so much better than the previous code! :D
> 
> nit: you could directly define this as ConstIterator in the ArrayTraits below,
> instead of defining it out-of-line and then adding an alias.

Done.

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:511: dq.quad =
const_cast<cc::DrawQuad*>(*iterator.it);
On 2016/07/28 18:25:19, yzshen1 wrote:
> Why do we need a non-const pointer here?

Done.

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass.typ...
File cc/ipc/render_pass.typemap (right):

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass.typ...
cc/ipc/render_pass.typemap:22: type_mappings = [
"cc.mojom.RenderPass=std::unique_ptr<cc::RenderPass>" ]
On 2016/07/28 18:25:19, yzshen1 wrote:
> nit: you probably want to add attributes: 
>
"cc.mojom.RenderPass=std::unique_ptr<cc::RenderPass>[move_only,nullable_is_same_type]"

Done.

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass_str...
File cc/ipc/render_pass_struct_traits.h (right):

https://codereview.chromium.org/2174843003/diff/100001/cc/ipc/render_pass_str...
cc/ipc/render_pass_struct_traits.h:46:
const_cast<cc::QuadList*>(&input->quad_list), nullptr};
On 2016/07/28 18:25:19, yzshen1 wrote:
> why do we need a non-const pointer?

Done.

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/lib/array_serialization.h (right):

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:309:
Serialize<Element>(input->GetNext(), &output->at(i), context);
On 2016/07/28 18:25:19, yzshen1 wrote:
> We need to do
> typename UserTypeIterator::GetNextResult next = input->GetNext();
> 
> here too.

Done.

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:365: typename
UserTypeIterator::GetNextResult next = input->GetNext();
On 2016/07/28 18:25:19, yzshen1 wrote:
> Please also update the documentation of array_traits.h about the new feature.

Done.

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:461: size +=
PrepareToSerialize<Element>(input->GetNext(), false, context);
On 2016/07/28 18:25:19, yzshen1 wrote:
> Please also do
> typename UserTypeIterator::GetNextResult next = input->GetNext();

Done.

https://codereview.chromium.org/2174843003/diff/100001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/lib/array_serialization.h:474:
Serialize<Element>(input->GetNext(), buf, &result, true, context);
On 2016/07/28 18:25:19, yzshen1 wrote:
> typename UserTypeIterator::GetNextResult next = input->GetNext();

Done.

[yzshen1, 2016-07-28 21:14:29]

LGTM!

[Fady Samuel, 2016-07-28 22:25:12]

fsamuel@chromium.org changed reviewers:
+ tsepez@chromium.org

[Fady Samuel, 2016-07-28 22:25:13]

+tsepez@ for security

[Tom Sepez, 2016-07-28 22:47:19]

Per conversation with fsamuel, we should check that the first drawquad should
have a non-null sqs.

Otherwise LGTM.

[Tom Sepez, 2016-07-28 22:56:04]

++lgtm

[Fady Samuel, 2016-07-28 22:56:27]

fsamuel@chromium.org changed reviewers:
+ danakj@chromium.org

[Fady Samuel, 2016-07-28 22:56:28]

TBR'ing danakj@ for:

cc/ipc/quads.typemap
cc/ipc/render_pass.typemap
cc/ipc/struct_traits_unittest.cc
cc/ipc/typemaps.gni

These are mojo specific files which Yuzhu has already reviewed but is not OWNER
in cc.

[Fady Samuel, 2016-07-28 22:56:49]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
TBR=danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-07-28 22:56:56]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-07-28 22:56:56]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2174843003/#ps160001
(title: "Better solution")

[commit-bot: I haz the power, 2016-07-28 22:57:26]

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, 2016-07-28 22:57:27]

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

[commit-bot: I haz the power, 2016-07-28 22:57:29]

Your CL was about to rely on recently removed CQ feature(s):
 * master.tryserver.blink builder linux_blink_rel was renamed to
linux_precise_blink_rel (http://crbug.com/590036)

[danakj, 2016-07-28 23:37:12]

On Thu, Jul 28, 2016 at 3:56 PM, <fsamuel@chromium.org> wrote:

> TBR'ing danakj@ for:
>
> cc/ipc/quads.typemap
> cc/ipc/render_pass.typemap
> cc/ipc/struct_traits_unittest.cc
> cc/ipc/typemaps.gni
>
> These are mojo specific files which Yuzhu has already reviewed but is not
> OWNER
> in cc.
>

Should Yuzhu be an owner of them then? Cuz that's not a reason to use TBR.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2016-07-28 23:37:51]

LGTM

https://codereview.chromium.org/2174843003/diff/160001/cc/ipc/struct_traits_u...
File cc/ipc/struct_traits_unittest.cc (right):

https://codereview.chromium.org/2174843003/diff/160001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:415: QuadList input;
unused?

[yzshen1, 2016-07-28 23:42:07]

Maybe it is okay to let mojo OWNERs be also owners for all *.typemap files?
I feel that it is still useful to let the domain experts review things like
struct_traits_unittest.cc.

On Thu, Jul 28, 2016 at 4:36 PM, <danakj@chromium.org> wrote:

> On Thu, Jul 28, 2016 at 3:56 PM, <fsamuel@chromium.org> wrote:
>
>> TBR'ing danakj@ for:
>>
>> cc/ipc/quads.typemap
>> cc/ipc/render_pass.typemap
>> cc/ipc/struct_traits_unittest.cc
>> cc/ipc/typemaps.gni
>>
>> These are mojo specific files which Yuzhu has already reviewed but is not
>> OWNER
>> in cc.
>>
>
> Should Yuzhu be an owner of them then? Cuz that's not a reason to use TBR.
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[yzshen1, 2016-07-28 23:43:17]

On Thu, Jul 28, 2016 at 4:41 PM, Yuzhu Shen <yzshen@chromium.org> wrote:

> Maybe it is okay to let mojo OWNERs be also owners for all *.typemap files?
>
Oh wait, I have a better idea:

The security OWNERs need to approve mojom and struct traits files. Maybe
they could be owners for *.typemap as well. These are also ipc-related.


> I feel that it is still useful to let the domain experts review things
> like struct_traits_unittest.cc.
>
> On Thu, Jul 28, 2016 at 4:36 PM, <danakj@chromium.org> wrote:
>
>> On Thu, Jul 28, 2016 at 3:56 PM, <fsamuel@chromium.org> wrote:
>>
>>> TBR'ing danakj@ for:
>>>
>>> cc/ipc/quads.typemap
>>> cc/ipc/render_pass.typemap
>>> cc/ipc/struct_traits_unittest.cc
>>> cc/ipc/typemaps.gni
>>>
>>> These are mojo specific files which Yuzhu has already reviewed but is
>>> not OWNER
>>> in cc.
>>>
>>
>> Should Yuzhu be an owner of them then? Cuz that's not a reason to use TBR.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Fady Samuel, 2016-07-29 00:34:11]

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

[commit-bot: I haz the power, 2016-07-29 00:34:30]

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, 2016-07-29 00:34:31]

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

[commit-bot: I haz the power, 2016-07-29 00:34:33]

Dry run: Your CL was about to rely on recently removed CQ feature(s):
 * master.tryserver.blink builder linux_blink_rel was renamed to
linux_precise_blink_rel (http://crbug.com/590036)

[Fady Samuel, 2016-07-29 10:55:02]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-07-29 10:55:03]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
danakj@chromium.org, yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2174843003/#ps180001
(title: "Addressed nit")

[commit-bot: I haz the power, 2016-07-29 10:55:22]

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, 2016-07-29 10:55:23]

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

[commit-bot: I haz the power, 2016-07-29 10:55:25]

Your CL was about to rely on recently removed CQ feature(s):
 * master.tryserver.blink builder linux_blink_rel was renamed to
linux_precise_blink_rel (http://crbug.com/590036)

[Fady Samuel, 2016-07-29 10:57:17]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-07-29 10:57:29]

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, 2016-07-29 10:57:30]

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

[commit-bot: I haz the power, 2016-07-29 10:57:32]

Your CL was about to rely on recently removed CQ feature(s):
 * master.tryserver.blink builder linux_blink_rel was renamed to
linux_precise_blink_rel (http://crbug.com/590036)

[Fady Samuel, 2016-07-29 11:02:54]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-07-29 11:03:08]

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, 2016-07-29 11:03:09]

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

[commit-bot: I haz the power, 2016-07-29 11:03:11]

Your CL was about to rely on recently removed CQ feature(s):
 * master.tryserver.blink builder linux_blink_rel was renamed to
linux_precise_blink_rel (http://crbug.com/590036)

[Fady Samuel, 2016-07-29 15:38:46]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-07-29 15:39:09]

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, 2016-07-29 15:39:10]

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

[commit-bot: I haz the power, 2016-07-29 15:39:12]

Your CL was about to rely on recently removed CQ feature(s):
 * master.tryserver.blink builder linux_blink_rel was renamed to
linux_precise_blink_rel (http://crbug.com/590036)

[Fady Samuel, 2016-07-29 15:55:03]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
TBR=danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
TBR=danakj@chromium.org
==========

[Fady Samuel, 2016-07-29 15:55:12]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-07-29 15:55:29]

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, 2016-07-29 16:08:56]

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

[commit-bot: I haz the power, 2016-07-29 16:08:57]

Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Fady Samuel, 2016-07-29 16:57:23]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-07-29 16:57:41]

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, 2016-07-29 17:12:58]

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

[commit-bot: I haz the power, 2016-07-29 17:13:00]

Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Fady Samuel, 2016-07-29 20:06:44]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
TBR=danakj@chromium.org
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
TBR=danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Fady Samuel, 2016-07-29 20:11:00]

Heh, I guess move_only as a keyword in the typemap did nothing on my local
build. The build broke applied to ToT because EchoRenderPass was taking a const
ref of std::unique_ptr<RenderPass> instead of std::move'ing into EchoRenderPass.
 I had to modify the RenderPass unit test slightly so that it creates a copy of
the RenderPass prior to moving it into EchoRenderPass so that I can continue to
compare against the built up input RenderPass. In a couple of cases, I added a
constant for size instead of reading the size of the input in other tests to
unbreak the tests. Any CQ objections, Dana?

[Fady Samuel, 2016-07-29 20:11:41]

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

[commit-bot: I haz the power, 2016-07-29 20:12:13]

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

[danakj, 2016-07-29 20:35:57]

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
File cc/ipc/struct_traits_unittest.cc (right):

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:488: ASSERT_EQ(6u, output->quad_list.size());
store this before moving it rather than hardcoding?

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:607: RenderPassList pass_list;
This is pretty roundabout to make a copy of a RenderPass.

Can you either
- leave a comment explaining that this is deep-copying the |input| render pass
into |input_echo|.
- make a helper function with a name that explains that and call it
- split RenderPass::CopyAll into RenderPass::Copy which CopyAll calls, and use
that.

Or.. alternatively.. is it possible to have EchoRenderPass return ownership of
the input?

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:621: EXPECT_EQ(3u, output->quad_list.size());
save it here also?

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:901: ASSERT_EQ(1u, output->quad_list.size());
ditto, save it?

[Fady Samuel, 2016-07-29 21:06:32]

PTAL Dana! Thanks!

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
File cc/ipc/struct_traits_unittest.cc (right):

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:488: ASSERT_EQ(6u, output->quad_list.size());
On 2016/07/29 20:35:57, danakj wrote:
> store this before moving it rather than hardcoding?

Restored original code.

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:607: RenderPassList pass_list;
On 2016/07/29 20:35:57, danakj wrote:
> This is pretty roundabout to make a copy of a RenderPass.
> 
> Can you either
> - leave a comment explaining that this is deep-copying the |input| render pass
> into |input_echo|.
> - make a helper function with a name that explains that and call it
> - split RenderPass::CopyAll into RenderPass::Copy which CopyAll calls, and use
> that.
> 
> Or.. alternatively.. is it possible to have EchoRenderPass return ownership of
> the input?

I agree. I hate it too :P I introduced RenderPass::DeepCopy. I now pass
EchoRenderPass a DeepCopy instead and most code remains unchanged! Thanks!

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:621: EXPECT_EQ(3u, output->quad_list.size());
On 2016/07/29 20:35:57, danakj wrote:
> save it here also?

Restored original.

https://codereview.chromium.org/2174843003/diff/200001/cc/ipc/struct_traits_u...
cc/ipc/struct_traits_unittest.cc:901: ASSERT_EQ(1u, output->quad_list.size());
On 2016/07/29 20:35:57, danakj wrote:
> ditto, save it?

Restored.

[Fady Samuel, 2016-07-29 21:13:16]

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

[commit-bot: I haz the power, 2016-07-29 21:13:42]

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

[danakj, 2016-07-29 21:15:06]

LGTM

[commit-bot: I haz the power, 2016-07-29 22:00:32]

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

[commit-bot: I haz the power, 2016-07-29 22:00:34]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Fady Samuel, 2016-07-29 22:18:42]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-07-29 22:18:42]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
yzshen@chromium.org, danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2174843003/#ps240001
(title: "Fix RenderPassId")

[commit-bot: I haz the power, 2016-07-29 22:19:10]

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, 2016-07-29 23:58:37]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-07-30 00:00:26]

Description was changed from

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of 
18% on an HP Z620. Deserialization performance improves 
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews 
in parent StructTraits, we had to do extra work at 
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of 
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization. 
StructTraits assume a preallocated type and populated 
fields in that type. 

Alternatively we could have deserialized DrawQuads into 
unique_ptrs, but cc required that we allocate DrawQuads 
inline within a buffer in order to minimize the cost of 
allocations (there can be hundreds of DrawQuads in a 
CompositorFrame).

2. Previously, SharedQuadStates were serialized 
independently of DrawQuads. However, in in-memory form, 
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize 
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in 
the SharedQuadStateList. After the QuadList and SharedQuadStateList were 
serialized,  shared_quad_state_references updated the raw 
pointers in the DrawQuads in the quad list to refer to the 
SharedQuadStates. This required revisiting all deserialized 
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...


BUG=624459
TBR=danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc mojo: Use ArrayDataViews in RenderPasses

This CL makes use of the newly introduced Array DataViews
in cc StructTraits in order to improve serialization (and
to a lesser degree deserialization) performance.

This CL improves serialization performance by an average of
18% on an HP Z620. Deserialization performance improves
between 7 to 10%.

Prior to the ability to inspect Array and Struct DataViews
in parent StructTraits, we had to do extra work at
serialization time in order to capture state that spans
individual structs within a larger struct.

This CL addresses two of those cases:

1. Previously, QuadList maintained an extra array of
materials. This array existed so that we could preallocate
the appropriately typed DrawQuads during deserialization.
StructTraits assume a preallocated type and populated
fields in that type.

Alternatively we could have deserialized DrawQuads into
unique_ptrs, but cc required that we allocate DrawQuads
inline within a buffer in order to minimize the cost of
allocations (there can be hundreds of DrawQuads in a
CompositorFrame).

2. Previously, SharedQuadStates were serialized
independently of DrawQuads. However, in in-memory form,
DrawQuads refer to SharedQuadStates via raw pointers. In
order to allow DrawQuads and SharedQuadStates to deserialize
independently, as was required previously by StructTraits, a third
array was serialized "shared_quad_state_references" which was equal in
size to the QuadList. Each slot corresponded to an index in
the SharedQuadStateList. After the QuadList and SharedQuadStateList were
serialized,  shared_quad_state_references updated the raw
pointers in the DrawQuads in the quad list to refer to the
SharedQuadStates. This required revisiting all deserialized
DrawQuads again.

In both 1. and 2. additional state needed to be allocated on the heap
during serialization. In pprof profiling, it became apparent that much
of the cost of serialization was in SetupContext. This CL eliminates
that cost.

Performance measures are available here:
https://docs.google.com/a/google.com/spreadsheets/d/13Y0I-BVDrQC4RHIzMSs0vl0Q...

BUG=624459
TBR=danakj@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/edd40fdc29170e97ab54578fdc5096fc564c0ccb
Cr-Commit-Position: refs/heads/master@{#408809}
==========

[commit-bot: I haz the power, 2016-07-30 00:00:28]

Patchset 13 (id:??) landed as
https://crrev.com/edd40fdc29170e97ab54578fdc5096fc564c0ccb
Cr-Commit-Position: refs/heads/master@{#408809}