[PATCH:xscope] Greatly reduce xscope's bss pages
walter harms
wharms at bfs.de
Sat Feb 26 02:52:08 PST 2011
the patch look reasonable, just one small thing: the naming in unfortune
the "original" calloc() uses (number,size) perhaps you can rename it simply
into zalloc() (z=zero) or what every you thing fits.
Just my two cents,
re,
wh
Am 26.02.2011 08:28, schrieb Alan Coopersmith:
> xscope had several static arrays of StaticMaxFD structures, which ended up
> in .bss sections. StaticMaxFD was initialized to FD_SETSIZE.
>
> On 32-bit Solaris, the default value FD_SETSIZE is 1024.
> On 64-bit Solaris, the FD_SETSIZE is 64k, due to the SPARCv9 ABI.
>
> One of the structures allocated included the 32k data buffer for each FD.
> This resulted in the highly excessive mapping of 2gb of .bss when building
> 64-bit binaries on Solaris, and 32mb for 32-bit binaries.
>
> After this patch, only 52k of .bss is mapped.
> (Actual RSS pages for xscope were barely changed.)
>
> To reduce this, the static tables were replaced with callocs of MaxFD
> tables, where MaxFD is now the smaller of StaticMaxFD or the current
> fd limit. StaticMaxFD is reduced by default to 256, since xscope is
> rarely used with large numbers of clients (it can be recompiled with a
> larger StaticMaxFD when needed).
>
> Additionally, the 32k buffers are allocated only when FD's are opened to
> use them, instead of for the maximum possible number of FD's.
>
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
> common.c | 16 ++++++++++++++++
> decode11.c | 11 +++--------
> fd.c | 11 +++--------
> fd.h | 14 +++++++++-----
> proto.h | 1 +
> scope.c | 16 ++++++++++++----
> scope.h | 4 ++--
> server.c | 2 +-
> table11.c | 1 +
> x11.h | 2 +-
> 10 files changed, 49 insertions(+), 29 deletions(-)
>
> diff --git a/common.c b/common.c
> index 1d48530..7ac342c 100644
> --- a/common.c
> +++ b/common.c
> @@ -93,6 +93,22 @@ Malloc (long n)
> return(p);
> }
>
> +void *
> +CallocPerFD (long n)
> +{
> + void *p;
> + p = calloc(MaxFD, n);
> + debug(64,(stderr, "%lx = calloc(%d, %ld)\n", (unsigned long) p, MaxFD, n));
> + if (p == NULL)
> + {
> + fprintf(stderr, "failed to allocate %d bytes for %d files\n",
> + MaxFD * n, MaxFD);
> + panic("cannot continue");
> + }
> + return(p);
> +}
> +
> +
> void
> Free(void *p)
> {
> diff --git a/decode11.c b/decode11.c
> index a9360ff..fb5cb73 100644
> --- a/decode11.c
> +++ b/decode11.c
> @@ -133,19 +133,14 @@ struct QueueHeader
> struct QueueEntry *Tail;
> };
>
> -static struct QueueHeader ReplyQ[StaticMaxFD];
> +static struct QueueHeader *ReplyQ;
>
> /* ************************************************************ */
>
> void
> InitReplyQ (void)
> {
> - short i;
> - for (i = 0; i < StaticMaxFD; i++)
> - {
> - ReplyQ[i].Head = NULL;
> - ReplyQ[i].Tail = NULL;
> - }
> + ReplyQ = CallocPerFD(sizeof(struct QueueHeader));
> }
>
> void
> @@ -209,7 +204,7 @@ SequencedReplyExpected (
>
> /* find the server associated with this client */
> fd = FDPair(fd);
> - if (fd < 0 || fd >= StaticMaxFD) return;
> + if (fd < 0 || fd >= MaxFD) return;
>
> /* attach the new queue entry to the end of the queue for the Server */
> if (ReplyQ[fd].Tail != NULL)
> diff --git a/fd.c b/fd.c
> index 5096e70..623e45f 100644
> --- a/fd.c
> +++ b/fd.c
> @@ -84,7 +84,7 @@
> void
> InitializeFD(void)
> {
> - register short i;
> + int i;
>
> enterprocedure("InitializeFD");
> /* get the number of file descriptors the system will let us use */
> @@ -100,17 +100,12 @@ InitializeFD(void)
> }
> if (MaxFD > StaticMaxFD)
> {
> - fprintf(stderr, "Recompile with larger StaticMaxFD value %d\n", MaxFD);
> MaxFD = StaticMaxFD;
> }
>
> /* allocate space for a File Descriptor (FD) Table */
> - FDD = (struct FDDescriptor *)
> - Malloc ((long)(MaxFD * sizeof (struct FDDescriptor)));
> - if (FDD == NULL) {
> - panic("Can't allocate memory!");
> - }
> - bzero(FDD, (MaxFD * sizeof (struct FDDescriptor)));
> + FDD = CallocPerFD(sizeof (struct FDDescriptor));
> + FDinfo = CallocPerFD(sizeof (struct fdinfo));
>
> /* be sure all fd's are closed and marked not busy */
> for (i = 0; i < MaxFD; i++)
> diff --git a/fd.h b/fd.h
> index 76a3e6e..a711359 100644
> --- a/fd.h
> +++ b/fd.h
> @@ -78,17 +78,21 @@ struct FDDescriptor
> };
>
> extern struct FDDescriptor *FDD /* array of FD descriptors */ ;
> -extern short MaxFD /* maximum number of FD's possible */ ;
> +extern int MaxFD /* maximum number of FD's possible */ ;
>
> -extern short nFDsInUse /* number of FD's actually in use */ ;
> +extern int nFDsInUse /* number of FD's actually in use */ ;
>
> extern fd_set ReadDescriptors /* bit map of FD's in use -- for select */ ;
> extern fd_set WriteDescriptors /* bit map of write blocked FD's -- for select */;
> extern fd_set BlockedReadDescriptors /* bit map of FD's blocked from reading */;
> -extern short HighestFD /* highest FD in use -- for select */ ;
> +extern int HighestFD /* highest FD in use -- for select */ ;
>
> -/* need to change the MaxFD to allow larger number of fd's */
> -#define StaticMaxFD FD_SETSIZE
> +/* cap the number of file descriptors to a reasonable size as long as
> + we're preallocating structs for every one
> + */
> +#ifndef StaticMaxFD
> +# define StaticMaxFD 256
> +#endif
>
> extern void InitializeFD(void);
> extern void UsingFD(FD fd, void (*Handler)(int), void (*FlushHandler)(int),
> diff --git a/proto.h b/proto.h
> index d7dfaec..268e212 100644
> --- a/proto.h
> +++ b/proto.h
> @@ -5,6 +5,7 @@ extern void enterprocedure (char *s);
> extern void warn (char *s);
> extern void panic (char *s);
> extern void *Malloc (long n);
> +extern void *CallocPerFD (long n);
> extern void Free (void *p);
> extern void SetSignalHandling (void);
> extern void SetUpConnectionSocket (int iport, void (*connectionFunc) (int));
> diff --git a/scope.c b/scope.c
> index 6f9df76..17e7061 100644
> --- a/scope.c
> +++ b/scope.c
> @@ -93,12 +93,12 @@ char TerminateClose = 0;
> int Interrupt = 0;
>
> struct FDDescriptor *FDD = NULL;
> -short MaxFD = 0;
> -short nFDsInUse = 0;
> +int MaxFD = 0;
> +int nFDsInUse = 0;
> fd_set ReadDescriptors;
> fd_set WriteDescriptors;
> fd_set BlockedReadDescriptors;
> -short HighestFD;
> +int HighestFD;
>
> short debuglevel = 0;
> short Verbose = 0;
> @@ -933,7 +933,7 @@ SetUpStdin (void)
> */
>
> static long clientNumber = 0;
> -struct fdinfo FDinfo[StaticMaxFD];
> +struct fdinfo *FDinfo;
>
> void
> SetUpPair(
> @@ -946,6 +946,8 @@ SetUpPair(
> FDinfo[client].Server = false;
> FDinfo[client].pair = server;
> FDinfo[client].ClientNumber = clientNumber;
> + if (FDinfo[client].buffer == NULL)
> + FDinfo[client].buffer = Malloc(BUFFER_SIZE);
> FDinfo[client].bufcount = 0;
> FDinfo[client].buflimit = -1;
> FDinfo[client].bufdelivered = 0;
> @@ -954,6 +956,8 @@ SetUpPair(
> FDinfo[server].Server = true;
> FDinfo[server].pair = client;
> FDinfo[server].ClientNumber = FDinfo[client].ClientNumber;
> + if (FDinfo[server].buffer == NULL)
> + FDinfo[server].buffer = Malloc(BUFFER_SIZE);
> FDinfo[server].bufcount = 0;
> FDinfo[server].buflimit = -1;
> FDinfo[server].bufdelivered = 0;
> @@ -973,12 +977,16 @@ ResetPair (
> {
> if (client >= 0)
> {
> + Free(FDinfo[client].buffer);
> + FDinfo[client].buffer = NULL;
> FDinfo[client].bufcount = 0;
> FDinfo[client].buflimit = -1;
> FDinfo[client].bufdelivered = 0;
> }
> if (server >= 0)
> {
> + Free(FDinfo[server].buffer);
> + FDinfo[server].buffer = NULL;
> FDinfo[server].bufcount = 0;
> FDinfo[server].buflimit = -1;
> FDinfo[server].bufdelivered = 0;
> diff --git a/scope.h b/scope.h
> index d440130..d14cfed 100644
> --- a/scope.h
> +++ b/scope.h
> @@ -109,7 +109,7 @@ struct fdinfo
> Boolean Server;
> long ClientNumber;
> FD pair;
> - unsigned char buffer[BUFFER_SIZE];
> + unsigned char *buffer;
> int bufcount;
> int bufstart;
> int buflimit; /* limited writes */
> @@ -117,7 +117,7 @@ struct fdinfo
> Boolean writeblocked;
> };
>
> -extern struct fdinfo FDinfo[StaticMaxFD];
> +extern struct fdinfo *FDinfo;
> extern int littleEndian;
> extern char HandleSIGUSR1;
> extern char Leader[];
> diff --git a/server.c b/server.c
> index e9c1d9e..29691f3 100644
> --- a/server.c
> +++ b/server.c
> @@ -53,7 +53,7 @@
> struct TypeDef TD[MaxTypes];
> unsigned char RBf[2];
> unsigned char SBf[4];
> -struct ConnState CS[StaticMaxFD];
> +struct ConnState *CS;
>
> /* ************************************************************ */
> /* */
> diff --git a/table11.c b/table11.c
> index bfcef73..05d454d 100644
> --- a/table11.c
> +++ b/table11.c
> @@ -107,6 +107,7 @@ static int PrintVISUALTYPE(const unsigned char *buf);
> void
> InitializeX11 (void)
> {
> + CS = CallocPerFD(sizeof(struct ConnState));
> InitReplyQ();
>
> InitBuiltInTypes();
> diff --git a/x11.h b/x11.h
> index 7303e1d..90f0e33 100644
> --- a/x11.h
> +++ b/x11.h
> @@ -508,7 +508,7 @@ struct ConnState
> long SequenceNumber;
> };
>
> -extern struct ConnState CS[StaticMaxFD];
> +extern struct ConnState *CS;
>
> typedef struct _Value {
> struct _Value *next;
More information about the xorg-devel
mailing list