Extended the ProtoDatabase to provide LoadKeys() functionality.

Extended the ProtoDatabase to provide LoadKeys() functionality.

Before this change, the only way to scan all keys was to call LoadEntries() which loads the whole database. In our use case, this means copying a lot of image data which is wasteful.
With LoadKeys(), we still need to iterate the whole database but we don't have to copy out (and parse) the values -- just the keys. LoadKeys() is useful when storing relatively large values and only access to the keys is required.

We'll use LoadKeys() for NTP content suggestions to garbage collect data. We do that at times when we have a list of all still alive elements and need to intersect that with the elements stored in the db.

BUG=649009
Committed: https://crrev.com/108e9c223a70d6dc4604354ab5afb0b42b28d30f
Cr-Commit-Position: refs/heads/master@{#422772}

[tschumann, 2016-09-29 22:13:23]

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

[commit-bot: I haz the power, 2016-09-29 22:13:46]

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

[tschumann, 2016-09-29 22:17:41]

Description was changed from

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

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

to

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

We'll use this for NTP content suggestions to garbage collect data. We do that
at times when we have a list of all still alive elements and need to intersect
that with the elements stored in the db..

BUG=649009
==========

[tschumann, 2016-09-29 22:17:41]

tschumann@chromium.org changed reviewers:
+ nyquist@chromium.org

[commit-bot: I haz the power, 2016-09-29 22:24:16]

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

[commit-bot: I haz the power, 2016-09-29 22:24:17]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_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_...)

[tschumann, 2016-09-29 22:30:17]

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

[commit-bot: I haz the power, 2016-09-29 22:30:49]

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

[tschumann, 2016-09-29 22:31:30]

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

[commit-bot: I haz the power, 2016-09-29 22:32:10]

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-09-29 22:38:46]

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

[commit-bot: I haz the power, 2016-09-29 22:38:46]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[tschumann, 2016-09-30 11:46:29]

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

[commit-bot: I haz the power, 2016-09-30 11:46:45]

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-09-30 12:42:58]

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

[commit-bot: I haz the power, 2016-09-30 12:42:59]

Dry run: This issue passed the CQ dry run.

[nyquist, 2016-10-03 21:46:42]

lgtm.

The CL description explains what this will be used for, but could you expand a
bit on the new API functionality? Why is this better than loading all values,
for example.

Usually I'd ask for one paragraph each of:
- How does this work now?
- Why is that bad?
- How does this make it better?

But this is more adding-feature in nature, but maybe at least expand it a bit
more about how it works and when/why callers should use this new functionality?

https://codereview.chromium.org/2379113002/diff/80001/components/leveldb_prot...
File components/leveldb_proto/proto_database_impl.h (right):

https://codereview.chromium.org/2379113002/diff/80001/components/leveldb_prot...
components/leveldb_proto/proto_database_impl.h:313: new
std::vector<std::string>());
Nit: Can these not use MakeUnique?

[tschumann, 2016-10-04 09:15:22]

Description was changed from

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

We'll use this for NTP content suggestions to garbage collect data. We do that
at times when we have a list of all still alive elements and need to intersect
that with the elements stored in the db..

BUG=649009
==========

to

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

Before this change, the only way to scan all keys was to call LoadEntries()
which loads the whole database. In our use case, this means copying a lot of
image data which is wasteful.
With LoadKeys(), we still need to iterate the whole database but we don't have
to copy out (and parse) the values -- just the keys. LoadKeys() is useful when
storing relatively large values and only access to the keys is required.

We'll use LoadKeys() for NTP content suggestions to garbage collect data. We do
that at times when we have a list of all still alive elements and need to
intersect that with the elements stored in the db.


BUG=649009
==========

[tschumann, 2016-10-04 09:41:38]

https://codereview.chromium.org/2379113002/diff/80001/components/leveldb_prot...
File components/leveldb_proto/proto_database_impl.h (right):

https://codereview.chromium.org/2379113002/diff/80001/components/leveldb_prot...
components/leveldb_proto/proto_database_impl.h:313: new
std::vector<std::string>());
On 2016/10/03 21:46:42, nyquist (OOO after 10-6) wrote:
> Nit: Can these not use MakeUnique?

Changed to not repeat the types anymore. I think it's cleaner now :-)

[tschumann, 2016-10-04 09:43:49]

The CQ bit was checked by tschumann@chromium.org

[tschumann, 2016-10-04 09:43:49]

