Re: strcache scaling issue

[Ralf Wildenhues]

Looking at strcache, I see two other glitches: the size of the cache is
not really aligned with page size: typical malloc implementations
require two size_t entries overhead per allocated area.  Also, there is
a one-off computing the overhead size of the strcache struct.

The patch below should correct these issues.  Detailed review is
appreciated.

Thanks,
Ralf

2011-01-09  Ralf Wildenhues  <address@hidden>

        * strcache.c (STRCACHE_HEADER_SIZE): New define, with fixed
        size.
        (CACHE_BUFFER_SIZE): Define in terms of STRCACHE_HEADER_SIZE,
        and leave two size_t for malloc overhead.
        (new_cache, add_string): Adjust sizes.

Index: strcache.c
===================================================================
RCS file: /cvsroot/make/make/strcache.c,v
retrieving revision 2.9
diff -u -r2.9 strcache.c
--- strcache.c  13 Jul 2010 01:20:43 -0000      2.9
+++ strcache.c  9 Jan 2011 07:55:51 -0000
@@ -1,5 +1,5 @@
 /* Constant string caching for GNU Make.
-Copyright (C) 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
 This file is part of GNU Make.
 
 GNU Make is free software; you can redistribute it and/or modify it under the
@@ -20,10 +20,6 @@
 
 #include "hash.h"
 
-/* The size (in bytes) of each cache buffer.
-   Try to pick something that will map well into the heap.  */
-#define CACHE_BUFFER_SIZE   (8192 - 16)
-
 
 /* A string cached here will never be freed, so we don't need to worry about
    reference counting.  We just store the string, and then remember it in a
@@ -34,9 +30,18 @@
   char *end;                /* Pointer to the beginning of the free space.  */
   int count;                /* # of strings in this buffer (for stats).  */
   int bytesfree;            /* The amount of the buffer that is free.  */
-  char buffer[1];           /* The buffer comes after this.  */
+  char buffer[1];           /* The buffer starts here.  */
 };
 
+/* The size of the strcache struct overhead.  */
+#define STRCACHE_HEADER_SIZE (sizeof (struct strcache) - 1)
+
+/* The size (in bytes) of each cache buffer.
+   Try to pick something that will map well into the heap:
+   typical malloc implementations require storing 2 size_t per block.  */
+#define CACHE_BUFFER_SIZE   (8192 - STRCACHE_HEADER_SIZE - 2 * sizeof(size_t))
+
+
 static int bufsize = CACHE_BUFFER_SIZE;
 static struct strcache *strcache = NULL;
 
@@ -49,7 +54,7 @@
 new_cache()
 {
   struct strcache *new;
-  new = xmalloc (sizeof (*new) + bufsize);
+  new = xmalloc (STRCACHE_HEADER_SIZE + bufsize);
   new->end = new->buffer;
   new->count = 0;
   new->bytesfree = bufsize;
@@ -70,7 +75,7 @@
   /* If the string we want is too large to fit into a single buffer, then
      we're screwed; nothing will ever fit!  Change the maximum size of the
      cache to be big enough.  */
-  if (len > bufsize)
+  if (len + 1 > bufsize)
     bufsize = len * 2;
 
   /* First, find a cache with enough free space.  We always look through all