[foozzie] Improve failure state deduplication

[foozzie] Improve failure state deduplication

The fuzz test cases now print the original test paths during execution.
This exploits this extra information and reports a hash of only one
original source file from the section that caused a difference.

The hash size is now limited to 3 to avoid possible duplicate
explosion, in case this doesn't work out as expected.

This prepares for patch 3 of:
https://chromereviews.googleplex.com/550337016/

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org

Review-Url: https://codereview.chromium.org/2620343005
Cr-Commit-Position: refs/heads/master@{#42305}
Committed: https://chromium.googlesource.com/v8/v8/+/00e98a39e3a3006e3c2ad593a3713a058fc40475

[Michael Achenbach, 2017-01-12 21:51:44]

Description was changed from

==========
[foozzie] Improve failures state deduplication

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

to

==========
[foozzie] Improve failures state deduplication

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org
==========

[Michael Achenbach, 2017-01-12 21:51:54]

Description was changed from

==========
[foozzie] Improve failures state deduplication

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org
==========

to

==========
[foozzie] Improve failure state deduplication

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org
==========

[Michael Achenbach, 2017-01-12 22:35:28]

Description was changed from

==========
[foozzie] Improve failure state deduplication

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org
==========

to

==========
[foozzie] Improve failure state deduplication

The fuzz test cases now print the original test paths during execution.
This exploits this extra information and reports a hash of only one
original source file from the section that caused a difference.

The hash size is now limited to 3 to avoid possible duplicate
explosion, in case this doesn't work out as expected.

This prepares for patch 3 of:
https://chromereviews.googleplex.com/550337016/

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org
==========

[Michael Achenbach, 2017-01-12 22:35:28]

machenbach@chromium.org changed reviewers:
+ mbarbella@chromium.org, tandrii@chromium.org

[Michael Achenbach, 2017-01-12 22:36:22]

PTAL

[Michael Achenbach, 2017-01-13 06:30:10]

The CQ bit was checked by machenbach@chromium.org

[commit-bot: I haz the power, 2017-01-13 06:30: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, 2017-01-13 06:31:55]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484289010082700,
"parent_rev": "d730551edcf7a07d3f16d5e22b589f0c198bbb21", "commit_rev":
"00e98a39e3a3006e3c2ad593a3713a058fc40475"}

[commit-bot: I haz the power, 2017-01-13 06:32:01]

Description was changed from

==========
[foozzie] Improve failure state deduplication

The fuzz test cases now print the original test paths during execution.
This exploits this extra information and reports a hash of only one
original source file from the section that caused a difference.

The hash size is now limited to 3 to avoid possible duplicate
explosion, in case this doesn't work out as expected.

This prepares for patch 3 of:
https://chromereviews.googleplex.com/550337016/

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org
==========

to

==========
[foozzie] Improve failure state deduplication

The fuzz test cases now print the original test paths during execution.
This exploits this extra information and reports a hash of only one
original source file from the section that caused a difference.

The hash size is now limited to 3 to avoid possible duplicate
explosion, in case this doesn't work out as expected.

This prepares for patch 3 of:
https://chromereviews.googleplex.com/550337016/

BUG=chromium:673246
NOTRY=true
TBR=tandrii@chromium.org,mbarbella@chromium.org

