GNU bug report logs - #17136
[PATCH] dfa: speed up memory allocation, port to IRIX

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17136 in the body.
You can then email your comments to 17136 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: grep mailing list <bug-grep <at> gnu.org>
Subject: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Fri, 28 Mar 2014 18:28:57 -0700

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

Tags: patch

I'm not going to install the attached memory-management changes yet, as 
they're fairly intrusive and I want to give them a chance to cool off. 
The benefits are minor but are there, e.g., this shrinks the overall 
grep text size by 1.2% on my platform (Fedora 20 x86-64).  Mostly, 
though, I wanted the memory management to be clearer.

[0001-dfa-speed-up-memory-allocation-port-to-IRIX.patch (text/plain, attachment)]

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 17136-done <at> debbugs.gnu.org
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Thu, 17 Apr 2014 22:59:17 -0700

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

I merged that patch into the grep trunk and am closing this bug report. 
 While merging, I removed one part of the patch that had a small but 
measurable performance degradation, namely, the introduction of struct 
dfatab.  I found some other minor simplifications and threw them in. 
There seems to be no significant change in CPU performance.  The patch's 
causes the dfa module's text size to shrink by 8% on my platform (x86-64 
Fedora 20), which I suppose is a small win, but the main point of the 
patch is simplifying the source.  I'm attaching what I installed, broken 
into 12 little patches to ease review.

[0001-dfa-clarify-memory-allocation-and-port-to-IRIX.patch (text/plain, attachment)]

[0002-dfa-avoid-unnecessary-work-and-other-initialization.patch (text/plain, attachment)]

[0003-dfa-better-size-overflow-check.patch (text/plain, attachment)]

[0004-dfa-simplify-transition-table-allocation.patch (text/plain, attachment)]

[0005-dfa-simplify-range-char-allocation.patch (text/plain, attachment)]

[0006-dfa-simplify-multibyte_prop-allocation.patch (text/plain, attachment)]

[0007-dfa-simplify-position-set-and-element-count-allocati.patch (text/plain, attachment)]

[0008-dfa-simplify-memory-allocation.patch (text/plain, attachment)]

[0009-dfa-avoid-duplicate-strlen-when-allocating-memory.patch (text/plain, attachment)]

[0010-dfa-simplify-freelist.patch (text/plain, attachment)]

[0011-dfa-simplify-dfmust-initialization.patch (text/plain, attachment)]

[0012-dfa-trans-reallocation-microoptimization.patch (text/plain, attachment)]

Message #13 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: 17136 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 17136-done <at> debbugs.gnu.org
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Fri, 18 Apr 2014 09:45:57 -0700

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

Thanks for all the improvements.
However, one of them introduced a bug that caused many new
"make check" failures.  I have fixed it with this just-pushed patch:

[k.txt (text/plain, attachment)]

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

From: Jim Meyering <jim <at> meyering.net>
To: 17136 <17136 <at> debbugs.gnu.org>, Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 17136-done <17136-done <at> debbugs.gnu.org>
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Fri, 18 Apr 2014 10:00:43 -0700

On Fri, Apr 18, 2014 at 9:45 AM, Jim Meyering <jim <at> meyering.net> wrote:
> Thanks for all the improvements.
> However, one of them introduced a bug that caused many new
> "make check" failures.  I have fixed it with this just-pushed patch:

I have just realized this:
It was only a bug if one neglected to update to the latest gnulib.
With the newer xn2realloc, there is no problem.  I will revert it shortly.

Message #25 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>, 17136 <17136 <at> debbugs.gnu.org>
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Fri, 18 Apr 2014 17:39:22 -0700

Jim Meyering wrote:
> It was only a bug if one neglected to update to the latest gnulib.

Yes, sorry, in hindsight I should have mentioned that in the ChangeLog 
entry.

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

From: Norihiro Tanaka <noritnk <at> kcn.ne.jp>
To: 17136-done <at> debbugs.gnu.org
Subject: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Sat, 19 Apr 2014 10:02:05 +0900

Jim Meyering wrote:
> It was only a bug if one neglected to update to the latest gnulib.

Sorry, I also neglected to update gnulib.

Message #31 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Norihiro Tanaka <noritnk <at> kcn.ne.jp>
To: 17136 <at> debbugs.gnu.org,
 eggert <at> cs.ucla.edu,
 eggert <at> cs.ucla.edu
