Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 15 Feb 2001 11:16:42 -0500
From:      "Brian F. Feldman" <green@FreeBSD.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        Warner Losh <imp@harmony.village.org>, arch@FreeBSD.org
Subject:   Re: The whole libc thing. 
Message-ID:  <200102151616.f1FGGgA21579@green.dyndns.org>
In-Reply-To: Message from Bruce Evans <bde@zeta.org.au>  of "Thu, 15 Feb 2001 21:38:45 %2B1100." <Pine.BSF.4.21.0102152135350.9729-100000@besplex.bde.org> 

next in thread | previous in thread | raw e-mail | index | archive | help
Bruce Evans <bde@zeta.org.au> wrote:
> On Thu, 15 Feb 2001, Warner Losh wrote:
> 
> > Green's solution of stealing a pointer and using that to "grow" FILE.
> > This would allow old binaris to keep working at the cost of binary
> > compatibility.  There's a few extra indirections in this case, you'll
> > likely not be able to measure that performance loss.  But it will work
> > with the case above and most of the others we threw at it.
> 
> Pointers to FILEs can be converted to pointers extensions of FILEs using
> a hash lookup.  This takes slightly more than a few indirections, but
> it doesn't cost binary compatibilty.


Replying _again_ because I'm pretty sure I didn't Cc: the list (damn 
Reply... menu).

Warner meant that it would allow old binaries to keep working _without_ the 
cost of binary compatibility which would accompany anything we do now that 
doesn't bump every single shared library major in all ports and in the base.

There's no need to even take a huge hit by making a hash of extra junk for 
__sFILE when there is at least one pointer just lying around which is used 
_so_ seldomly it will never be measurably noticed in anything but (imagining 
here...) a .00001% difference on some memory-backed-file microbenchmark that
does nothing but attempt a worst case with ungetc ops and locked ops that 
force all CPU time to be spent in libc/stdio.

Here's the change, not updated to -current with peter's introduction of 
__stdxxx, but that would of course be incorporated (with or without the new 
libc version bump, doesn't matter).  I've not fixed the style yet, and 
noone's verified my pointer arithmetic yet, but I've tested it with a
-D2/13/2001 HEAD and noticed no problems, and it doens't break _any_ binaries
like an incompatible ABI would.

(Please note that this ABI compatibility hack can be deprecated after one 
generation of using __stdxxx instead of __sF[].  So, probably after 5.x.  
IMHO, that's very acceptable.)

Index: include/stdio.h
===================================================================
RCS file: /usr2/ncvs/src/include/stdio.h,v
retrieving revision 1.28
diff -u -r1.28 stdio.h
--- include/stdio.h	2001/02/12 03:31:23	1.28
+++ include/stdio.h	2001/02/15 06:00:52
@@ -70,6 +70,12 @@
 
 struct __file_lock;
 
+/* hold a buncha junk that would grow the ABI */
+struct __sFILEX {
+	struct __file_lock *_lock;	/* used for MT-safety */
+	unsigned char *_up;	/* saved _p when _p is doing ungetc data */
+};
+
 /*
  * stdio state variables.
  *
@@ -111,10 +117,13 @@
 	int	(*_read)  __P((void *, char *, int));
 	fpos_t	(*_seek)  __P((void *, fpos_t, int));
 	int	(*_write) __P((void *, const char *, int));
+	struct __sFILEX *_extra; /* stuff that needs to move to not break ABI */
 
 	/* separate buffer for long sequences of ungetc() */
 	struct	__sbuf _ub;	/* ungetc buffer */
+#if 0
 	unsigned char *_up;	/* saved _p when _p is doing ungetc data */
+#endif
 	int	_ur;		/* saved _r when _r is counting ungetc data */
 
 	/* tricks to meet minimum requirements even when malloc() fails */
@@ -127,7 +136,6 @@
 	/* Unix stdio files get aligned to block boundaries on fseek() */
 	int	_blksize;	/* stat.st_blksize (may be != _bf._size) */
 	fpos_t	_offset;	/* current lseek offset (see WARNING) */
-	struct __file_lock *_lock;	/* used for MT-safety */
 } FILE;
 
 __BEGIN_DECLS
Index: lib/libc//stdio/_flock_stub.c
===================================================================
RCS file: /usr2/ncvs/src/lib/libc/stdio/_flock_stub.c,v
retrieving revision 1.5
diff -u -r1.5 _flock_stub.c
--- lib/libc//stdio/_flock_stub.c	2001/02/11 22:06:39	1.5
+++ lib/libc//stdio/_flock_stub.c	2001/02/15 06:14:52
@@ -82,7 +82,7 @@
 		p->fl_mutex = PTHREAD_MUTEX_INITIALIZER;
 		p->fl_owner = NULL;
 		p->fl_count = 0;
-		fp->_lock = p;
+		fp->_extra->_lock = p;
 		ret = 0;
 	}
 	return (ret);
