[PATCH] Decrease number of threads used by goroutines.go test

[PATCH] Decrease number of threads used by goroutines.go test

[Jakub Jelinek]

Hi!

On Fedora/RHEL we default to ulimit -Su 1024 (just soft-limit, to better
avoid non-root for bombs apparently).
goroutines.go test by default attempts to spawn 10000 threads, which means
not only that goroutines.go test fails (no big deal), but that random other
tests that happen to be tested at the same time (I'm testing with make -j48)
sometimes fail too.

The following patch makes sure the tests spawns at most $[`ulimit -u`/4]
threads on Linux native, and just limits number of threads to 64 for
cross-testing and other OSes.

Tested on x86_64-linux with various values of ulimit -Su.  Ok for trunk?

2011-03-08  Jakub Jelinek  <jakub@redhat.com>

	* go.test/go-test.exp: For goroutines.go test pass
	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
	default.

--- gcc/testsuite/go.test/go-test.exp.jj	2011-01-15 11:26:32.000000000 +0100
+++ gcc/testsuite/go.test/go-test.exp	2011-03-08 13:23:36.078402148 +0100
@@ -265,6 +265,23 @@ proc go-gc-tests { } {
 	    verbose -log "$test: go_execute_args is $go_execute_args"
 	    set index [string last " $progargs" $test_line]
 	    set test_line [string replace $test_line $index end]
+	} elseif { [string match "*go.test/test/chan/goroutines.go" $test] } {
+	    # goroutines.go spawns by default 10000 threads, which is too much
+	    # for many OSes.
+	    set go_execute_args 64
+	    if { [ishost "*-linux*" ] && ![is_remote host] && ![is_remote target] } {
+		# On Linux when using low ulimit -u limit, use maximum of
+		# a quarter of that limit and 10000
+		set go_execute_args [lindex [remote_exec host {sh -c ulimit\ -u}] 1]
+		if { [string is integer -strict $go_execute_args] } {
+			set go_execute_args [expr $go_execute_args / 4]
+			if { $go_execute_args > 10000 } { set go_execute_args 10000 }
+			if { $go_execute_args < 16 } { set go_execute_args 16 }
+		} else {
+			set go_execute_args 64
+		}
+	    }
+	    verbose -log "$test: go_execute_args is $go_execute_args"
 	}
 
 	if { $test_line == "// \$G \$D/\$F\.go && \$L \$F\.\$A && \./\$A\.out >tmp.go &&" \

	Jakub

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Ian Lance Taylor]

Jakub Jelinek <jakub@redhat.com> writes:

> 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
>
> 	* go.test/go-test.exp: For goroutines.go test pass
> 	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
> 	default.

How about if we do this unless the environment variable
GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full
testsuite.  I can also change the libgo testsuite to only run the
networking tests if that environment variable is set.  This patch is OK
with that change.  Thanks for doing it.

Ian

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Jakub Jelinek]

On Tue, Mar 08, 2011 at 07:20:28AM -0800, Ian Lance Taylor wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
> >
> > 	* go.test/go-test.exp: For goroutines.go test pass
> > 	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
> > 	default.
> 
> How about if we do this unless the environment variable
> GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full
> testsuite.  I can also change the libgo testsuite to only run the
> networking tests if that environment variable is set.  This patch is OK
> with that change.  Thanks for doing it.

Perhaps we could just use $[`ulimit -u`-100] or similar limit instead
in that case, but if you know you can't run more than 1024 threads
simultaneously and run it anyway, it is always going to fail and likely
break other tests too.  Sure, the "safe" fallback of 64 maybe shouldn't be
used in that case.

	Jakub

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Rainer Orth]

Ian Lance Taylor <iant@google.com> writes:

> Jakub Jelinek <jakub@redhat.com> writes:
>
>> 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
>>
>> 	* go.test/go-test.exp: For goroutines.go test pass
>> 	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
>> 	default.
>
> How about if we do this unless the environment variable
> GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full
> testsuite.  I can also change the libgo testsuite to only run the
> networking tests if that environment variable is set.  This patch is OK
> with that change.  Thanks for doing it.

Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already
used to control dg-require-effective-target run_expensive_tests.  This
avoids separate mechanisms per testsuite/language.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Jakub Jelinek]

On Tue, Mar 08, 2011 at 04:27:04PM +0100, Rainer Orth wrote:
> >> 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
> >>
> >> 	* go.test/go-test.exp: For goroutines.go test pass
> >> 	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
> >> 	default.
> >
> > How about if we do this unless the environment variable
> > GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full
> > testsuite.  I can also change the libgo testsuite to only run the
> > networking tests if that environment variable is set.  This patch is OK
> > with that change.  Thanks for doing it.
> 
> Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already
> used to control dg-require-effective-target run_expensive_tests.  This
> avoids separate mechanisms per testsuite/language.

