Subject: Re: why python+pth?
To: Marc Recht <recht@netbsd.org>
From: Christian Limpach <chris@pin.lu>
List: tech-pkg
Date: 11/18/2003 14:44:12
--5476859-6297-1069163055=:2956
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII
Content-Disposition: INLINE

On Tue, 18 Nov 2003 10:14:47 +0100 Marc Recht <recht@netbsd.org> wrote:

> > I know of no other systems where this is necessary.
> 
> AFAIK the other systems implement "pthread_attr_setstacksize".

well, we have "pthread_attr_setstacksize" but it doesn't change the
stack size.  This is probably because we only support one stack size for
all stacks.

> > why can't we just increase the default stack size for pthread apps in
> > NetBSD to something more appropriate for applications which exist in
> > the real world...?
> 
> I'm all for it. The small stack also seems to hurt lang/mono (a C#/.Net 
> implementation by Novell/Ximian).

The problem is that increasing the default stack size (from 256KB now to
2MB, which is what Python's test_re needs) will limit the number of threads
we can have on machines with 32bit address space.

The patch below sets the default stack size to the RLIMIT_STACK current
limit or to the size given in PTHREAD_STACKSIZE.  The size is reduced to
the largest power of 2 below or equal to the limit.  The actual stack size
is 8KB (2*pagesize) smaller.  The environment variable takes precedence. 
The old behaviour (fixed stack size of 256KB) can be restored by defining
PT_FIXEDSTACKSIZE_LG in the Makefile(s).

With this change and the default RLIMIT_STACK of 2MB (on i386), the 2
python tests test_re and test_cpickle pass.

I'll commit this in a couple of days if there are no
objections/suggestions.

If we wanted to support different stack sizes, we could use a tree to
find the thread.  We'd take a performance hit searching the tree, so we'd
only use this if there are any stacks with a different size.  Other
suggestions are welcome...


--5476859-6297-1069163055=:2956
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII; NAME="ulimitstack.txt"
Content-Disposition: INLINE; FILENAME="ulimitstack.txt"

Index: libpthread/Makefile
===================================================================
RCS file: /cvs/netbsd/src/lib/libpthread/Makefile,v
retrieving revision 1.23
diff -u -r1.23 Makefile
--- libpthread/Makefile	12 Nov 2003 02:44:22 -0000	1.23
+++ libpthread/Makefile	18 Nov 2003 12:02:24 -0000
@@ -3,6 +3,9 @@
 
 WARNS=	2
 
+# Define PT_FIXEDSTACKSIZE_LG to set a fixed stacksize
+#CPPFLAGS+=-DPT_FIXEDSTACKSIZE_LG=18
+
 .include <bsd.own.mk>
 
 .if exists(${.CURDIR}/arch/${MACHINE_ARCH})
Index: libpthread/pthread_int.h
===================================================================
RCS file: /cvs/netbsd/src/lib/libpthread/pthread_int.h,v
retrieving revision 1.20
diff -u -r1.20 pthread_int.h
--- libpthread/pthread_int.h	9 Nov 2003 18:56:48 -0000	1.20
+++ libpthread/pthread_int.h	18 Nov 2003 12:42:01 -0000
@@ -203,9 +203,24 @@
 #define PT_ATTR_MAGIC	0x22220002
 #define PT_ATTR_DEAD	0xDEAD0002
 
-#define PT_STACKSIZE_LG	18
-#define PT_STACKSIZE	(1<<(PT_STACKSIZE_LG)) 
-#define PT_STACKMASK	(PT_STACKSIZE-1)
+#ifdef PT_FIXEDSTACKSIZE_LG
+
+#define	PT_STACKSIZE_LG	PT_FIXEDSTACKSIZE_LG 
+#define	PT_STACKSIZE	(1<<(PT_STACKSIZE_LG)) 
+#define	PT_STACKMASK	(PT_STACKSIZE-1)
+
+#else  /* PT_FIXEDSTACKSIZE_LG */
+
+extern	int		pt_stacksize_lg;
+extern	size_t		pt_stacksize;
+extern	vaddr_t		pt_stackmask;
+
+#define	PT_STACKSIZE_LG	pt_stacksize_lg
+#define	PT_STACKSIZE	pt_stacksize
+#define	PT_STACKMASK	pt_stackmask
+
+#endif /* PT_FIXEDSTACKSIZE_LG */
+
 
 #define PT_UPCALLSTACKS	16
 
Index: libpthread/pthread_stack.c
===================================================================
RCS file: /cvs/netbsd/src/lib/libpthread/pthread_stack.c,v
retrieving revision 1.8
diff -u -r1.8 pthread_stack.c
--- libpthread/pthread_stack.c	17 Jul 2003 21:07:39 -0000	1.8
+++ libpthread/pthread_stack.c	18 Nov 2003 12:41:45 -0000
@@ -48,13 +48,29 @@
 #include <unistd.h>
 #include <sys/queue.h>
 #include <sys/mman.h>
+#include <sys/resource.h>
 
 #include <sched.h>
 #include "pthread.h"
 #include "pthread_int.h"
 
 static pthread_t
-pthread__stackid_setup(void *base, int size);
+pthread__stackid_setup(void *base, size_t size);
+
+#ifndef PT_FIXEDSTACKSIZE_LG
+/* 
+ * We have to initialize the pt_stack* variables here because mutexes
+ * are used before pthread_init() and thus pthread__initmain() are
+ * called.  Since mutexes only save the stack pointer and not a
+ * pointer to the thread data, it is safe to change the mapping from
+ * stack pointer to thread data afterwards.
+ */
+#define	_STACKSIZE_LG 18
+int	pt_stacksize_lg = _STACKSIZE_LG;
+size_t	pt_stacksize = 1 << _STACKSIZE_LG;
+vaddr_t	pt_stackmask = (1 << _STACKSIZE_LG) - 1;
+#undef	_STACKSIZE_LG
+#endif /* !PT_FIXEDSTACKSIZE_LG */
 
 
 /*
@@ -89,6 +105,34 @@
 {
 	void *base;
 
+#ifndef PT_FIXEDSTACKSIZE_LG
+	struct rlimit slimit;
+	char *value;
+	int ret;
+
+	pt_stacksize = 0;
+	ret = getrlimit(RLIMIT_STACK, &slimit);
+	if (ret == -1)
+		err(1, "Couldn't get stack resource consumption limits");
+	value = getenv("PTHREAD_STACKSIZE");
+	if (value) {
+		pt_stacksize = atoi(value);
+		if (pt_stacksize > slimit.rlim_max)
+			pt_stacksize = (size_t)slimit.rlim_max;
+	}
+	if (pt_stacksize == 0)
+		pt_stacksize = (size_t)slimit.rlim_cur;
+
+	pt_stacksize_lg = -1;
+	while (pt_stacksize) {
+		pt_stacksize >>= 1;
+		pt_stacksize_lg++;
+	}
+
+	pt_stacksize = (1 << pt_stacksize_lg);
+	pt_stackmask = pt_stacksize - 1;
+#endif /* PT_FIXEDSTACKSIZE_LG */
+
 	base = (void *) (pthread__sp() & ~PT_STACKMASK);
 
 	*newt = pthread__stackid_setup(base, PT_STACKSIZE);
@@ -96,7 +140,7 @@
 
 static pthread_t
 /*ARGSUSED*/
-pthread__stackid_setup(void *base, int size)
+pthread__stackid_setup(void *base, size_t size)
 {
 	pthread_t t;
 	size_t pagesize;
Index: libpthread_dbg/Makefile
===================================================================
RCS file: /cvs/netbsd/src/lib/libpthread_dbg/Makefile,v
retrieving revision 1.3
diff -u -r1.3 Makefile
--- libpthread_dbg/Makefile	26 Oct 2003 07:25:34 -0000	1.3
+++ libpthread_dbg/Makefile	18 Nov 2003 13:12:28 -0000
@@ -7,6 +7,9 @@
 
 LIBPDIR=	${.CURDIR}/../libpthread
 
+# Define PT_FIXEDSTACKSIZE_LG to set a fixed stacksize (same as libpthread)
+#CPPFLAGS+=-DPT_FIXEDSTACKSIZE_LG=18
+
 .if exists(${LIBPDIR}/arch/${MACHINE_ARCH})
 ARCHSUBDIR=	${MACHINE_ARCH}
 .elif exists(${LIBPDIR}/arch/${MACHINE_CPU}) 
Index: libpthread_dbg/pthread_dbg.c
===================================================================
RCS file: /cvs/netbsd/src/lib/libpthread_dbg/pthread_dbg.c,v
retrieving revision 1.9
diff -u -r1.9 pthread_dbg.c
--- libpthread_dbg/pthread_dbg.c	11 Sep 2003 21:57:32 -0000	1.9
+++ libpthread_dbg/pthread_dbg.c	18 Nov 2003 13:03:16 -0000
@@ -55,6 +55,16 @@
 
 static int td__getthread(td_proc_t *proc, caddr_t addr, td_thread_t **threadp);
 static int td__getsync(td_proc_t *proc, caddr_t addr, td_sync_t **syncp);
+static int td__getstacksize(td_proc_t *proc);
+
+#ifndef PT_FIXEDSTACKSIZE_LG
+
+caddr_t	pt_stacksize_lg_addr = NULL;
+int	pt_stacksize_lg = -1;
+size_t	pt_stacksize;
+vaddr_t	pt_stackmask;
+#endif /* !PT_FIXEDSTACKSIZE_LG */
+
 
 int
 td_open(struct td_proc_callbacks_t *cb, void *arg, td_proc_t **procp)
@@ -524,6 +534,8 @@
 				ptm_owner),
 			    &taddr, sizeof(taddr))) != 0)
 				return val;
