GNU bug report logs - #24159
[PATCH] dfa: minor fix for whether dfa is fast or not

Previous Next

Full log

Message #19 received at 24159-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Norihiro Tanaka <noritnk <at> kcn.ne.jp>
Cc: 24159-done <at> debbugs.gnu.org
Subject: Re: bug#24159: [PATCH] dfa: minor fix for whether dfa is fast or not
Date: Fri, 5 Aug 2016 22:05:38 -0700

[Message part 1 (text/plain, inline)]

On Fri, Aug 5, 2016 at 10:02 PM, Jim Meyering <jim <at> meyering.net> wrote:
> On Fri, Aug 5, 2016 at 7:32 PM, Norihiro Tanaka <noritnk <at> kcn.ne.jp> wrote:
>>
>> On Fri, 5 Aug 2016 13:29:43 -0700
>> Jim Meyering <jim <at> meyering.net> wrote:
>>
>>> On Fri, Aug 5, 2016 at 4:30 AM, Norihiro Tanaka <noritnk <at> kcn.ne.jp> wrote:
>>> > dfaoptimize() is not set fast flag even if it is success, but it is wrong.
>>> > If success, dfa matcher uses algorithm for single byte, and it is so fast.
>>> >
>>> > I think this bug does not affect for grep, but it will affect with the
>>> > patch that I just sent to gawk.
>>>
>>> Thank you for the patch.
>>> I was going to push it with the attached slightly updated log message.
>>> Note however that grep does use that -> fast member via dfasearch.c's
>>> use of the dfaisfast function.
>>> But then I realized I should at least verify with "make check", and
>>> found that this makes grep's dfa-match test fail.
>>> Thus, I will not be pushing it as-is.
>>
>> Thanks for review and adjustment.  I re-ran all tests including dfa-match,
>> and they were passwd again in my machine.  Next, I will re-run them on
>> Fedora24, as my machine is RHEL 6.8 and GCC 4.4.7 which is too old.
>>
>> However, I do not know why dfa-match test fails on your machine.
>> dfa-match test does not use grep.  It directly calls dfa functions through
>> dfa-match-aux executable in order to test codes of dfa which grep does
>> not use.  dfa-match-aux does not referer to the ->fast member.
>
> I have examined the logs, which suggest it was a false positive in a
> parallelized "make check" run, due to that test's 3-second timeout. I
> have tried repeatedly to reproduce that failure, so far without
> success, but in coreutils development, with parallelized tests, we
> fixed many hard-to-reproduce tests with small timeout limits like this
> -- most of them now use 10 seconds as the limit, so I will change this
> one, too (and several others) with the attached patch.
>
> I have pushed your patch.

While trying to reproduce that, I ran some tests on OS X and saw
frequent failure of one of the tests, so wrote the attached to work
around what I assume is an aggressive write-to-/dev/null optimization:

[0001-tests-avoid-occasional-false-positive-on-OS-X.diff (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.