I guess [getenv GCC_TEST_RUN_EXPENSIVE] != "" could be a usable test here,
if false it could always use 64 threads or something like that, if true
it should IMNSHO still bound it to at most max($[`ulimit -u`/2], 10000)
when ulimit -u is available, because running the thread when it is known
to break other things is a bad idea.
But of course if Ian wants to guard networking libgo tests with some env
var, GCC_TEST_RUN_EXPENSIVE probably isn't an env var to use...

	Jakub

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Ian Lance Taylor]

Jakub Jelinek <jakub@redhat.com> writes:

> On Tue, Mar 08, 2011 at 07:20:28AM -0800, Ian Lance Taylor wrote:
>> Jakub Jelinek <jakub@redhat.com> writes:
>> 
>> > 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
>> >
>> > 	* go.test/go-test.exp: For goroutines.go test pass
>> > 	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
>> > 	default.
>> 
>> How about if we do this unless the environment variable
>> GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full
>> testsuite.  I can also change the libgo testsuite to only run the
>> networking tests if that environment variable is set.  This patch is OK
>> with that change.  Thanks for doing it.
>
> Perhaps we could just use $[`ulimit -u`-100] or similar limit instead
> in that case, but if you know you can't run more than 1024 threads
> simultaneously and run it anyway, it is always going to fail and likely
> break other tests too.  Sure, the "safe" fallback of 64 maybe shouldn't be
> used in that case.

I wouldn't worry about it too much because this is all going to change
completely at some point anyhow, hopefully sooner rather than later.
I'm going to change libgo to not use a separate thread for every
goroutine.  I'd just like to have a way to run the full testsuite for
now.

Ian

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Rainer Orth]

Jakub Jelinek <jakub@redhat.com> writes:

> I guess [getenv GCC_TEST_RUN_EXPENSIVE] != "" could be a usable test here,
> if false it could always use 64 threads or something like that, if true
> it should IMNSHO still bound it to at most max($[`ulimit -u`/2], 10000)
> when ulimit -u is available, because running the thread when it is known
> to break other things is a bad idea.

Right.

> But of course if Ian wants to guard networking libgo tests with some env
> var, GCC_TEST_RUN_EXPENSIVE probably isn't an env var to use...

Indeed, although I'd prefer a different solution, as outlined in PR
go/48017.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Ian Lance Taylor]

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> Ian Lance Taylor <iant@google.com> writes:
>
>> Jakub Jelinek <jakub@redhat.com> writes:
>>
>>> 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
>>>
>>> 	* go.test/go-test.exp: For goroutines.go test pass
>>> 	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
>>> 	default.
>>
>> How about if we do this unless the environment variable
>> GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full
>> testsuite.  I can also change the libgo testsuite to only run the
>> networking tests if that environment variable is set.  This patch is OK
>> with that change.  Thanks for doing it.
>
> Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already
> used to control dg-require-effective-target run_expensive_tests.  This
> avoids separate mechanisms per testsuite/language.

Works for me.  Thanks.

Ian

Re: [PATCH] Decrease number of threads used by goroutines.go test

[Ian Lance Taylor]

Jakub Jelinek <jakub@redhat.com> writes:

> On Tue, Mar 08, 2011 at 04:27:04PM +0100, Rainer Orth wrote:
>> >> 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
>> >>
>> >> 	* go.test/go-test.exp: For goroutines.go test pass
>> >> 	max($[`ulimit -u`/4], 10000) as first argument, or 64 as a safe
>> >> 	default.
>> >
>> > How about if we do this unless the environment variable
>> > GCCGO_RUN_ALL_TESTS is set, so that people have a way to run the full
>> > testsuite.  I can also change the libgo testsuite to only run the
>> > networking tests if that environment variable is set.  This patch is OK
>> > with that change.  Thanks for doing it.
>> 
>> Alternatively, one might use GCC_TEST_RUN_EXPENSIVE, which is already
>> used to control dg-require-effective-target run_expensive_tests.  This
>> avoids separate mechanisms per testsuite/language.
>
> I guess [getenv GCC_TEST_RUN_EXPENSIVE] != "" could be a usable test here,
> if false it could always use 64 threads or something like that, if true
> it should IMNSHO still bound it to at most max($[`ulimit -u`/2], 10000)
> when ulimit -u is available, because running the thread when it is known
> to break other things is a bad idea.
> But of course if Ian wants to guard networking libgo tests with some env
> var, GCC_TEST_RUN_EXPENSIVE probably isn't an env var to use...

I don't really care what environment variable we use, I just want some
way to run the full test, without having DejaGNU silently change the
test on me.  It's perfectly reasonable to have the default check ulimit,
I just want some way to not check it.

Ian

[PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Jakub Jelinek]

