Update dbus_statistics to use C++11 more effectively, remove obsolete stl_util use.

Update dbus_statistics to use C++11 more effectively, remove obsolete stl_util use.

BUG=555865
Committed: https://crrev.com/80df07371603411c8359c8ff81237b52a18cd1d7
Cr-Commit-Position: refs/heads/master@{#425105}

[Avi (use Gerrit), 2016-10-10 01:24:55]

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

[commit-bot: I haz the power, 2016-10-10 01:25: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-10-10 01:36:31]

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

[commit-bot: I haz the power, 2016-10-10 01:36:31]

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

[Avi (use Gerrit), 2016-10-10 01:46:24]

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

[commit-bot: I haz the power, 2016-10-10 01:46:31]

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-10-10 02:24:22]

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

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

Dry run: This issue passed the CQ dry run.

[Avi (use Gerrit), 2016-10-10 02:28:27]

avi@chromium.org changed reviewers:
+ satorux@chromium.org

[satorux1, 2016-10-11 01:29:57]

satorux@chromium.org changed reviewers:
+ stevenjb@chromium.org

[satorux1, 2016-10-11 01:29:58]

+stevenjb who wrote the code.

https://codereview.chromium.org/2402893002/diff/20001/dbus/dbus_statistics.cc
File dbus/dbus_statistics.cc (right):

https://codereview.chromium.org/2402893002/diff/20001/dbus/dbus_statistics.cc...
dbus/dbus_statistics.cc:51: using StatSet = std::map<Stat*,
std::unique_ptr<Stat>, Stat::PtrCompare>;
why was it changed from set to map?

if it's a map something like below may be easier to understand:

map<tuple<string, string, string>, Stat> 

where 'Stat' does no have include service, interface, method.

[Avi (use Gerrit), 2016-10-11 15:38:47]

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

[commit-bot: I haz the power, 2016-10-11 15:39:12]

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-10-11 15:50:09]

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

[commit-bot: I haz the power, 2016-10-11 15:50:10]

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

[Avi (use Gerrit), 2016-10-11 15:56:48]

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

[commit-bot: I haz the power, 2016-10-11 15:57:03]

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

[Avi (use Gerrit), 2016-10-11 16:31:46]

ptal

https://codereview.chromium.org/2402893002/diff/20001/dbus/dbus_statistics.cc
File dbus/dbus_statistics.cc (right):

https://codereview.chromium.org/2402893002/diff/20001/dbus/dbus_statistics.cc...
dbus/dbus_statistics.cc:51: using StatSet = std::map<Stat*,
std::unique_ptr<Stat>, Stat::PtrCompare>;
On 2016/10/11 01:29:58, satorux1 wrote:
> why was it changed from set to map?
> 
> if it's a map something like below may be easier to understand:
> 
> map<tuple<string, string, string>, Stat> 
> 
> where 'Stat' does no have include service, interface, method.

In doing the conversions I've been switching because you lose the ability to do
a find() on a set<unique_ptr> as well as the ability to remove items.

Switching to a map that way makes sense.

[commit-bot: I haz the power, 2016-10-11 16:47:12]

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

[commit-bot: I haz the power, 2016-10-11 16:47:12]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2016-10-13 17:34:11]

On 2016/10/11 16:31:46, Avi wrote:
> ptal
> 
> https://codereview.chromium.org/2402893002/diff/20001/dbus/dbus_statistics.cc
> File dbus/dbus_statistics.cc (right):
> 
>
https://codereview.chromium.org/2402893002/diff/20001/dbus/dbus_statistics.cc...
> dbus/dbus_statistics.cc:51: using StatSet = std::map<Stat*,
> std::unique_ptr<Stat>, Stat::PtrCompare>;
> On 2016/10/11 01:29:58, satorux1 wrote:
> > why was it changed from set to map?
> > 
> > if it's a map something like below may be easier to understand:
> > 
> > map<tuple<string, string, string>, Stat> 
> > 
> > where 'Stat' does no have include service, interface, method.
> 
> In doing the conversions I've been switching because you lose the ability to
do
> a find() on a set<unique_ptr> as well as the ability to remove items.
> 
> Switching to a map that way makes sense.

I think a map here makes more sense. A single struct was convenient with the
helpers we had at the time, but I like this better. I also prefer using a struct
for the key vs a tuple.

LGTM with one suggestion.

[stevenjb, 2016-10-13 17:34:28]

https://codereview.chromium.org/2402893002/diff/60001/dbus/dbus_statistics.cc
File dbus/dbus_statistics.cc (right):

https://codereview.chromium.org/2402893002/diff/60001/dbus/dbus_statistics.cc...
dbus/dbus_statistics.cc:31: struct StatValues {
nit: StatValues sounds like a list or set, maybe just StatValue?

https://codereview.chromium.org/2402893002/diff/60001/dbus/dbus_statistics.cc...
dbus/dbus_statistics.cc:181: const StatValues& stat_values = cur_iter->second;
nit: stat_value

[Avi (use Gerrit), 2016-10-13 17:44:56]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2016-10-13 17:44:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2402893002/#ps80001
(title: "nits")

[commit-bot: I haz the power, 2016-10-13 17:45:21]

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

[Avi (use Gerrit), 2016-10-13 18:07:03]

https://codereview.chromium.org/2402893002/diff/60001/dbus/dbus_statistics.cc
File dbus/dbus_statistics.cc (right):

https://codereview.chromium.org/2402893002/diff/60001/dbus/dbus_statistics.cc...
dbus/dbus_statistics.cc:31: struct StatValues {
On 2016/10/13 17:34:28, stevenjb wrote:
> nit: StatValues sounds like a list or set, maybe just StatValue?

Done.

https://codereview.chromium.org/2402893002/diff/60001/dbus/dbus_statistics.cc...
dbus/dbus_statistics.cc:181: const StatValues& stat_values = cur_iter->second;
On 2016/10/13 17:34:28, stevenjb wrote:
> nit: stat_value

"stat" to match other places.

[commit-bot: I haz the power, 2016-10-13 18:46:16]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-13 18:47:38]

Description was changed from

==========
Update dbus_statistics to use C++11 more effectively, remove obsolete stl_util
use.

BUG=555865
==========

to

==========
Update dbus_statistics to use C++11 more effectively, remove obsolete stl_util
use.

BUG=555865

Committed: https://crrev.com/80df07371603411c8359c8ff81237b52a18cd1d7
Cr-Commit-Position: refs/heads/master@{#425105}
==========

[commit-bot: I haz the power, 2016-10-13 18:47:39]

Patchset 5 (id:??) landed as
https://crrev.com/80df07371603411c8359c8ff81237b52a18cd1d7
Cr-Commit-Position: refs/heads/master@{#425105}