Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 17 Feb 1997 23:14:32 +0300 (MSK)
From:      =?KOI8-R?B?4c7E0sXKIP7F0s7P1w==?= <ache@nagual.ru>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        cvs-all@freefall.freebsd.org, CVS-committers@freefall.freebsd.org, cvs-sys@freefall.freebsd.org, mpp@freefall.freebsd.org, peter@spinner.dialix.com
Subject:   Re: cvs commit: src/sys/sys types.h
Message-ID:  <Pine.BSF.3.95q.970217230822.1510A-100000@nagual.ru>
In-Reply-To: <199702170639.RAA18161@godzilla.zeta.org.au>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 17 Feb 1997, Bruce Evans wrote:

> It is really the allocation unit size.  2048 is almost arbitrary.  It is
> the next power of 2 up from the number of bits required for 6 fd_set's
> with the current FD_SETSIZE (6 * 256 = 1536).

Hmm. I don't think that this constant plays any essential role, the only
requirement that re-allocation must not occurse too often.
In my new variant I not use it at all and code looks much cleaner.

> Don't round up to the allocation unit here.  Set ni = roundup(uap->nd,
> NFDBITS).  Then if (ni <= (OLD_FD_SETSIZE = 32 * NBBY)), use an auto
> buffer of size 6*OLD_FD_SETSIZE bits.  Otherwise, use a malloced buffer
> (malloc a larger one if ni > p->p_selbits_size).  This can be simplified
> by always mallocing and freeing at the end.  Space can be saved by
> reducing ni first for the common case when there are no exceptfd's etc.

I implement somehing like you say.

> >--- 549,579 ----
> >  			free (p->p_selbits, M_SELECT);
> >  
> >  		while (p->p_selbits_size < ni)
> >! 			p->p_selbits_size +=
> >! 				howmany(FD_ROUNDSIZE, NFDBITS) *
> >! 				sizeof(fd_mask);
> 
> This can be simplified to something like p_selbytes_allocated =
> roundup2(6 * ni, FD_ROUNDSIZE) / NBBY.  Don't use exactly this -
> it assumes that FD_ROUNDSIZE is both a multiple of NBBY and a
> power of 2.  FD_ROUNDSIZE should be defined as a multiple of NBBY,
> and it won't be a power of 2 on weird machines.

As I already say I use much simpler thing here, just roundup().

> I think there is a problem caused by stupid handling of NULL fdsets.
> Nothing is copied in, but selscan() wastes time scanning the buffers.
> It depends on the current behaviour of bzeroing everything.  Fixing
> this would be a useful optimization than reducing the amount to
> bzero :-).

I fix this thing in selscan().

Updated patch variant:

*** sys_generic.c.orig	Tue Jan 14 23:11:28 1997
--- sys_generic.c	Mon Feb 17 22:57:03 1997
***************
*** 508,513 ****
--- 508,515 ----
  	return (error);
  }
  
+ #define FD_CHUNKSIZE 256
+ 
  static int	nselcoll;
  int	selwait;
  
