Switch cc::RollingTimeDeltaHistory to use a vector

Switch cc::RollingTimeDeltaHistory to use a vector

BUG=625351
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/9d71ccf7550bfaed0cd64cd168b55471fdfc8330
Cr-Commit-Position: refs/heads/master@{#404899}

[chongjiang, 2016-07-07 00:53:40]

Description was changed from

==========
Switch cc::RollingTimeDeltaHistory to use a vector

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

to

==========
Switch cc::RollingTimeDeltaHistory to use a vector

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

[chongjiang, 2016-07-07 01:00:37]

Description was changed from

==========
Switch cc::RollingTimeDeltaHistory to use a vector

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

to

==========
Switch cc::RollingTimeDeltaHistory to use a vector

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

[chongjiang, 2016-07-07 01:01:10]

Patchset #1 (id:1) has been deleted

[chongjiang, 2016-07-07 17:34:45]

chongjiang@chromium.org changed reviewers:
+ danakj@chromium.org, dhsharp@chromium.org

[dhsharp, 2016-07-12 19:56:23]

lgtm, but owners should review.

[danakj, 2016-07-12 20:08:43]

LGTM

https://codereview.chromium.org/2127003002/diff/40001/cc/base/rolling_time_de...
File cc/base/rolling_time_delta_history.cc (right):

https://codereview.chromium.org/2127003002/diff/40001/cc/base/rolling_time_de...
cc/base/rolling_time_delta_history.cc:35: sample_vector_.clear();
should we reserve() after this again? it's not clear to me that this would
preserve the capacity. or can you verify that it does and leave a comment?

[dhsharp, 2016-07-12 20:25:01]

https://codereview.chromium.org/2127003002/diff/40001/cc/base/rolling_time_de...
File cc/base/rolling_time_delta_history.cc (right):

https://codereview.chromium.org/2127003002/diff/40001/cc/base/rolling_time_de...
cc/base/rolling_time_delta_history.cc:35: sample_vector_.clear();
On 2016/07/12 20:08:43, danakj wrote:
> should we reserve() after this again? it's not clear to me that this would
> preserve the capacity. or can you verify that it does and leave a comment?

http://en.cppreference.com/w/cpp/container/vector/clear
"Leaves the capacity() of the vector unchanged"

So assuming that site is correct, and the libs follow the standard, it should be
fine.

[danakj, 2016-07-12 20:26:56]

On Tue, Jul 12, 2016 at 1:25 PM, <dhsharp@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2127003002/diff/40001/cc/base/rolling_time_de...
> File cc/base/rolling_time_delta_history.cc (right):
>
>
>
https://codereview.chromium.org/2127003002/diff/40001/cc/base/rolling_time_de...
> cc/base/rolling_time_delta_history.cc:35: sample_vector_.clear();
> On 2016/07/12 20:08:43, danakj wrote:
> > should we reserve() after this again? it's not clear to me that this
> would
> > preserve the capacity. or can you verify that it does and leave a
> comment?
>
> http://en.cppreference.com/w/cpp/container/vector/clear
> "Leaves the capacity() of the vector unchanged"
>
> So assuming that site is correct, and the libs follow the standard, it
> should be fine.
>

Oh awesome. http://www.cplusplus.com/reference/vector/vector/clear/ was
must less "clear" about it.

-- 
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.

[chongjiang, 2016-07-12 20:45:27]

The CQ bit was checked by chongjiang@chromium.org

[commit-bot: I haz the power, 2016-07-12 20:47:17]

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-13 02:16:47]

Description was changed from

==========
Switch cc::RollingTimeDeltaHistory to use a vector

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

to

==========
Switch cc::RollingTimeDeltaHistory to use a vector

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

[commit-bot: I haz the power, 2016-07-13 02:16:48]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-07-13 02:19:17]

Description was changed from

==========
Switch cc::RollingTimeDeltaHistory to use a vector

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

to

==========
Switch cc::RollingTimeDeltaHistory to use a vector

BUG=625351
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/9d71ccf7550bfaed0cd64cd168b55471fdfc8330
Cr-Commit-Position: refs/heads/master@{#404899}
==========

[commit-bot: I haz the power, 2016-07-13 02:19:18]

Patchset 2 (id:??) landed as
https://crrev.com/9d71ccf7550bfaed0cd64cd168b55471fdfc8330
Cr-Commit-Position: refs/heads/master@{#404899}

[chongjiang, 2016-07-19 23:02:52]

A revert of this CL (patchset #2 id:40001) has been created in
https://codereview.chromium.org/2161113004/ by chongjiang@chromium.org.

The reason for reverting is: Performance regression:
https://bugs.chromium.org/p/chromium/issues/detail?id=629349.