Add a version tag for cached data.

Add a version tag for cached data.

BUG=399580, 431699
LOG=N
R=dcarney@chromium.org, marja@chromium.org

Committed: https://chromium.googlesource.com/v8/v8/+/4f6c75f83a37c79dc6339006aabcf4f8eb97475b

[vogelheim, 2014-11-12 14:36:01]

Patchset #1 (id:1) has been deleted

[vogelheim, 2014-11-12 14:37:52]

vogelheim@chromium.org changed reviewers:
+ dcarney@chromium.org, marja@chromium.org

[vogelheim, 2014-11-12 14:37:52]

Does this version of the API make any sense?

Also, any ideas for unit tests? I'll add one that the tag changes when the flag
list changes; but I don't see how I can easily unit test any of the other
issues.

[dcarney, 2014-11-12 14:49:27]

lgtm, a simple unit test is really all you can do here

[Sven Panne, 2014-11-12 15:55:30]

svenpanne@chromium.org changed reviewers:
+ svenpanne@chromium.org

[Sven Panne, 2014-11-12 15:55:31]

Please revisit this before landing

https://codereview.chromium.org/718043002/diff/20001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/718043002/diff/20001/src/api.cc#newcode1851
src/api.cc:1851: internal::CpuFeatures::SupportedFeatures();
This is a very bad hash, please use the hashing facilitites from
src/base/functional.h

https://codereview.chromium.org/718043002/diff/20001/src/flags.cc
File src/flags.cc (right):

https://codereview.chromium.org/718043002/diff/20001/src/flags.cc#newcode553
src/flags.cc:553: uint32_t FlagList::Hash() {
Again, src/base/functional.h might help here

[vogelheim, 2014-11-12 18:34:07]

On 2014/11/12 14:49:27, dcarney wrote:
> lgtm, a simple unit test is really all you can do here

Yeah. And I'm having difficulty with changing the command line flags in cctest.
If I can't resolve this tomorrow, I'll commit without unit test.

[vogelheim, 2014-11-12 18:34:30]

https://codereview.chromium.org/718043002/diff/20001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/718043002/diff/20001/src/api.cc#newcode1851
src/api.cc:1851: internal::CpuFeatures::SupportedFeatures();
On 2014/11/12 15:55:31, Sven Panne wrote:
> This is a very bad hash, please use the hashing facilitites from
> src/base/functional.h

Done.

Thanks; I was looking for something like those hash functions, but somehow
didn't find them. :(

https://codereview.chromium.org/718043002/diff/20001/src/flags.cc
File src/flags.cc (right):

https://codereview.chromium.org/718043002/diff/20001/src/flags.cc#newcode553
src/flags.cc:553: uint32_t FlagList::Hash() {
On 2014/11/12 15:55:31, Sven Panne wrote:
> Again, src/base/functional.h might help here

Done.

[marja, 2014-11-12 18:48:29]

lgtm 2

Now it's clear from the function name that this is not to be used as the cache
tag as is, but that it merely provides the "version" part of it.

I'm not sure what kind of unit test would add value - I'm okay with adding none.
The test would basically need to be such that it supports making code changes in
this area.. and I think the most probable code change will be that we figure out
"oops, XYZ should've also been taken into account in the cache tag", and there's
no test to tell us that. Tests like "tests that the tag changes if the flag
change" are mildly useful.

[vogelheim, 2014-11-13 15:59:30]

Patchset #4 (id:80001) has been deleted

[vogelheim, 2014-11-13 15:59:40]

Patchset #4 (id:100001) has been deleted

[vogelheim, 2014-11-13 15:59:47]

Patchset #4 (id:120001) has been deleted

[vogelheim, 2014-11-13 15:59:53]

Patchset #4 (id:140001) has been deleted

[vogelheim, 2014-11-13 16:00:00]

Patchset #3 (id:60001) has been deleted

[vogelheim, 2014-11-13 17:17:51]

Committed patchset #4 (id:180001) manually as
4f6c75f83a37c79dc6339006aabcf4f8eb97475b (presubmit successful).