***************
*** 527,533 ****
  	register struct select_args *uap;
  	int *retval;
  {
! 	fd_mask *ibits[3], *obits[3];
  	struct timeval atv;
  	int s, ncoll, error = 0, timo, i;
  	u_int ni;
--- 529,536 ----
  	register struct select_args *uap;
  	int *retval;
  {
! 	fd_mask s_selbits[(3 + 3) * howmany(FD_CHUNKSIZE, NFDBITS)];
! 	fd_mask *ibits[3], *obits[3], *selbits;
  	struct timeval atv;
  	int s, ncoll, error = 0, timo, i;
  	u_int ni;
***************
*** 538,571 ****
  	if (uap->nd > p->p_fd->fd_nfiles)
  		uap->nd = p->p_fd->fd_nfiles;   /* forgiving; slightly wrong */
  
! 	/* The amount of space we need to allocate */
! 	ni = howmany(roundup2 (uap->nd, FD_SETSIZE), NFDBITS) *
! 		sizeof(fd_mask);
! 
! 	if (ni > p->p_selbits_size) {
! 		if (p->p_selbits_size)
! 			free (p->p_selbits, M_SELECT);
  
! 		while (p->p_selbits_size < ni)
! 			p->p_selbits_size += 32; /* Increase by 256 bits */
  
! 		p->p_selbits = malloc(p->p_selbits_size * 6, M_SELECT,
! 			M_WAITOK);
  	}
  	for (i = 0; i < 3; i++) {
! 		ibits[i] = (fd_mask *)(p->p_selbits + i * p->p_selbits_size);
! 		obits[i] = (fd_mask *)(p->p_selbits + (i + 3) *
! 			p->p_selbits_size);
  	}
! 
! 	/*
! 	 * This buffer is usually very small therefore it's probably faster
! 	 * to just zero it, rather than calculate what needs to be zeroed.
! 	 */
! 	bzero (p->p_selbits, p->p_selbits_size * 6);
! 
! 	/* The amount of space we need to copyin/copyout */
! 	ni = howmany(uap->nd, NFDBITS) * sizeof(fd_mask);
  
  #define	getbits(name, x) \
  	if (uap->name && \
--- 541,587 ----
  	if (uap->nd > p->p_fd->fd_nfiles)
  		uap->nd = p->p_fd->fd_nfiles;   /* forgiving; slightly wrong */
  
! 	/* The amount of space we need to copyin/copyout */
! 	ni = howmany(uap->nd, NFDBITS) * sizeof(fd_mask);
  
! 	if (ni <= sizeof(s_selbits) / (3 + 3))
! 		selbits = s_selbits;
! 	else {
! 		if (ni > p->p_selbits_size) {
! 			if (p->p_selbits_size)
! 				free (p->p_selbits, M_SELECT);
! 
! 			p->p_selbits_size = roundup(ni,
! 				howmany(FD_CHUNKSIZE, NFDBITS) *
! 				sizeof(fd_mask));
  
! 			p->p_selbits = malloc(p->p_selbits_size * 6, M_SELECT,
! 				M_WAITOK);
! 		}
! 		selbits = p->p_selbits;
  	}
+ 
  	for (i = 0; i < 3; i++) {
! 		obits[i] = (fd_mask *)(selbits + i * ni);
! 		ibits[i] = (fd_mask *)(selbits + (i + 3) * ni);
  	}
! 	i = 0;
! 	if (uap->ex)
! 		i = 3;
! 	else
! 		ibits[2] = NULL;
! 	if (uap->ou) {
! 		if (i == 0)
! 			i = 2;
! 	} else
! 		ibits[1] = NULL;
! 	if (uap->in) {
! 		if (i == 0)
! 			i = 1;
! 	} else
! 		ibits[0] = NULL;
! 	if (i != 0)
! 		bzero(selbits, i * ni);
  
  #define	getbits(name, x) \
  	if (uap->name && \
***************
*** 654,670 ****
  	static int flag[3] = { FREAD, FWRITE, 0 };
  
  	for (msk = 0; msk < 3; msk++) {
! 		for (i = 0; i < nfd; i += NFDBITS) {
! 			bits = ibits[msk][i/NFDBITS];
! 			while ((j = ffs(bits)) && (fd = i + --j) < nfd) {
! 				bits &= ~(1 << j);
! 				fp = fdp->fd_ofiles[fd];
! 				if (fp == NULL)
! 					return (EBADF);
! 				if ((*fp->f_ops->fo_select)(fp, flag[msk], p)) {
! 					obits[msk][(fd)/NFDBITS] |= 
! 						(1 << ((fd) % NFDBITS));
! 					n++;
  				}
  			}
  		}
--- 670,688 ----
  	static int flag[3] = { FREAD, FWRITE, 0 };
  
  	for (msk = 0; msk < 3; msk++) {
! 		if (ibits[msk]) {
! 			for (i = 0; i < nfd; i += NFDBITS) {
! 				bits = ibits[msk][i/NFDBITS];
! 				while ((j = ffs(bits)) && (fd = i + --j) < nfd) {
! 					bits &= ~(1 << j);
! 					fp = fdp->fd_ofiles[fd];
! 					if (fp == NULL)
! 						return (EBADF);
! 					if ((*fp->f_ops->fo_select)(fp, flag[msk], p)) {
! 						obits[msk][(fd)/NFDBITS] |=
! 							(1 << ((fd) % NFDBITS));
! 						n++;
! 					}
  				}
  			}
  		}


-- 
Andrey A. Chernov
<ache@null.net>
http://www.nagual.ru/~ache/




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.3.95q.970217230822.1510A-100000>