The patchset sent to the CQ was uploaded after l-g-t-m from nyquist@chromium.org
Link to the patchset: https://codereview.chromium.org/2379113002/#ps100001
(title: "Avoid repeating long types via MakeUnique")

[commit-bot: I haz the power, 2016-10-04 09:43:58]

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-04 09:50:26]

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

[commit-bot: I haz the power, 2016-10-04 09:50:28]

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

[tschumann, 2016-10-04 12:52:05]

The CQ bit was checked by tschumann@chromium.org

[tschumann, 2016-10-04 12:52:05]

The patchset sent to the CQ was uploaded after l-g-t-m from nyquist@chromium.org
Link to the patchset: https://codereview.chromium.org/2379113002/#ps140001
(title: "ScopedTempDir::path() --> ScopedTempDir::GetPath()")

[commit-bot: I haz the power, 2016-10-04 12:52:18]

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-04 13:35:21]

Description was changed from

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

Before this change, the only way to scan all keys was to call LoadEntries()
which loads the whole database. In our use case, this means copying a lot of
image data which is wasteful.
With LoadKeys(), we still need to iterate the whole database but we don't have
to copy out (and parse) the values -- just the keys. LoadKeys() is useful when
storing relatively large values and only access to the keys is required.

We'll use LoadKeys() for NTP content suggestions to garbage collect data. We do
that at times when we have a list of all still alive elements and need to
intersect that with the elements stored in the db.


BUG=649009
==========

to

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

Before this change, the only way to scan all keys was to call LoadEntries()
which loads the whole database. In our use case, this means copying a lot of
image data which is wasteful.
With LoadKeys(), we still need to iterate the whole database but we don't have
to copy out (and parse) the values -- just the keys. LoadKeys() is useful when
storing relatively large values and only access to the keys is required.

We'll use LoadKeys() for NTP content suggestions to garbage collect data. We do
that at times when we have a list of all still alive elements and need to
intersect that with the elements stored in the db.


BUG=649009
==========

[commit-bot: I haz the power, 2016-10-04 13:35:22]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-10-04 13:37:25]

Description was changed from

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

Before this change, the only way to scan all keys was to call LoadEntries()
which loads the whole database. In our use case, this means copying a lot of
image data which is wasteful.
With LoadKeys(), we still need to iterate the whole database but we don't have
to copy out (and parse) the values -- just the keys. LoadKeys() is useful when
storing relatively large values and only access to the keys is required.

We'll use LoadKeys() for NTP content suggestions to garbage collect data. We do
that at times when we have a list of all still alive elements and need to
intersect that with the elements stored in the db.


BUG=649009
==========

to

==========
Extended the ProtoDatabase to provide LoadKeys() functionality.

Before this change, the only way to scan all keys was to call LoadEntries()
which loads the whole database. In our use case, this means copying a lot of
image data which is wasteful.
With LoadKeys(), we still need to iterate the whole database but we don't have
to copy out (and parse) the values -- just the keys. LoadKeys() is useful when
storing relatively large values and only access to the keys is required.

We'll use LoadKeys() for NTP content suggestions to garbage collect data. We do
that at times when we have a list of all still alive elements and need to
intersect that with the elements stored in the db.

BUG=649009

Committed: https://crrev.com/108e9c223a70d6dc4604354ab5afb0b42b28d30f
Cr-Commit-Position: refs/heads/master@{#422772}
==========

[commit-bot: I haz the power, 2016-10-04 13:37:26]

Patchset 8 (id:??) landed as
https://crrev.com/108e9c223a70d6dc4604354ab5afb0b42b28d30f
Cr-Commit-Position: refs/heads/master@{#422772}

[nyquist, 2016-10-04 14:22:46]

That was a super helpful CL description! Thanks!

https://codereview.chromium.org/2379113002/diff/80001/components/leveldb_prot...
File components/leveldb_proto/proto_database_impl.h (right):

https://codereview.chromium.org/2379113002/diff/80001/components/leveldb_prot...
components/leveldb_proto/proto_database_impl.h:313: new
std::vector<std::string>());
On 2016/10/04 09:41:38, tschumann wrote:
> On 2016/10/03 21:46:42, nyquist (OOO after 10-6) wrote:
> > Nit: Can these not use MakeUnique?
> 
> Changed to not repeat the types anymore. I think it's cleaner now :-)

Agreed. I think this looks very clean.