On Tue, Mar 08, 2011 at 08:04:23AM -0800, Ian Lance Taylor wrote:
> I don't really care what environment variable we use, I just want some
> way to run the full test, without having DejaGNU silently change the
> test on me.  It's perfectly reasonable to have the default check ulimit,
> I just want some way to not check it.

Ok, here is an updated patch which uses both proposed env vars:

GCCGO_RUN_ALL_TESTS=1 makes it fail for me as before (i.e. 10000 threads)

GCC_TEST_RUN_EXPENSIVE=1 makes it run with max($[`ulimit -u`/4], 10000)
threads on Linux native, 10000 everywhere else

by default it is run just with 64 threads

2011-03-08  Jakub Jelinek  <jakub@redhat.com>

	* go.test/go-test.exp: For goroutines.go test if GCCGO_RUN_ALL_TESTS
	is not set in the environment, pass 64 as first argument when not
	running expensive tests or pass max($[`ulimit -u`/4], 10000) on
	Linux native.

--- gcc/testsuite/go.test/go-test.exp.jj	2011-01-15 11:26:32.000000000 +0100
+++ gcc/testsuite/go.test/go-test.exp	2011-03-08 13:23:36.078402148 +0100
@@ -265,6 +265,27 @@ proc go-gc-tests { } {
 	    verbose -log "$test: go_execute_args is $go_execute_args"
 	    set index [string last " $progargs" $test_line]
 	    set test_line [string replace $test_line $index end]
+	} elseif { [string match "*go.test/test/chan/goroutines.go" $test] \
+		   && [getenv GCCGO_RUN_ALL_TESTS] == "" } {
+	    # goroutines.go spawns by default 10000 threads, which is too much
+	    # for many OSes.
+	    if { [getenv GCC_TEST_RUN_EXPENSIVE] == "" } {
+		set go_execute_args 64
+	    } elseif { [ishost "*-linux*" ] && ![is_remote host] && ![is_remote target] } {
+		# On Linux when using low ulimit -u limit, use maximum of
+		# a quarter of that limit and 10000 even when running expensive
+		# tests, otherwise parallel tests might fail after fork failures.
+		set nproc [lindex [remote_exec host {sh -c ulimit\ -u}] 1]
+		if { [string is integer -strict $nproc] } {
+			set nproc [expr $nproc / 4]
+			if { $nproc > 10000 } { set nproc 10000 }
+			if { $nproc < 16 } { set nproc 16 }
+			set go_execute_args $nproc
+		}
+	    }
+	    if { "$go_execute_args" != "" } {
+		verbose -log "$test: go_execute_args is $go_execute_args"
+	    }
 	}
 
 	if { $test_line == "// \$G \$D/\$F\.go && \$L \$F\.\$A && \./\$A\.out >tmp.go &&" \


	Jakub

Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Rainer Orth]

Jakub Jelinek <jakub@redhat.com> writes:

> Ok, here is an updated patch which uses both proposed env vars:
>
> GCCGO_RUN_ALL_TESTS=1 makes it fail for me as before (i.e. 10000 threads)
>
> GCC_TEST_RUN_EXPENSIVE=1 makes it run with max($[`ulimit -u`/4], 10000)
> threads on Linux native, 10000 everywhere else

Why should this be Linux-specific?  I think the same logic applies
everywhere.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Jakub Jelinek]

On Tue, Mar 08, 2011 at 07:40:38PM +0100, Rainer Orth wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > Ok, here is an updated patch which uses both proposed env vars:
> >
> > GCCGO_RUN_ALL_TESTS=1 makes it fail for me as before (i.e. 10000 threads)
> >
> > GCC_TEST_RUN_EXPENSIVE=1 makes it run with max($[`ulimit -u`/4], 10000)
> > threads on Linux native, 10000 everywhere else
> 
> Why should this be Linux-specific?  I think the same logic applies
> everywhere.

Because ulimit -u is Linux specific?  At least, google doesn't show any
hints about any other OSes having such limit, neither RLIMIT_NPROC nor
ulimit -u.

	Jakub

Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Rainer Orth]

Jakub Jelinek <jakub@redhat.com> writes:

>> Why should this be Linux-specific?  I think the same logic applies
>> everywhere.
>
> Because ulimit -u is Linux specific?  At least, google doesn't show any
> hints about any other OSes having such limit, neither RLIMIT_NPROC nor
> ulimit -u.

At best, it's shell-specific: Solaris 11 /bin/sh (which is ksh93) does
have it, although admittedly previous Solaris/IRIX/Tru64 UNIX shells
don't.  On the other hand, bash has it on all of those systems.

Why not simply test if ulimit -u doesn't error and then use it?  I'd
very much prefer this to a solution that is unnecessarily OS-specific.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Jakub Jelinek]