@@ -98,17 +98,17 @@
 	 * the lock if needed:
 	 */
 	if ((fp->_file >= 0) &&
-	    ((fp->_lock != NULL) || (init_lock(fp) == 0))) {
-		if (fp->_lock->fl_owner == curthread)
-			fp->_lock->fl_count++;
+	    ((fp->_extra->_lock != NULL) || (init_lock(fp) == 0))) {
+		if (fp->_extra->_lock->fl_owner == curthread)
+			fp->_extra->_lock->fl_count++;
 		else {
 			/*
 			 * Make sure this mutex is treated as a private
 			 * internal mutex:
 			 */
-			_pthread_mutex_lock(&fp->_lock->fl_mutex);
-			fp->_lock->fl_owner = curthread;
-			fp->_lock->fl_count = 1;
+			_pthread_mutex_lock(&fp->_extra->_lock->fl_mutex);
+			fp->_extra->_lock->fl_owner = curthread;
+			fp->_extra->_lock->fl_count = 1;
 		}
 	}
 }
@@ -133,16 +133,16 @@
 	 * the lock if needed:
 	 */
 	if ((fp->_file >= 0) &&
-	    ((fp->_lock != NULL) || (init_lock(fp) == 0))) {
-		if (fp->_lock->fl_owner == curthread)
-			fp->_lock->fl_count++;
+	    ((fp->_extra->_lock != NULL) || (init_lock(fp) == 0))) {
+		if (fp->_extra->_lock->fl_owner == curthread)
+			fp->_extra->_lock->fl_count++;
 		/*
 		 * Make sure this mutex is treated as a private
 		 * internal mutex:
 		 */
-		else if (_pthread_mutex_trylock(&fp->_lock->fl_mutex) == 0) {
-			fp->_lock->fl_owner = curthread;
-			fp->_lock->fl_count = 1;
+		else if (_pthread_mutex_trylock(&fp->_extra->_lock->fl_mutex) == 0) {
+			fp->_extra->_lock->fl_owner = curthread;
+			fp->_extra->_lock->fl_count = 1;
 		}
 		else
 			ret = -1;
@@ -161,27 +161,27 @@
 	 * Check if this is a real file with a valid lock owned
 	 * by the current thread:
 	 */
-	if ((fp->_file >= 0) && (fp->_lock != NULL) &&
-	    (fp->_lock->fl_owner == curthread)) {
+	if ((fp->_file >= 0) && (fp->_extra->_lock != NULL) &&
+	    (fp->_extra->_lock->fl_owner == curthread)) {
 		/*
 		 * Check if this thread has locked the FILE
 		 * more than once:
 		 */
-		if (fp->_lock->fl_count > 1)
+		if (fp->_extra->_lock->fl_count > 1)
 			/*
 			 * Decrement the count of the number of
 			 * times the running thread has locked this
 			 * file:
 			 */
-			fp->_lock->fl_count--;
+			fp->_extra->_lock->fl_count--;
 		else {
 			/*
 			 * The running thread will release the
 			 * lock now:
 			 */
-			fp->_lock->fl_count = 0;
-			fp->_lock->fl_owner = NULL;
-			_pthread_mutex_unlock(&fp->_lock->fl_mutex);
+			fp->_extra->_lock->fl_count = 0;
+			fp->_extra->_lock->fl_owner = NULL;
+			_pthread_mutex_unlock(&fp->_extra->_lock->fl_mutex);
 		}
 	}
 }
Index: lib/libc//stdio/findfp.c
===================================================================
RCS file: /usr2/ncvs/src/lib/libc/stdio/findfp.c,v
retrieving revision 1.12
diff -u -r1.12 findfp.c
--- lib/libc//stdio/findfp.c	2001/02/12 03:31:23	1.12
+++ lib/libc//stdio/findfp.c	2001/02/15 06:31:21
@@ -60,13 +60,15 @@
 #define	NDYNAMIC 10		/* add ten more whenever necessary */
 
 #define	std(flags, file) \
-	{0,0,0,flags,file,{0},0,__sF+file,__sclose,__sread,__sseek,__swrite}
-/*	 p r w flags file _bf z  cookie      close    read    seek    write */
+	{0,0,0,flags,file,{0},0,__sF+file,__sclose,__sread,__sseek,__swrite, __sFX + file}
+/*	 p r w flags file _bf z  cookie      close    read    seek    write  extra*/
 
 				/* the usual - (stdin + stdout + stderr) */
 static FILE usual[FOPEN_MAX - 3];
 static struct glue uglue = { NULL, FOPEN_MAX - 3, usual };
 
+static struct __sFILEX __sFX[3];
+
 FILE __sF[3] = {
 	std(__SRD, STDIN_FILENO),		/* stdin */
 	std(__SWR, STDOUT_FILENO),		/* stdout */
@@ -92,18 +94,26 @@
 	int n;
 {
 	struct glue *g;
-	FILE *p;
 	static FILE empty;
+	static struct __sFILEX emptyx;
+	FILE *p;
+	struct __sFILEX *fx;
 
-	g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE));
+	g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE) +
+	    n * sizeof(struct __sFILEX));
 	if (g == NULL)
 		return (NULL);
 	p = (FILE *)ALIGN(g + 1);
