Re: [PATCH:xscope] Greatly reduce xscope's bss pages
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.coopersm...@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 (longn)
return(p);
}
+void *
+CallocPerFD (longn)
+{
+ 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
Re: [PATCH:xscope] Greatly reduce xscope's bss pages
From: Alan Coopersmith alan.coopersm...@oracle.com Date: Fri, 25 Feb 2011 23:28:24 -0800 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). Sorry, but the combination of your statements here and the actual patch doesn't completely make sense to me. Is reducing StaticMaxFD really necessary now that you have made the allocations more dynamical? Most people will run with ulimit -n set to something sane, so the allocations won't be excessive. You'll probably still want to clamp at FD_SETSIZE though, given how 32-bit Solaris switches around the select(2) and poll(2) implementations if the user sets FD_SETSIZE to a value larger than 1024. If you do decide that the number of file descriptors should be clamped at 256, please leave the warning about recompiling the code with a larger value for StaticMaxFD. Cheers, Mark ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH:xscope] Greatly reduce xscope's bss pages
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.coopersm...@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 (longn)
return(p);
}
+void *
+CallocPerFD (longn)
+{
+ 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
+++
