Defer allocation in cc::ContiguousContainerBase::Buffer.

Defer allocation in cc::ContiguousContainerBase::Buffer.

This CL optimizes memory usage of cc::ContiguousContainer by deferring memory
allocation in ContiguousContainerBase::Buffer until the first Allocate() call.
Previously memory was allocated in the Buffer constructor, and sometimes was
never written to (see the bug for more info).

BUG=635142
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[Dmitry Skiba, 2016-08-23 18:01:22]

Description was changed from

==========
Defer allocation in cc::ContiguousContainerBase::Buffer.

This CL optimizes memory usage of cc::ContiguousContainer by deferring memory
allocation in ContiguousContainerBase::Buffer until the first Allocate() call.
Previously memory was allocated in the Buffer constructor, and sometimes was
never written to (see the bug for more info).

BUG=635142
==========

to

==========
Defer allocation in cc::ContiguousContainerBase::Buffer.

This CL optimizes memory usage of cc::ContiguousContainer by deferring memory
allocation in ContiguousContainerBase::Buffer until the first Allocate() call.
Previously memory was allocated in the Buffer constructor, and sometimes was
never written to (see the bug for more info).

BUG=635142
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Dmitry Skiba, 2016-08-23 18:03:22]

dskiba@google.com changed reviewers:
+ danakj@chromium.org

[Dmitry Skiba, 2016-08-23 18:03:45]

The CQ bit was checked by dskiba@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 18:04:27]

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

[danakj, 2016-08-23 18:16:00]

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
File cc/base/contiguous_container.cc (right):

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
cc/base/contiguous_container.cc:84: for (const auto& buffer : buffers_) {
nit: no {}

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
File cc/base/contiguous_container_unittest.cc (right):

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
cc/base/contiguous_container_unittest.cc:513: 
Can you add a test that verifies we can use begin() and end() to iterate over
the empty container before anything was allocated? It should iterate 0 times and
not crash.

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
cc/base/contiguous_container_unittest.cc:525:
EXPECT_EQ(list1.MemoryUsageInBytes(), list2.MemoryUsageInBytes());
You could check LT initial_size here?

[commit-bot: I haz the power, 2016-08-23 19:07:25]

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

[commit-bot: I haz the power, 2016-08-23 19:07:26]

Dry run: This issue passed the CQ dry run.

[Dmitry Skiba, 2016-08-24 17:58:46]

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
File cc/base/contiguous_container.cc (right):

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
cc/base/contiguous_container.cc:84: for (const auto& buffer : buffers_) {
On 2016/08/23 18:16:00, danakj wrote:
> nit: no {}

Done.

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
File cc/base/contiguous_container_unittest.cc (right):

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
cc/base/contiguous_container_unittest.cc:513: 
On 2016/08/23 18:16:00, danakj wrote:
> Can you add a test that verifies we can use begin() and end() to iterate over
> the empty container before anything was allocated? It should iterate 0 times
and
> not crash.

Done.

https://codereview.chromium.org/2277433002/diff/1/cc/base/contiguous_containe...
cc/base/contiguous_container_unittest.cc:525:
EXPECT_EQ(list1.MemoryUsageInBytes(), list2.MemoryUsageInBytes());
On 2016/08/23 18:16:00, danakj wrote:
> You could check LT initial_size here?

I think that checking that would make us depend on internal implementation
details. For example in the constructor we append one element to 
ContiguousContainer::elements_, and that pushes memory usage to some
implementation-defined levels (elements_.capacity()).

I did change the checks to make sure memory usage grows by at least initial_size
though.

[danakj, 2016-08-24 20:05:00]

LGTM thanks!

[Dmitry Skiba, 2016-08-24 20:31:17]

The CQ bit was checked by dskiba@google.com

[commit-bot: I haz the power, 2016-08-24 20:33:00]

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-08-24 22:57:34]

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

[commit-bot: I haz the power, 2016-08-24 22:57:35]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Dmitry Skiba, 2016-08-24 23:01:18]

The CQ bit was checked by dskiba@google.com

[commit-bot: I haz the power, 2016-08-24 23:02:54]

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-08-24 23:53:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-24 23:55:17]

Description was changed from

==========
Defer allocation in cc::ContiguousContainerBase::Buffer.

This CL optimizes memory usage of cc::ContiguousContainer by deferring memory
allocation in ContiguousContainerBase::Buffer until the first Allocate() call.
Previously memory was allocated in the Buffer constructor, and sometimes was
never written to (see the bug for more info).

BUG=635142
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Defer allocation in cc::ContiguousContainerBase::Buffer.

This CL optimizes memory usage of cc::ContiguousContainer by deferring memory
allocation in ContiguousContainerBase::Buffer until the first Allocate() call.
Previously memory was allocated in the Buffer constructor, and sometimes was
never written to (see the bug for more info).

BUG=635142
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[commit-bot: I haz the power, 2016-08-24 23:55:18]

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