diff mbox

GDB/testsuite: Bump up `match_max'

Message ID alpine.DEB.1.10.1405172136000.12061@tp.orcam.me.uk
State Committed
Headers

Commit Message

Maciej W. Rozycki May 17, 2014, 8:56 p.m. UTC 
  Hi,

 This fixes:

PASS: gdb.base/info-macros.exp: info macro  -a  --  FOO
ERROR: internal buffer is full.
UNRESOLVED: gdb.base/info-macros.exp: info macros 2
ERROR: internal buffer is full.
UNRESOLVED: gdb.base/info-macros.exp: info macros 3
ERROR: internal buffer is full.
UNRESOLVED: gdb.base/info-macros.exp: info macros 4
FAIL: gdb.base/info-macros.exp: info macros *$pc
ERROR: internal buffer is full.
UNRESOLVED: gdb.base/info-macros.exp: next
FAIL: gdb.base/info-macros.exp: info macros
ERROR: internal buffer is full.
UNRESOLVED: gdb.base/info-macros.exp: next
FAIL: gdb.base/info-macros.exp: info macros 6
ERROR: internal buffer is full.
UNRESOLVED: gdb.base/info-macros.exp: next
FAIL: gdb.base/info-macros.exp: info macros 7
ERROR: internal buffer is full.
UNRESOLVED: gdb.base/info-macros.exp: info macros info-macros.c:42 (PRMS gdb/NNNN)

with the arm-eabi target tested on the i686-mingw32 host where GCC defines 
enough macros to exhaust expect's 30000 characters of buffer space.

 OK to apply?

2014-05-17  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/testsuite/
	* lib/gdb.exp (default_gdb_init): Bump `match_max' up from
	30000 to 65536.

  Maciej

gdb-test-match-max.diff

Comments

Tom Tromey May 19, 2014, 2:18 p.m. UTC | #1 
>>>>> "Maciej" == Maciej W Rozycki <macro@codesourcery.com> writes:

Maciej> 2014-05-17  Maciej W. Rozycki  <macro@codesourcery.com>