Review-Url: https://codereview.chromium.org/2620343005
Cr-Commit-Position: refs/heads/master@{#42305}
Committed:
https://chromium.googlesource.com/v8/v8/+/00e98a39e3a3006e3c2ad593a3713a058fc...
==========

[commit-bot: I haz the power, 2017-01-13 06:32:02]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/00e98a39e3a3006e3c2ad593a3713a058fc...

[tandrii(chromium), 2017-01-13 06:38:34]

lgtm % shouldn't description be "to avoid DEduplication explosion"?

https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
File tools/foozzie/v8_suppressions.py (right):

https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
tools/foozzie/v8_suppressions.py:195: It is the string 'none' if no such output
existed.
i don't understand why 'none' is a good choice, but I also don't see why it's a
bad one.

[Michael Achenbach, 2017-01-13 07:49:53]

On 2017/01/13 06:38:34, tandrii(chromium) wrote:
> lgtm % shouldn't description be "to avoid DEduplication explosion"?

No. DEduplication is a good thing, i.e. avoiding to get too many duplicate error
reports. I'll get good deduplication if the same root causes map to the same
hash used in the failure state. If that mapping is bad I get duplicates (i.e.
get the same root cause reported multiple times). If the mapping is really bad,
this might explode. But if I keep the hashes short, there is a bound on how bad
it can be.

Of course, this CL also focuses on not deduping too much. E.g. I could trivially
map all reports to one key. Maximum deduplication, but little value. So it's a
bit of both I think.

https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
File tools/foozzie/v8_suppressions.py (right):

https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
tools/foozzie/v8_suppressions.py:195: It is the string 'none' if no such output
existed.
On 2017/01/13 06:38:34, tandrii(chromium) wrote:
> i don't understand why 'none' is a good choice, but I also don't see why it's
a
> bad one.

It's a random string. Could be anything (except one of the real filenames). I
don't want to use an empty string as that can later not be differentiated
between running or not running this code at all.

In practice I hope we're not going to see this value at all. I we do, then all
failure states with the none value will be deduped on the first that occurred.
But I can use the output of that one to figure out why it happened, resolve it
and close it again.

[Michael Achenbach, 2017-01-13 07:52:39]

https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
File tools/foozzie/v8_suppressions.py (right):

https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
tools/foozzie/v8_suppressions.py:195: It is the string 'none' if no such output
existed.
On 2017/01/13 07:49:52, Michael Achenbach wrote:
> On 2017/01/13 06:38:34, tandrii(chromium) wrote:
> > i don't understand why 'none' is a good choice, but I also don't see why
it's
> a
> > bad one.
> 
> It's a random string. Could be anything (except one of the real filenames). I
> don't want to use an empty string as that can later not be differentiated
> between running or not running this code at all.
> 
> In practice I hope we're not going to see this value at all. I we do, then all
> failure states with the none value will be deduped on the first that occurred.
> But I can use the output of that one to figure out why it happened, resolve it
> and close it again. 

But there's of course a bug :/. The none value shouldn't get hashed. It should
stay as is. Otherwise it might be mapped together with some other values... hmm.
Maybe I just pull hashing into this function...

[tandrii(chromium), 2017-01-13 07:53:21]

On 2017/01/13 07:49:53, Michael Achenbach wrote:
> On 2017/01/13 06:38:34, tandrii(chromium) wrote:
> > lgtm % shouldn't description be "to avoid DEduplication explosion"?
> 
> No. DEduplication is a good thing, i.e. avoiding to get too many duplicate
error
> reports. I'll get good deduplication if the same root causes map to the same
> hash used in the failure state. If that mapping is bad I get duplicates (i.e.
> get the same root cause reported multiple times). If the mapping is really
bad,
> this might explode. But if I keep the hashes short, there is a bound on how
bad
> it can be.
> 
> Of course, this CL also focuses on not deduping too much. E.g. I could
trivially
> map all reports to one key. Maximum deduplication, but little value. So it's a
> bit of both I think.
Upon reading your reply, I don't understand why I thought there was an error in
description. Agree with what you wrote.

> 
>
https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
> File tools/foozzie/v8_suppressions.py (right):
> 
>
https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
> tools/foozzie/v8_suppressions.py:195: It is the string 'none' if no such
output
> existed.
> On 2017/01/13 06:38:34, tandrii(chromium) wrote:
> > i don't understand why 'none' is a good choice, but I also don't see why
it's
> a
> > bad one.
> 
> It's a random string. Could be anything (except one of the real filenames). I
> don't want to use an empty string as that can later not be differentiated
> between running or not running this code at all.
> 
> In practice I hope we're not going to see this value at all. I we do, then all
> failure states with the none value will be deduped on the first that occurred.
> But I can use the output of that one to figure out why it happened, resolve it
> and close it again.
ACK.

[Michael Achenbach, 2017-01-13 08:14:52]

On 2017/01/13 07:52:39, Michael Achenbach wrote:
>
https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
> File tools/foozzie/v8_suppressions.py (right):
> 
>
https://codereview.chromium.org/2620343005/diff/20001/tools/foozzie/v8_suppre...
> tools/foozzie/v8_suppressions.py:195: It is the string 'none' if no such
output
> existed.
> On 2017/01/13 07:49:52, Michael Achenbach wrote:
> > On 2017/01/13 06:38:34, tandrii(chromium) wrote:
> > > i don't understand why 'none' is a good choice, but I also don't see why
> it's
> > a
> > > bad one.
> > 
> > It's a random string. Could be anything (except one of the real filenames).
I
> > don't want to use an empty string as that can later not be differentiated
> > between running or not running this code at all.
> > 
> > In practice I hope we're not going to see this value at all. I we do, then
all
> > failure states with the none value will be deduped on the first that
occurred.
> > But I can use the output of that one to figure out why it happened, resolve
it
> > and close it again. 
> 
> But there's of course a bug :/. The none value shouldn't get hashed. It should
> stay as is. Otherwise it might be mapped together with some other values...
hmm.
> Maybe I just pull hashing into this function...

Follow up fix for the bug:
https://codereview.chromium.org/2632623002/