+	fx = (struct __sFILEX *)&p[n];
 	g->next = NULL;
 	g->niobs = n;
 	g->iobs = p;
-	while (--n >= 0)
-		*p++ = empty;
+	while (--n >= 0) {
+		*p = empty;
+		p->_extra = fx;
+		*p->_extra = emptyx;
+		p++, fx++;
+	}
 	return (g);
 }
 
Index: lib/libc//stdio/fseek.c
===================================================================
RCS file: /usr2/ncvs/src/lib/libc/stdio/fseek.c,v
retrieving revision 1.10
diff -u -r1.10 fseek.c
--- lib/libc//stdio/fseek.c	2001/01/24 13:00:45	1.10
+++ lib/libc//stdio/fseek.c	2001/02/15 06:08:49
@@ -204,7 +204,7 @@
 	 */
 	if (HASUB(fp)) {
 		curoff += fp->_r;	/* kill off ungetc */
-		n = fp->_up - fp->_bf._base;
+		n = fp->_extra->_up - fp->_bf._base;
 		curoff -= n;
 		n += fp->_ur;
 	} else {
Index: lib/libc//stdio/refill.c
===================================================================
RCS file: /usr2/ncvs/src/lib/libc/stdio/refill.c,v
retrieving revision 1.10
diff -u -r1.10 refill.c
--- lib/libc//stdio/refill.c	2001/02/11 22:06:42	1.10
+++ lib/libc//stdio/refill.c	2001/02/15 06:08:58
@@ -109,7 +109,7 @@
 		if (HASUB(fp)) {
 			FREEUB(fp);
 			if ((fp->_r = fp->_ur) != 0) {
-				fp->_p = fp->_up;
+				fp->_p = fp->_extra->_up;
 				return (0);
 			}
 		}
Index: lib/libc//stdio/ungetc.c
===================================================================
RCS file: /usr2/ncvs/src/lib/libc/stdio/ungetc.c,v
retrieving revision 1.8
diff -u -r1.8 ungetc.c
--- lib/libc//stdio/ungetc.c	2001/01/24 13:00:47	1.8
+++ lib/libc//stdio/ungetc.c	2001/02/15 06:09:12
@@ -164,7 +164,7 @@
 	 * Initially, we will use the `reserve' buffer.
 	 */
 	fp->_ur = fp->_r;
-	fp->_up = fp->_p;
+	fp->_extra->_up = fp->_p;
 	fp->_ub._base = fp->_ubuf;
 	fp->_ub._size = sizeof(fp->_ubuf);
 	fp->_ubuf[sizeof(fp->_ubuf) - 1] = c;


-- 
 Brian Fundakowski Feldman           \  FreeBSD: The Power to Serve!  /
 green@FreeBSD.org                    `------------------------------'




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200102151616.f1FGGgA21579>