Re: Go patch committed: Multiplex goroutines onto OS threads
Ian Lance Taylor i...@google.com writes: Ian Lance Taylor i...@google.com writes: Right now it looks like there is a bug, or at least an incompatibility, in the 64-bit versions of getcontext and setcontext. It looks like calling setcontext on the 32-bit version does not change the value of TLS variables, which is also the case on GNU/Linux. In the 64-bit version, calling setcontext does change the value of TLS variables. That is, on the 64-bit version, getcontext preserves the value of TLS variables and setcontext restores the old value. (Of course it's really the pointer, not the TLS variables themselves). This incompatibility has to be a bug, and of course I would prefer that the incompatibility be resolved in favor of the implementation used on GNU/Linux. Well, I thought I could fix this in a sensible way, but I really couldn't. It's a fairly serious bug. Calling setcontext in thread T2 when getcontext was called in thread T1 causes T2 to start using T1's TLS variables, T1's pthread_getspecific values, and even T1's pthread_self. I couldn't figure out any way that T2 could do any thread specific. So I cheated and wrote a system-specific hack. With this patch applied, I see only 4 failures running the testsuite on x86/x86_64 Solaris. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu, where the code is not used. Committed to mainline. Excellent, works like a charm, thanks. The patches for Solaris 10 and 11 are expected within the next month or two. I only noticed two problems: * On Solaris 8/x86 with native TLS, SETCONTEXT_CLOBBERS_TLS was incorrectly defined, causing a proc.c compilation failure: /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:105:4: error: #error unknown case for SETCONTEXT_CLOBBERS_TLS /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function 'runtime_gogo': /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:299:2: error: implicit declaration of function 'fixcontext' [-Werror=implicit-function-declaration] /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function 'runtime_schedinit': /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:366:2: error: implicit declaration of function 'initcontext' [-Werror=implicit-function-declaration] cc1: all warnings being treated as errors make[4]: *** [proc.lo] Error 1 The configure test failed like this: configure:14894: ./conftest ld.so.1: conftest: fatal: conftest: object requires TLS, but TLS failed to initi alize /vol/gcc/src/hg/trunk/local/libgo/configure[20]: 28491 Killed The problem is that on Solaris 8 one needs to link with -pthread which takes care of finding the correct libthread.so. The following patch takes care of that and gives decent testsuite results. * In the cross-compilation case, one needs to test both the i?86-*-solaris2.1[01] and x86_64-*-solaris2.1[10] target triplets, but only for 64-bit. The first is the default, while one can configure for the second if need be. The patch contains the necessary change, adapted from libgcc/configure.ac's type size detection. Besides, the bug affects both Solaris 10 and 11, so the workaround should trigger on either, too. Rainer diff --git a/libgo/configure.ac b/libgo/configure.ac --- a/libgo/configure.ac +++ b/libgo/configure.ac @@ -584,8 +584,12 @@ fi dnl See whether setcontext changes the value of TLS variables. AC_CACHE_CHECK([whether setcontext clobbers TLS variables], [libgo_cv_lib_setcontext_clobbers_tls], -[LIBS_hold=$LIBS +[CFLAGS_hold=$CFLAGS +CFLAGS=$PTHREAD_CFLAGS +LIBS_hold=$LIBS LIBS=$LIBS $PTHREAD_LIBS +AC_CHECK_SIZEOF([void *]) +AS_VAR_ARITH([ptr_type_size], [$ac_cv_sizeof_void_p \* 8]) AC_RUN_IFELSE( [AC_LANG_SOURCE([ #include pthread.h @@ -650,11 +654,14 @@ main () ])], [libgo_cv_lib_setcontext_clobbers_tls=no], [libgo_cv_lib_setcontext_clobbers_tls=yes], -[case $target in - x86_64*-*-solaris2.10) libgo_cv_lib_setcontext_clobbers_tls=yes ;; - *) libgo_cv_lib_setcontext_clobbers_tls=no ;; +[case $target:$ptr_type_size in + i?86-*-solaris2.1[[01]]:64 | x86_64*-*-solaris2.1[[01]]:64) +libgo_cv_lib_setcontext_clobbers_tls=yes ;; + *) +libgo_cv_lib_setcontext_clobbers_tls=no ;; esac ]) +CFLAGS=$CFLAGS_hold LIBS=$LIBS_hold ]) if test $libgo_cv_lib_setcontext_clobbers_tls = yes; then -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Go patch committed: Multiplex goroutines onto OS threads
Rainer Orth r...@cebitec.uni-bielefeld.de writes: * On Solaris 8/x86 with native TLS, SETCONTEXT_CLOBBERS_TLS was incorrectly defined, causing a proc.c compilation failure: /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:105:4: error: #error unknown case for SETCONTEXT_CLOBBERS_TLS /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function 'runtime_gogo': /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:299:2: error: implicit declaration of function 'fixcontext' [-Werror=implicit-function-declaration] /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function 'runtime_schedinit': /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:366:2: error: implicit declaration of function 'initcontext' [-Werror=implicit-function-declaration] cc1: all warnings being treated as errors make[4]: *** [proc.lo] Error 1 The configure test failed like this: configure:14894: ./conftest ld.so.1: conftest: fatal: conftest: object requires TLS, but TLS failed to initi alize /vol/gcc/src/hg/trunk/local/libgo/configure[20]: 28491 Killed The problem is that on Solaris 8 one needs to link with -pthread which takes care of finding the correct libthread.so. The following patch takes care of that and gives decent testsuite results. * In the cross-compilation case, one needs to test both the i?86-*-solaris2.1[01] and x86_64-*-solaris2.1[10] target triplets, but only for 64-bit. The first is the default, while one can configure for the second if need be. The patch contains the necessary change, adapted from libgcc/configure.ac's type size detection. Besides, the bug affects both Solaris 10 and 11, so the workaround should trigger on either, too. Thanks. Committed. Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
Ian Lance Taylor i...@google.com writes:
Right now it looks like there is a bug, or at least an incompatibility,
in the 64-bit versions of getcontext and setcontext. It looks like
calling setcontext on the 32-bit version does not change the value of
TLS variables, which is also the case on GNU/Linux. In the 64-bit
version, calling setcontext does change the value of TLS variables.
That is, on the 64-bit version, getcontext preserves the value of TLS
variables and setcontext restores the old value. (Of course it's really
the pointer, not the TLS variables themselves). This incompatibility
has to be a bug, and of course I would prefer that the incompatibility
be resolved in favor of the implementation used on GNU/Linux.
Well, I thought I could fix this in a sensible way, but I really
couldn't. It's a fairly serious bug. Calling setcontext in thread T2
when getcontext was called in thread T1 causes T2 to start using T1's
TLS variables, T1's pthread_getspecific values, and even T1's
pthread_self. I couldn't figure out any way that T2 could do any thread
specific.
So I cheated and wrote a system-specific hack. With this patch applied,
I see only 4 failures running the testsuite on x86/x86_64 Solaris.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu, where the
code is not used. Committed to mainline.
Ian
diff -r d9bce1ef2e17 libgo/configure.ac
--- a/libgo/configure.ac Tue Feb 07 11:19:23 2012 -0800
+++ b/libgo/configure.ac Tue Feb 07 21:19:06 2012 -0800
@@ -549,6 +549,87 @@
STRUCT_EPOLL_EVENT_FD_OFFSET=${libgo_cv_c_epoll_event_fd_offset}
AC_SUBST(STRUCT_EPOLL_EVENT_FD_OFFSET)
+dnl See whether setcontext changes the value of TLS variables.
+AC_CACHE_CHECK([whether setcontext clobbers TLS variables],
+[libgo_cv_lib_setcontext_clobbers_tls],
+[LIBS_hold=$LIBS
+LIBS=$LIBS $PTHREAD_LIBS
+AC_RUN_IFELSE(
+ [AC_LANG_SOURCE([
+#include pthread.h
+#include stdlib.h
+#include ucontext.h
+#include unistd.h
+
+__thread int tls;
+
+static char stack[[10 * 1024 * 1024]];
+static ucontext_t c;
+
+/* Called via makecontext/setcontext. */
+
+static void
+cfn (void)
+{
+ exit (tls);
+}
+
+/* Called via pthread_create. */
+
+static void *
+tfn (void *dummy)
+{
+ /* The thread should still see this value after calling
+ setcontext. */
+ tls = 0;
+
+ setcontext (c);
+
+ /* The call to setcontext should not return. */
+ abort ();
+}
+
+int
+main ()
+{
+ pthread_t tid;
+
+ /* The thread should not see this value. */
+ tls = 1;
+
+ if (getcontext (c) 0)
+abort ();
+
+ c.uc_stack.ss_sp = stack;
+ c.uc_stack.ss_flags = 0;
+ c.uc_stack.ss_size = sizeof stack;
+ c.uc_link = NULL;
+ makecontext (c, cfn, 0);
+
+ if (pthread_create (tid, NULL, tfn, NULL) != 0)
+abort ();
+
+ if (pthread_join (tid, NULL) != 0)
+abort ();
+
+ /* The thread should have called exit. */
+ abort ();
+}
+])],
+[libgo_cv_lib_setcontext_clobbers_tls=no],
+[libgo_cv_lib_setcontext_clobbers_tls=yes],
+[case $target in
+ x86_64*-*-solaris2.10) libgo_cv_lib_setcontext_clobbers_tls=yes ;;
+ *) libgo_cv_lib_setcontext_clobbers_tls=no ;;
+ esac
+])
+LIBS=$LIBS_hold
+])
+if test $libgo_cv_lib_setcontext_clobbers_tls = yes; then
+ AC_DEFINE(SETCONTEXT_CLOBBERS_TLS, 1,
+ [Define if setcontext clobbers TLS variables])
+fi
+
AC_CACHE_SAVE
if test ${multilib} = yes; then
diff -r d9bce1ef2e17 libgo/runtime/proc.c
--- a/libgo/runtime/proc.c Tue Feb 07 11:19:23 2012 -0800
+++ b/libgo/runtime/proc.c Tue Feb 07 21:19:06 2012 -0800
@@ -60,6 +60,54 @@
static __thread G *g;
static __thread M *m;
+#ifndef SETCONTEXT_CLOBBERS_TLS
+
+static inline void
+initcontext(void)
+{
+}
+
+static inline void
+fixcontext(ucontext_t *c __attribute__ ((unused)))
+{
+}
+
+# else
+
+# if defined(__x86_64__) defined(__sun__)
+
+// x86_64 Solaris 10 and 11 have a bug: setcontext switches the %fs
+// register to that of the thread which called getcontext. The effect
+// is that the address of all __thread variables changes. This bug
+// also affects pthread_self() and pthread_getspecific. We work
+// around it by clobbering the context field directly to keep %fs the
+// same.
+
+static __thread greg_t fs;
+
+static inline void
+initcontext(void)
+{
+ ucontext_t c;
+
+ getcontext(c);
+ fs = c.uc_mcontext.gregs[REG_FSBASE];
+}
+
+static inline void
+fixcontext(ucontext_t* c)
+{
+ c-uc_mcontext.gregs[REG_FSBASE] = fs;
+}
+
+# else
+
+# error unknown case for SETCONTEXT_CLOBBERS_TLS
+
+# endif
+
+#endif
+
// We can not always refer to the TLS variables directly. The
// compiler will call tls_get_addr to get the address of the variable,
// and it may hold it in a register across a call to schedule. When
@@ -248,7 +296,9 @@
#endif
g = newg;
newg-fromgogo = true;
+ fixcontext(newg-context);
setcontext(newg-context);
+ runtime_throw(gogo setcontext returned);
}
// Save context and call fn passing g as a parameter. This is like
@@ -287,6 +337,7 @@
m-g0-entry = (byte*)pfn;
m-g0-param = g;
g
Re: Go patch committed: Multiplex goroutines onto OS threads
Rainer Orth r...@cebitec.uni-bielefeld.de writes:
All tests hang with the default -test.timeout=240.
Thanks for providing access to a Solaris system.
Right now it looks like there is a bug, or at least an incompatibility,
in the 64-bit versions of getcontext and setcontext. It looks like
calling setcontext on the 32-bit version does not change the value of
TLS variables, which is also the case on GNU/Linux. In the 64-bit
version, calling setcontext does change the value of TLS variables.
That is, on the 64-bit version, getcontext preserves the value of TLS
variables and setcontext restores the old value. (Of course it's really
the pointer, not the TLS variables themselves). This incompatibility
has to be a bug, and of course I would prefer that the incompatibility
be resolved in favor of the implementation used on GNU/Linux.
Here is a program which shows the issue. Compile with -pthread. With
-m32 it prints go1: tls == 2. With -m64 it prints go1: tls == 1. The
program is too complex for the problem, but it does show the issue.
Ian
#include errno.h
#include stdio.h
#include stdlib.h
#include limits.h
#include pthread.h
#include ucontext.h
__thread int tls;
static void die (const char *) __attribute__ ((noreturn));
static void
die (const char *msg)
{
perror (msg);
exit (EXIT_FAILURE);
}
static ucontext_t c1;
static void *s1[10];
static ucontext_t c2;
static void *s2[10];
static void swap (ucontext_t *, void *fs[10], ucontext_t *, void *ts[10]);
static void
swap (ucontext_t *fu, void *fs[10], ucontext_t *tu, void *ts[10])
{
swapcontext (fu, tu);
}
static void use_buffer (char *buf) __attribute__ ((noinline));
static void
use_buffer (char *buf)
{
buf[0] = '\0';
}
static void
down (int i, const char *msg, ucontext_t *me, void *mes[10],
ucontext_t *other, void *others[10])
{
char buf[1];
printf (%s %d\n, msg, i);
if (i 0)
{
use_buffer (buf);
swap (me, mes, other, others);
down (i - 1, msg, me, mes, other, others);
}
else
{
printf (i == 0\n);
}
}
static void
go1 (void)
{
printf (go1: tls == %d\n, tls);
down (2, go1, c1, s1, c2, s2);
pthread_exit (NULL);
}
static void
go2 (void)
{
printf (go2: tls == %d\n, tls);
down (2, go2, c2, s2, c1, s1);
pthread_exit (NULL);
}
struct thread_context
{
ucontext_t *u;
void **s;
};
static void *
start_thread (void *context)
{
struct thread_context *tc = (struct thread_context *) context;
int block;
printf (start_thread: tls == %d\n, tls);
tls = 2;
block = 0;
setcontext (tc-u);
die (setcontext);
return NULL;
}
int
main (int argc __attribute__ ((unused)), char **argv __attribute__ ((unused)))
{
pthread_t tid;
int err;
size_t size;
struct thread_context tc;
int block;
tls = 1;
if (getcontext (c1) 0)
die (getcontext);
c2 = c1;
c1.uc_stack.ss_sp = malloc (1024 * 1024 * 1024);
if (c1.uc_stack.ss_sp == NULL)
die (malloc);
c1.uc_stack.ss_flags = 0;
c1.uc_stack.ss_size = 1024 * 1024 * 1024;
c1.uc_link = NULL;
block = 0;
makecontext (c1, go1, 0);
c2.uc_stack.ss_sp = malloc (1024 * 1024 * 1024);
if (c2.uc_stack.ss_sp == NULL)
die (malloc);
c2.uc_stack.ss_flags = 0;
c2.uc_stack.ss_size = 1024 * 1024 * 1024;
c2.uc_link = NULL;
makecontext (c2, go2, 0);
tc.u = c1;
tc.s = s1[0];
err = pthread_create (tid, NULL, start_thread, tc);
if (err != 0)
{
errno = err;
die (pthread_create);
}
err = pthread_join (tid, NULL);
if (err != 0)
{
errno = err;
die (pthread_join);
}
exit (EXIT_SUCCESS);
}
Re: Go patch committed: Multiplex goroutines onto OS threads
Ian Lance Taylor i...@google.com writes: Rainer Orth r...@cebitec.uni-bielefeld.de writes: All tests hang with the default -test.timeout=240. Thanks for providing access to a Solaris system. Right now it looks like there is a bug, or at least an incompatibility, in the 64-bit versions of getcontext and setcontext. It looks like calling setcontext on the 32-bit version does not change the value of TLS variables, which is also the case on GNU/Linux. In the 64-bit version, calling setcontext does change the value of TLS variables. That is, on the 64-bit version, getcontext preserves the value of TLS variables and setcontext restores the old value. (Of course it's really the pointer, not the TLS variables themselves). This incompatibility has to be a bug, and of course I would prefer that the incompatibility be resolved in favor of the implementation used on GNU/Linux. Here is a program which shows the issue. Compile with -pthread. With -m32 it prints go1: tls == 2. With -m64 it prints go1: tls == 1. The program is too complex for the problem, but it does show the issue. Thanks for the detective work. I'll try to find in the OpenSolaris sources what's going on, at the same time contacting the Solaris engineers about the issue. There's more to the problem, though: on Solaris/SPARC (mayon is a Solaris 11 machine you could use here), both 32 and 64-bit versions of the test program print tls == 2, but many (not all) libgo tests still fail. I also tried the program on IRIX 6.5 (columba, dog slow unfortunately), which doesn't have native TLS. It doesn't go beyond the go1: tls == 2 line, and I had to reduce the stack sizes to 32 MB to allow it to run at all. On Tru64 UNIX (haydn), also using emutls, it does run until the i == 0 line, but doesn't finish. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Go patch committed: Multiplex goroutines onto OS threads
On Wed, Dec 14, 2011 at 12:15 AM, Ian Lance Taylor i...@google.com wrote: This patch changes the Go library to multiplex goroutines onto operating system threads. Previously, each new goroutine ran in a separate thread. That is inefficient for programs with lots of goroutines. This patch changes the library such that it runs a certain numbers of threads, and lets each thread switch between goroutines. This is how the master Go library works, and this patch brings in code from the master Go library, adjusted for use by gccgo. For some reason I get this failure on alphaev68-pc-linux-gnu: --- FAIL: runtime_test.TestGcSys (4.64 seconds) using 64 MB using too much memory: 64 MB Raising the value in runtime/gc_test.go to 10e8 runs the test OK. Thanks for reporting this. I just committed the appended patch to both the master Go library and to libgo. I hope this will fix this problem. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Still no success, now I get: --- FAIL: runtime_test.TestGcSys (4.21 seconds) ???:1: used 2554104 extra bytes ???:1: using too much memory: 2554104 bytes FAIL FAIL: runtime Uros.
Re: Go patch committed: Multiplex goroutines onto OS threads
On 11/28/2011 06:46 AM, Ian Lance Taylor wrote: This implementation relies on the functions getcontext, setcontext, and makecontext. If there are systems which don't have those, getcontext and setcontext can be replaced by setjmp and longjmp, but there is no obvious replacement for makecontext. I think that will have to be implemented using processor-specific code, as it needs to set the stack pointer to point to a new stack, and I don't know how to do that in a portable processor-independent manner without using makecontext. ARM glibc does not implement these functions, so we have the same problem in QEMU. (On top of this, there is no way that alternate stacks can work on OpenBSD, so we still have one coroutine per thread). For QEMU, I tried replacing for makecontext using sigaltstack. It works on Linux but it needs a dedicated signal, so it is likely inappropriate for libgo. It would also need to be tested across many operating systems because it may trigger subtleties in other OSes. Paolo
Re: Go patch committed: Multiplex goroutines onto OS threads
Uros Bizjak ubiz...@gmail.com writes:
Still no success, now I get:
--- FAIL: runtime_test.TestGcSys (4.21 seconds)
???:1: used 2554104 extra bytes
???:1: using too much memory: 2554104 bytes
FAIL
FAIL: runtime
Yeah, I was too aggressive. It had to be fixed in the master library
too. This time for sure. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r e97b850a6f33 libgo/go/runtime/gc_test.go
--- a/libgo/go/runtime/gc_test.go Wed Dec 14 06:46:23 2011 -0800
+++ b/libgo/go/runtime/gc_test.go Wed Dec 14 06:48:27 2011 -0800
@@ -22,7 +22,7 @@
sys = runtime.MemStats.Sys - sys
}
t.Logf(used %d extra bytes, sys)
- if sys 220 {
+ if sys 420 {
t.Fatalf(using too much memory: %d bytes, sys)
}
}
Re: Go patch committed: Multiplex goroutines onto OS threads
Paolo Bonzini bonz...@gnu.org writes: On 11/28/2011 06:46 AM, Ian Lance Taylor wrote: This implementation relies on the functions getcontext, setcontext, and makecontext. If there are systems which don't have those, getcontext and setcontext can be replaced by setjmp and longjmp, but there is no obvious replacement for makecontext. I think that will have to be implemented using processor-specific code, as it needs to set the stack pointer to point to a new stack, and I don't know how to do that in a portable processor-independent manner without using makecontext. ARM glibc does not implement these functions, so we have the same problem in QEMU. (On top of this, there is no way that alternate stacks can work on OpenBSD, so we still have one coroutine per thread). The other Go compiler already supports OpenBSD, so it must be possible in some sense. For QEMU, I tried replacing for makecontext using sigaltstack. It works on Linux but it needs a dedicated signal, so it is likely inappropriate for libgo. It would also need to be tested across many operating systems because it may trigger subtleties in other OSes. I don't have any problem using a dedicated signal. That's an interesting approach. I wonder what the relative performance would be. Anyhow, makecontext is easy to write in a system specific manner. It doesn't even have to be written in assembler, though getcontext and setcontext do have to be assembler. Why not just implement them for ARM? Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
On 14 December 2011 15:00, Ian Lance Taylor i...@google.com wrote: Anyhow, makecontext is easy to write in a system specific manner. It doesn't even have to be written in assembler, though getcontext and setcontext do have to be assembler. Why not just implement them for ARM? We're looking at implementing them on ARM, yes. However, obviously all the existing systems out there have versions that return ENOSYS, so you might need some fallback code anyway. -- PMM
Re: Go patch committed: Multiplex goroutines onto OS threads
Peter Maydell peter.mayd...@linaro.org writes: On 14 December 2011 15:00, Ian Lance Taylor i...@google.com wrote: Anyhow, makecontext is easy to write in a system specific manner. It doesn't even have to be written in assembler, though getcontext and setcontext do have to be assembler. Why not just implement them for ARM? We're looking at implementing them on ARM, yes. However, obviously all the existing systems out there have versions that return ENOSYS, so you might need some fallback code anyway. My fallback would be to simply include the implementations of makecontext, et. al., with libgo. Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
On 12/14/2011 04:00 PM, Ian Lance Taylor wrote: This implementation relies on the functions getcontext, setcontext, and makecontext. If there are systems which don't have those, getcontext and setcontext can be replaced by setjmp and longjmp, but there is no obvious replacement for makecontext. I think that will have to be implemented using processor-specific code, as it needs to set the stack pointer to point to a new stack, and I don't know how to do that in a portable processor-independent manner without using makecontext. ARM glibc does not implement these functions, so we have the same problem in QEMU. (On top of this, there is no way that alternate stacks can work on OpenBSD, so we still have one coroutine per thread). The other Go compiler already supports OpenBSD, so it must be possible in some sense. Do they have specialised code for OpenBSD? I said it cannot work in the sense that: - *context functions are not there; - sigaltstack always returns EINVAL when using threads; - the threads are identified by the runtime from their stack pointer, so you risk confusing the runtime very much if you switch between coroutines. I don't have any problem using a dedicated signal. That's an interesting approach. I wonder what the relative performance would be. You can look at the code at git://github.com/bonzini/qemu.git, branch coroutine-fix. Paolo
Re: Go patch committed: Multiplex goroutines onto OS threads
Paolo Bonzini bonz...@gnu.org writes: On 12/14/2011 04:00 PM, Ian Lance Taylor wrote: The other Go compiler already supports OpenBSD, so it must be possible in some sense. Do they have specialised code for OpenBSD? I said it cannot work in the sense that: - *context functions are not there; The other Go compiler does not use the *context functions--it doesn't use libc at all. It uses assembler code which has a similar effect. - sigaltstack always returns EINVAL when using threads; That is odd. sigaltstack is very useful when using threads. - the threads are identified by the runtime from their stack pointer, so you risk confusing the runtime very much if you switch between coroutines. I'm not sure what this means--I'm not sure which runtime you're referring to here. Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
On 12/14/2011 04:45 PM, Ian Lance Taylor wrote: - sigaltstack always returns EINVAL when using threads; That is odd. sigaltstack is very useful when using threads. - the threads are identified by the runtime from their stack pointer, so you risk confusing the runtime very much if you switch between coroutines. I'm not sure what this means--I'm not sure which runtime you're referring to here. The OpenBSD thread runtime. It's similar to old LinuxThreads, but with only 1 kernel thread per process IIUC. Paolo
Re: Go patch committed: Multiplex goroutines onto OS threads
Paolo Bonzini bonz...@gnu.org writes: On 12/14/2011 04:45 PM, Ian Lance Taylor wrote: - sigaltstack always returns EINVAL when using threads; That is odd. sigaltstack is very useful when using threads. - the threads are identified by the runtime from their stack pointer, so you risk confusing the runtime very much if you switch between coroutines. I'm not sure what this means--I'm not sure which runtime you're referring to here. The OpenBSD thread runtime. It's similar to old LinuxThreads, but with only 1 kernel thread per process IIUC. Oh, hmmm, that might be a problem. The other Go compiler seems to use the rfork system call to create new threads, passing RFPROC | RFTHREAD | RFMEM | RFNOWAIT. Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
On 12/14/2011 05:04 PM, Ian Lance Taylor wrote: The OpenBSD thread runtime. It's similar to old LinuxThreads, but with only 1 kernel thread per process IIUC. Oh, hmmm, that might be a problem. The other Go compiler seems to use the rfork system call to create new threads, passing RFPROC | RFTHREAD | RFMEM | RFNOWAIT. Yeah, found it now... it implements basically its own thread runtime. But it only has spinlocks right now. Paolo
Re: Go patch committed: Multiplex goroutines onto OS threads
Rainer Orth r...@cebitec.uni-bielefeld.de writes: Ian Lance Taylor i...@google.com writes: of any differences between Solaris and GNU/Linux when it comes to the getcontext, setcontext, and makecontext functions? I've just found something in Solaris 11 makecontext(3C) (NOTES section), but that doesn't explain why 32-bit Solaris/x86 works, while 64-bit doesn't: http://docs.oracle.com/cd/E23824_01/html/821-1465/makecontext-3c.html Wow, that's annoying. You're right, though, it seems that that ought to affect both x86 and x86_64. Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
Ian Lance Taylor i...@google.com writes:
Rainer Orth r...@cebitec.uni-bielefeld.de writes:
Ian Lance Taylor i...@google.com writes:
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Tested
both with and without -fsplit-stack support. Committed to mainline.
Once Go bootstrap works again on Solaris, I notice that there are many
64-bit testsuite failures, which have been introduced between 2025
(r181724) and 2030 (r181837), so this patch is the obvious culprit.
I just committed another libgo update, which will no doubt lead to
further problems.
Only a few minor compilation problems, fixed with the following patch.
Rainer
diff --git a/libgo/go/net/fd_select.go b/libgo/go/net/fd_select.go
--- a/libgo/go/net/fd_select.go
+++ b/libgo/go/net/fd_select.go
@@ -85,7 +85,8 @@ func (p *pollster) WaitFD(s *pollServer,
timeout = tv
}
- var n, e int
+ var n int
+ var e error
var tmpReadFds, tmpWriteFds syscall.FdSet
for {
// Temporary syscall.FdSet's into which the values are copied
@@ -101,7 +102,7 @@ func (p *pollster) WaitFD(s *pollServer,
break
}
}
- if e != 0 {
+ if e != nil {
return -1, 0, os.NewSyscallError(select, e)
}
if n == 0 {
diff --git a/libgo/go/os/sys_uname.go b/libgo/go/os/sys_uname.go
--- a/libgo/go/os/sys_uname.go
+++ b/libgo/go/os/sys_uname.go
@@ -10,7 +10,7 @@ import syscall
func Hostname() (name string, err error) {
var u syscall.Utsname
- if errno := syscall.Uname(u); errno != 0 {
+ if errno := syscall.Uname(u); errno != nil {
return , NewSyscallError(uname, errno)
}
b := make([]byte, len(u.Nodename))
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Go patch committed: Multiplex goroutines onto OS threads
Rainer Orth r...@cebitec.uni-bielefeld.de writes: Only a few minor compilation problems, fixed with the following patch. Thanks. Committed. Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
Uros Bizjak ubiz...@gmail.com writes:
This patch changes the Go library to multiplex goroutines onto operating
system threads. Previously, each new goroutine ran in a separate
thread. That is inefficient for programs with lots of goroutines. This
patch changes the library such that it runs a certain numbers of
threads, and lets each thread switch between goroutines. This is how
the master Go library works, and this patch brings in code from the
master Go library, adjusted for use by gccgo.
For some reason I get this failure on alphaev68-pc-linux-gnu:
--- FAIL: runtime_test.TestGcSys (4.64 seconds)
using 64 MB
using too much memory: 64 MB
Raising the value in runtime/gc_test.go to 10e8 runs the test OK.
Thanks for reporting this. I just committed the appended patch to both
the master Go library and to libgo. I hope this will fix this problem.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Ian
diff -r 832d9ebdb3c6 libgo/go/runtime/gc_test.go
--- a/libgo/go/runtime/gc_test.go Tue Dec 13 14:24:59 2011 -0800
+++ b/libgo/go/runtime/gc_test.go Tue Dec 13 15:13:43 2011 -0800
@@ -6,16 +6,24 @@
)
func TestGcSys(t *testing.T) {
+ runtime.GC()
+ runtime.UpdateMemStats()
+ sys := runtime.MemStats.Sys
+
for i := 0; i 100; i++ {
workthegc()
}
// Should only be using a few MB.
runtime.UpdateMemStats()
- sys := runtime.MemStats.Sys
- t.Logf(using %d MB, sys20)
- if sys 10e6 {
- t.Fatalf(using too much memory: %d MB, sys20)
+ if sys runtime.MemStats.Sys {
+ sys = 0
+ } else {
+ sys = runtime.MemStats.Sys - sys
+ }
+ t.Logf(used %d extra bytes, sys)
+ if sys 220 {
+ t.Fatalf(using too much memory: %d bytes, sys)
}
}
Re: Go patch committed: Multiplex goroutines onto OS threads
Uros Bizjak ubiz...@gmail.com writes: For some reason I get this failure on alphaev68-pc-linux-gnu: --- FAIL: runtime_test.TestGcSys (4.64 seconds) using 64 MB using too much memory: 64 MB I see the same on Solaris/x86: --- FAIL: runtime_test.TestGcSys (2.76 seconds) using 63 MB using too much memory: 63 MB FAIL FAIL: runtime Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Go patch committed: Multiplex goroutines onto OS threads
Ian Lance Taylor i...@google.com writes: Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Tested both with and without -fsplit-stack support. Committed to mainline. Once Go bootstrap works again on Solaris, I notice that there are many 64-bit testsuite failures, which have been introduced between 2025 (r181724) and 2030 (r181837), so this patch is the obvious culprit. The failures are as follows: * In the go.* tests: FAIL: ./select5-out.go compilation, -O2 -g WARNING: program timed out. 32-bit compilation on idle system (1.6 GHz Core i7 Q720, so a reasonably up-to-date system): ./select5-out.go: In function 'main.$init4': ./select5-out.go:19326:1: note: variable tracking size limit exceeded with -fvar-tracking-assignments, retrying without real2:04.06 user2:03.43 sys0.35 with -fno-var-tracking-assignments: real1:09.87 user1:09.57 sys0.20 With parallel make check, the runtime can easily exceed the default 300s timeout. * 64-bit failures: FAIL: go.test/test/chan/doubleselect.go execution, -O2 -g throw: malloc/free - deadlock panic during panic The messages don't show up in go.log, they goes to stdout, cf. runtime/runtime.h where runtime_printf is just printf. FAIL: go.test/test/chan/select2.go execution, -O2 -g BUG: too much memory for 100,000 selects: 2098448 This test used to fail intermittendly, now the failure seems constant. FAIL: go.test/test/chan/select3.go execution, -O2 -g throw: gosched of g0 FAIL: go.test/test/fixedbugs/bug147.go execution, -O2 -g throw: gosched of g0 FAIL: go.test/test/goprint.go execution, -O2 -g throw: gosched holding locks sometimes throw: bad gp-status in sched FAIL: go.test/test/mallocfin.go execution, -O2 -g throw: gosched holding locks FAIL: go.test/test/nil.go execution, -O2 -g throw: gosched holding locks FAIL: go.test/test/stack.go execution, -O2 -g SEGV from a stack overflow, this test seems to require split-stack support and doesn't even work with unlimited stack size. * libgo testsuite failures: All tests hang with the default -test.timeout=240. When I remove that, some (like bufio) simply pass, while others run into errors similar to the above: goroutine 2 has status 1 throw: bad g-status in ready I've tried to make some sense of it, but don't really know where to start looking. The situation on Solaris/SPARC is considerably worse (large number of 32-bit failures, too, all 32-bit libgo tests fail), but I think it's preferable to get it working again on x86 before checking what remains on SPARC. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Go patch committed: Multiplex goroutines onto OS threads
Rainer Orth r...@cebitec.uni-bielefeld.de writes: Ian Lance Taylor i...@google.com writes: Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Tested both with and without -fsplit-stack support. Committed to mainline. Once Go bootstrap works again on Solaris, I notice that there are many 64-bit testsuite failures, which have been introduced between 2025 (r181724) and 2030 (r181837), so this patch is the obvious culprit. I just committed another libgo update, which will no doubt lead to further problems. In order to debug these problems I think I will need access to an x86 Solaris system. Otherwise I don't know what is happening. Do you know of any differences between Solaris and GNU/Linux when it comes to the getcontext, setcontext, and makecontext functions? Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
Ian Lance Taylor i...@google.com writes:
This patch changes the Go library to multiplex goroutines onto operating
system threads. Previously, each new goroutine ran in a separate
thread. That is inefficient for programs with lots of goroutines. This
patch changes the library such that it runs a certain numbers of
threads, and lets each thread switch between goroutines. This is how
the master Go library works, and this patch brings in code from the
master Go library, adjusted for use by gccgo.
[...]
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Tested
both with and without -fsplit-stack support. Committed to mainline.
Unfortunately, this patch broke Solaris bootstrap (and would break IRIX
bootstrap if this ever started working again):
/vol/gcc/src/hg/trunk/local/libgo/runtime/go-signal.c:221:1: error: conflicting
types for 'sigignore'
In file included from /vol/gcc/src/hg/trunk/local/libgo/runtime/go-signal.c:7:0:
/var/gcc/regression/trunk/8-gcc/build/./gcc/include-fixed/signal.h:100:12: note:
previous declaration of 'sigignore' was here
make[4]: *** [go-signal.lo] Error 1
signal.h on all of Solaris, IRIX, and Tru64 UNIX has
extern int sigignore(int);
I've fixed this by using sig_ignore instead.
Rainer
diff --git a/libgo/runtime/go-signal.c b/libgo/runtime/go-signal.c
--- a/libgo/runtime/go-signal.c
+++ b/libgo/runtime/go-signal.c
@@ -218,7 +218,7 @@ sighandler (int sig)
/* Ignore a signal. */
static void
-sigignore (int sig __attribute__ ((unused)))
+sig_ignore (int sig __attribute__ ((unused)))
{
}
@@ -247,7 +247,7 @@ runtime_initsig (int32 queue)
if (signals[i].catch || signals[i].queue)
sa.sa_handler = sighandler;
else
- sa.sa_handler = sigignore;
+ sa.sa_handler = sig_ignore;
sa.sa_flags = signals[i].restart ? SA_RESTART : 0;
if (sigaction (signals[i].sig, sa, NULL) != 0)
__go_assert (0);
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Go patch committed: Multiplex goroutines onto OS threads
Rainer Orth r...@cebitec.uni-bielefeld.de writes: Unfortunately, this patch broke Solaris bootstrap (and would break IRIX bootstrap if this ever started working again): /vol/gcc/src/hg/trunk/local/libgo/runtime/go-signal.c:221:1: error: conflicting types for 'sigignore' In file included from /vol/gcc/src/hg/trunk/local/libgo/runtime/go-signal.c:7:0: /var/gcc/regression/trunk/8-gcc/build/./gcc/include-fixed/signal.h:100:12: note: previous declaration of 'sigignore' was here make[4]: *** [go-signal.lo] Error 1 signal.h on all of Solaris, IRIX, and Tru64 UNIX has extern int sigignore(int); I've fixed this by using sig_ignore instead. Thanks. Patch committed. Ian
Re: Go patch committed: Multiplex goroutines onto OS threads
Hello! This patch changes the Go library to multiplex goroutines onto operating system threads. Previously, each new goroutine ran in a separate thread. That is inefficient for programs with lots of goroutines. This patch changes the library such that it runs a certain numbers of threads, and lets each thread switch between goroutines. This is how the master Go library works, and this patch brings in code from the master Go library, adjusted for use by gccgo. For some reason I get this failure on alphaev68-pc-linux-gnu: --- FAIL: runtime_test.TestGcSys (4.64 seconds) using 64 MB using too much memory: 64 MB Raising the value in runtime/gc_test.go to 10e8 runs the test OK. Uros.