Maciej> 	gdb/testsuite/
Maciej> 	* lib/gdb.exp (default_gdb_init): Bump `match_max' up from
Maciej> 	30000 to 65536.

Ok.

I wonder whether you timed the test suite?
The expect man page says:

    This may be changed with the function match_max.  (Note that excessively
    large values can slow down the pattern matcher.)

If it is notably slower then it would be better to rewrite the macro
tests to avoid this need.

Tom
Joel Brobecker May 19, 2014, 2:23 p.m. UTC | #2 
> I wonder whether you timed the test suite?
> The expect man page says:
> 
>     This may be changed with the function match_max.  (Note that excessively
>     large values can slow down the pattern matcher.)
> 
> If it is notably slower then it would be better to rewrite the macro
> tests to avoid this need.

Funny you would say that! I was reviewing the patch, and decided to
do exactly that. Ran into trouble (fresh install), but almost there...
Joel Brobecker May 19, 2014, 2:37 p.m. UTC | #3 
> > I wonder whether you timed the test suite?
> > The expect man page says:
> > 
> >     This may be changed with the function match_max.  (Note that excessively
> >     large values can slow down the pattern matcher.)
> > 
> > If it is notably slower then it would be better to rewrite the macro
> > tests to avoid this need.
> 
> Funny you would say that! I was reviewing the patch, and decided to
> do exactly that. Ran into trouble (fresh install), but almost there...

Here are the results. As I hoped, it doesn't seem to introduce
any noticeable difference (at -j16 on an 8-thread machine).

    Before: 1093.79s user 153.20s system 589% cpu 3:31.68 total
    After:  1097.58s user 155.08s system 589% cpu 3:32.39 total
Doug Evans May 19, 2014, 9:22 p.m. UTC | #4 
On Mon, May 19, 2014 at 7:37 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> > I wonder whether you timed the test suite?
>> > The expect man page says:
>> >
>> >     This may be changed with the function match_max.  (Note that excessively
>> >     large values can slow down the pattern matcher.)
>> >
>> > If it is notably slower then it would be better to rewrite the macro
>> > tests to avoid this need.
>>
>> Funny you would say that! I was reviewing the patch, and decided to
>> do exactly that. Ran into trouble (fresh install), but almost there...
>
> Here are the results. As I hoped, it doesn't seem to introduce
> any noticeable difference (at -j16 on an 8-thread machine).
>
>     Before: 1093.79s user 153.20s system 589% cpu 3:31.68 total
>     After:  1097.58s user 155.08s system 589% cpu 3:32.39 total

Hi.
fwiw, I did several runs of before/after with the testsuite running
serially and didn't find any statistical difference.
All runs were in the range 14:07s to 14:25s elapsed, and sometimes
with-patch was faster.
Not unexpected I guess - most of the time what's actually in the
buffer is pretty small, much less than the buffer size, so other
factors would (generally) have more of an influence on run time.
Maciej W. Rozycki May 20, 2014, 12:46 a.m. UTC | #5 
On Mon, 19 May 2014, Doug Evans wrote:

> >> > I wonder whether you timed the test suite?
> >> > The expect man page says:
> >> >
> >> >     This may be changed with the function match_max.  (Note that excessively
> >> >     large values can slow down the pattern matcher.)
> >> >
> >> > If it is notably slower then it would be better to rewrite the macro
> >> > tests to avoid this need.
> >>
> >> Funny you would say that! I was reviewing the patch, and decided to
> >> do exactly that. Ran into trouble (fresh install), but almost there...
> >
> > Here are the results. As I hoped, it doesn't seem to introduce
> > any noticeable difference (at -j16 on an 8-thread machine).
> >
> >     Before: 1093.79s user 153.20s system 589% cpu 3:31.68 total
> >     After:  1097.58s user 155.08s system 589% cpu 3:32.39 total
> 
> fwiw, I did several runs of before/after with the testsuite running
> serially and didn't find any statistical difference.
> All runs were in the range 14:07s to 14:25s elapsed, and sometimes
> with-patch was faster.
> Not unexpected I guess - most of the time what's actually in the
> buffer is pretty small, much less than the buffer size, so other
> factors would (generally) have more of an influence on run time.

 Have we reached consensus?  At this point of discussion I don't have 
anything to add -- all has been already written AFAICT.  Thank you all for 
verifying the change.

  Maciej
Joel Brobecker May 20, 2014, 2:05 a.m. UTC | #6 
>  Have we reached consensus?  At this point of discussion I don't have 
> anything to add -- all has been already written AFAICT.  Thank you all for 
> verifying the change.

Yes. I think Tom gave the OK, and I would have too if he hadn't.
Doug and I were just confirming that there is no performance impact.
Maciej W. Rozycki May 21, 2014, 7:39 p.m. UTC | #7 
On Tue, 20 May 2014, Joel Brobecker wrote:

> >  Have we reached consensus?  At this point of discussion I don't have 
> > anything to add -- all has been already written AFAICT.  Thank you all for 
> > verifying the change.
> 
> Yes. I think Tom gave the OK, and I would have too if he hadn't.
> Doug and I were just confirming that there is no performance impact.

 Committed now, thanks.

  Maciej
diff mbox

Patch

Index: gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/lib/gdb.exp	2014-05-13 02:52:11.347706187 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/lib/gdb.exp	2014-05-17 21:36:38.618201312 +0100
@@ -3539,8 +3539,9 @@  proc default_gdb_init { args } {
     
     # Unlike most tests, we have a small number of tests that generate
     # a very large amount of output.  We therefore increase the expect
-    # buffer size to be able to contain the entire test output.
-    match_max -d 30000
+    # buffer size to be able to contain the entire test output.  This
+    # is especially needed by gdb.base/info-macros.exp.
+    match_max -d 65536
     # Also set this value for the currently running GDB. 
     match_max [match_max -d]