On Tue, Mar 08, 2011 at 07:56:38PM +0100, Rainer Orth wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> At best, it's shell-specific: Solaris 11 /bin/sh (which is ksh93) does
> have it, although admittedly previous Solaris/IRIX/Tru64 UNIX shells
> don't.  On the other hand, bash has it on all of those systems.
> 
> Why not simply test if ulimit -u doesn't error and then use it?  I'd
> very much prefer this to a solution that is unnecessarily OS-specific.

I'm happy to drop the [ ishost "*-linux*" ] && if you are going to look for
failures on weirdo OSes.  I have no idea what ulimit -u does on anything but
Linux, while the tcl code only uses its value if it printed a number,
whether it is something similar to limit on number of each user's threads
or something completely else is unclear.

	Jakub

Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Rainer Orth]

Jakub Jelinek <jakub@redhat.com> writes:

> I'm happy to drop the [ ishost "*-linux*" ] && if you are going to look for
> failures on weirdo OSes.  I have no idea what ulimit -u does on anything but
> Linux, while the tcl code only uses its value if it printed a number,
> whether it is something similar to limit on number of each user's threads
> or something completely else is unclear.

In both bash and every non-bash shell I have that implements it at all,
ulimit -u does exactly the same as on Linux.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

[PATCH] Decrease number of threads used by goroutines.go test (take 3)

[Jakub Jelinek]

On Tue, Mar 08, 2011 at 08:10:31PM +0100, Rainer Orth wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > I'm happy to drop the [ ishost "*-linux*" ] && if you are going to look for
> > failures on weirdo OSes.  I have no idea what ulimit -u does on anything but
> > Linux, while the tcl code only uses its value if it printed a number,
> > whether it is something similar to limit on number of each user's threads
> > or something completely else is unclear.
> 
> In both bash and every non-bash shell I have that implements it at all,
> ulimit -u does exactly the same as on Linux.

Ok then:

2011-03-08  Jakub Jelinek  <jakub@redhat.com>

	* go.test/go-test.exp: For goroutines.go test if GCCGO_RUN_ALL_TESTS
	is not set in the environment, pass 64 as first argument when not
	running expensive tests or pass max($[`ulimit -u`/4], 10000) on
	native where ulimit -u is supported.

--- gcc/testsuite/go.test/go-test.exp.jj	2011-01-15 11:26:32.000000000 +0100
+++ gcc/testsuite/go.test/go-test.exp	2011-03-08 13:23:36.078402148 +0100
@@ -265,6 +265,27 @@ proc go-gc-tests { } {
 	    verbose -log "$test: go_execute_args is $go_execute_args"
 	    set index [string last " $progargs" $test_line]
 	    set test_line [string replace $test_line $index end]
+	} elseif { [string match "*go.test/test/chan/goroutines.go" $test] \
+		   && [getenv GCCGO_RUN_ALL_TESTS] == "" } {
+	    # goroutines.go spawns by default 10000 threads, which is too much
+	    # for many OSes.
+	    if { [getenv GCC_TEST_RUN_EXPENSIVE] == "" } {
+		set go_execute_args 64
+	    } elseif { ![is_remote host] && ![is_remote target] } {
+		# When using low ulimit -u limit, use maximum of
+		# a quarter of that limit and 10000 even when running expensive
+		# tests, otherwise parallel tests might fail after fork failures.
+		set nproc [lindex [remote_exec host {sh -c ulimit\ -u}] 1]
+		if { [string is integer -strict $nproc] } {
+			set nproc [expr $nproc / 4]
+			if { $nproc > 10000 } { set nproc 10000 }
+			if { $nproc < 16 } { set nproc 16 }
+			set go_execute_args $nproc
+		}
+	    }
+	    if { "$go_execute_args" != "" } {
+		verbose -log "$test: go_execute_args is $go_execute_args"
+	    }
 	}
 
 	if { $test_line == "// \$G \$D/\$F\.go && \$L \$F\.\$A && \./\$A\.out >tmp.go &&" \

	Jakub

Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Ian Lance Taylor]

Jakub Jelinek <jakub@redhat.com> writes:

> 2011-03-08  Jakub Jelinek  <jakub@redhat.com>
>
> 	* go.test/go-test.exp: For goroutines.go test if GCCGO_RUN_ALL_TESTS
> 	is not set in the environment, pass 64 as first argument when not
> 	running expensive tests or pass max($[`ulimit -u`/4], 10000) on
> 	Linux native.

This is OK, and it's also OK if you remove the ishost conditional as
Rainer suggests.

Thanks.

Ian

Re: [PATCH] Decrease number of threads used by goroutines.go test (take 2)

[Mike Stump]

On Mar 8, 2011, at 10:44 AM, Jakub Jelinek wrote:
> Because ulimit -u is Linux specific?

Seems to work on darwin (266).