Subject: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Sat, 19 Apr 2014 10:56:44 +0900

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

Shown two warnings around memory allocation in dfa.c even if update gnulib.
I submit the log at make and the patch to fix it.

Perhaps, I may run accross it in order to use old GCC (4.1.2 on CentOS
5.10). :-)

[warnings.txt (text/plain, attachment)]

[patch.txt (application/octet-stream, attachment)]

Message #34 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Norihiro Tanaka <noritnk <at> kcn.ne.jp>, 17136 <at> debbugs.gnu.org
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Fri, 18 Apr 2014 23:22:14 -0700

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

Norihiro Tanaka wrote:
> Perhaps, I may run accross it in order to use old GCC (4.1.2 on CentOS
> 5.10).

Thanks, it's lucky you did, so that we won't be inundated by other 
people reporting similar problems.  I see now that we were passing 
-Wno-pointer-sign to GCC, and that this suppressed useful diagnostics on 
newer GCC instances (but not on your older one).  I installed the 
attached patch to fix both the signedness problem, and the 
diagnostic-suppression problem.

[0001-dfa-fix-pointer-type-conversion-bug.patch (text/plain, attachment)]

Message #37 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Norihiro Tanaka <noritnk <at> kcn.ne.jp>, 17136 <at> debbugs.gnu.org
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Sat, 19 Apr 2014 14:21:45 -0700

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

On Fri, Apr 18, 2014 at 11:22 PM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:
> Norihiro Tanaka wrote:
>>
>> Perhaps, I may run accross it in order to use old GCC (4.1.2 on CentOS
>> 5.10).
>
>
> Thanks, it's lucky you did, so that we won't be inundated by other people
> reporting similar problems.  I see now that we were passing
> -Wno-pointer-sign to GCC, and that this suppressed useful diagnostics on
> newer GCC instances (but not on your older one).  I installed the attached
> patch to fix both the signedness problem, and the diagnostic-suppression
> problem.

This has encouraged me to revisit grep's list of disabled gcc warnings.
It turns out that we've been disabling many unnecessarily.
The attached patch reenables those that do not elicit a warning
from gcc-4.9.

[k.txt (text/plain, attachment)]

Message #40 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>
Cc: 17136 <at> debbugs.gnu.org
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Thu, 24 Apr 2014 01:11:34 -0700

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

Jim Meyering wrote:
> The attached patch reenables those that do not elicit a warning
> from gcc-4.9.

Thanks, with an unusual configuration I ran into another warning from 
GCC 4.9.0 and installed the attached followup patch.

[0001-build-suppress-unsafe-loop-optimizations-warnings.patch (text/plain, attachment)]

Message #43 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: 17136 <17136 <at> debbugs.gnu.org>, Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 17136-done <17136-done <at> debbugs.gnu.org>
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Thu, 24 Apr 2014 09:22:38 -0700

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

On Thu, Apr 17, 2014 at 10:59 PM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:
> I merged that patch into the grep trunk and am closing this bug report.
> While merging, I removed one part of the patch that had a small but
> measurable performance degradation, namely, the introduction of struct
> dfatab.  I found some other minor simplifications and threw them in. There
> seems to be no significant change in CPU performance.  The patch's causes
> the dfa module's text size to shrink by 8% on my platform (x86-64 Fedora
> 20), which I suppose is a small win, but the main point of the patch is
> simplifying the source.  I'm attaching what I installed, broken into 12
> little patches to ease review.

Running the tests with ASAN-enabled grep exposed a buffer overrun
that was introduced by the "simplify range char allocation" one.
I've fixed it with the attached patch:

[k.txt (text/plain, attachment)]

Message #49 received at 17136 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>, 17136 <17136 <at> debbugs.gnu.org>
Subject: Re: bug#17136: [PATCH] dfa: speed up memory allocation, port to IRIX
Date: Thu, 24 Apr 2014 12:54:34 -0700

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

Thanks for fixing that bug, which I introduced.  I added the attached 
change to fix the comment (which I also wrote) that led me astray.

[0001-dfa-fix-incorrect-comment-that-led-to-heap-overrun.patch (text/x-patch, attachment)]

Previous Next

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