Replace std::deque usage in drp_event_store with a circular buffer.

Replace std::deque usage in drp_event_store with a circular buffer.

Since std::deque is memory inefficient for small numbers of elements,
especially on Android, (see http://crbug.com/674287), this CL replaces
the std::deque in DataReductionProxyEventStore with a simple circular
buffer.

BUG=679603
Review-Url: https://codereview.chromium.org/2619373003
Cr-Commit-Position: refs/heads/master@{#443073}
Committed: https://chromium.googlesource.com/chromium/src/+/c37c6180c36f4c69be3bdccafbbc2a2bee556ed7

[sclittle, 2017-01-10 03:44:40]

Patchset #1 (id:1) has been deleted

[sclittle, 2017-01-10 03:44:55]

sclittle@chromium.org changed reviewers:
+ tbansal@chromium.org

[tbansal1, 2017-01-10 17:42:23]

Do you think we should wait a little bit to see if there is a drop-in
replacement added to //base. The discussion is going on in
https://bugs.chromium.org/p/chromium/issues/detail?id=674287.

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
(right):

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:139:
auto events_list = base::MakeUnique<base::ListValue>();
DCHECK(first_event_index_ == 0 || stored_events_.size() == kMaxEventsToStore);

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:155:
}
DCHECK_EQ(kMaxEventsToStore, stored_events_.size());

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h
(right):

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h:77:
size_t first_event_index_;
call it oldest_event_index_?

[sclittle, 2017-01-11 22:08:32]

We could wait for a drop in replacement, but hopefully changing it to a circular
buffer like this is simple enough to land anyways, plus it's more efficient than
a drop-in replacement would be anyways.

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
(right):

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:139:
auto events_list = base::MakeUnique<base::ListValue>();
On 2017/01/10 17:42:23, tbansal1 wrote:
> DCHECK(first_event_index_ == 0 || stored_events_.size() == kMaxEventsToStore);

Done.

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:155:
}
On 2017/01/10 17:42:23, tbansal1 wrote:
> DCHECK_EQ(kMaxEventsToStore, stored_events_.size());

Done.

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h
(right):

https://codereview.chromium.org/2619373003/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h:77:
size_t first_event_index_;
On 2017/01/10 17:42:23, tbansal1 wrote:
> call it oldest_event_index_?

Done.

[tbansal1, 2017-01-11 22:16:16]

lgtm % optional nit.

https://codereview.chromium.org/2619373003/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
(right):

https://codereview.chromium.org/2619373003/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:8:
#include <stdint.h>
nit, feel free to ignore: While you are cleaning up, stdint.h is not needed here
either (already there in header file).

[sclittle, 2017-01-11 22:47:10]

https://codereview.chromium.org/2619373003/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
(right):

https://codereview.chromium.org/2619373003/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:8:
#include <stdint.h>
On 2017/01/11 22:16:15, tbansal1 wrote:
> nit, feel free to ignore: While you are cleaning up, stdint.h is not needed
here
> either (already there in header file).

Done.

[sclittle, 2017-01-11 22:47:19]

The CQ bit was checked by sclittle@chromium.org

[sclittle, 2017-01-11 22:47:20]

The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2619373003/#ps60001
(title: "removed unneeded includes from .cc file")

[commit-bot: I haz the power, 2017-01-11 22:48: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, 2017-01-12 00:17:44]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484174839755140,
"parent_rev": "90d7824a5136d081090646b7ec7f7158aaae8aca", "commit_rev":
"c37c6180c36f4c69be3bdccafbbc2a2bee556ed7"}

[commit-bot: I haz the power, 2017-01-12 00:18:36]

Description was changed from

==========
Replace std::deque usage in drp_event_store with a circular buffer.

Since std::deque is memory inefficient for small numbers of elements,
especially on Android, (see http://crbug.com/674287), this CL replaces
the std::deque in DataReductionProxyEventStore with a simple circular
buffer.

BUG=679603
==========

to

==========
Replace std::deque usage in drp_event_store with a circular buffer.

Since std::deque is memory inefficient for small numbers of elements,
especially on Android, (see http://crbug.com/674287), this CL replaces
the std::deque in DataReductionProxyEventStore with a simple circular
buffer.

BUG=679603

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

[commit-bot: I haz the power, 2017-01-12 00:18:37]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/c37c6180c36f4c69be3bdccafbbc...