+			if ((val = td__getstacksize(s->proc)) != 0)
+				return val;
 			taddr = pthread__id(taddr);
 			td__getthread(s->proc, (void *)taddr, 
 			    &info->sync_data.mutex.owner);
@@ -773,6 +785,10 @@
 
 	PTHREAD_REG_TO_UCONTEXT(&uc, &gregs);
 
+	val = td__getstacksize(proc);
+	if (val != 0)
+		return val;
+
 	th = pthread__id(pthread__uc_sp(&uc));
 
 	val = READ(proc, th, &magic, sizeof(magic));
@@ -942,3 +958,26 @@
 	return val;
 }
 
+
+static int
+td__getstacksize(td_proc_t *proc)
+{
+#ifndef PT_FIXEDSTACKSIZE_LG
+	int lg, val;
+
+	if (pt_stacksize_lg_addr == NULL) {
+		val = LOOKUP(proc, "pt_stacksize_lg", &pt_stacksize_lg_addr);
+		if (val != 0)
+			return val;
+	}
+	val = READ(proc, pt_stacksize_lg_addr, &lg, sizeof(int));
+	if (val != 0)
+		return val;
+	if (lg != pt_stacksize_lg) {
+		pt_stacksize_lg = lg;
+		pt_stacksize = (1 << pt_stacksize_lg);
+		pt_stackmask = pt_stacksize - 1;
+	}
+#endif /* !PT_FIXEDSTACKSIZE_LG */
+	return 0;
+}

--5476859-6297-1069163055=:2956
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII
Content-Disposition: INLINE


-- 
Christian Limpach <chris@pin.lu>

--5476859-6297-1069163055=:2956--