Herewith the first set of patches to the memory allocation routines.

There is no new functionality here yet; basically I have been working on
trying to remove some of the code that is duplicated between the various
pools, before even more copies get made for the new stuff.

The result is some fairly major changes, but I believe it will make adding
the new features easier. I have tested the end product using the standard
test suite, 5000 generations of life, loading Clint's basic wumpus, and
reversing resources.c using Melvin's reverse.cola. We don't have much in the
way of PMC-intensive test code at present.

A summary of the changes:
struct free_pool has new member 'replenish'; this is a function to be called
when the pool is empty
Initialisation of the free pools moved from memory.c to resources.c - mainly
to avoid exporting more functions from resources.c
New function new_free_pool to create a free pool - just to stop doing the
same thing several times
New function add_to_free_pool - replaces existing add_pmc_to_free and
add_header_to_free
New function get_from_free_pool - called by the new_*_header functions to
avoid duplicating the code involved in extracting an entry from the free
pool; also incorporates the calling of do_dod_run and the replenishment of
the pools
[Mike - in the process, I have removed some of your GC_DEBUG stuff; if this
patch is applied, feel free to decide where you want to reinstate this]

Essentially, the free pools have become the centre of the allocation
process. If anybody has any problems with this basic concept, please let me
know asap.

The next stage will be the creation of a header arena and memory pool for
constant strings.

Files changed: resources.h, resources.c, memory.c

--
Peter Gibbs
EmKel Systems


Index: include/parrot/resources.h
===================================================================
RCS file: /home/perlcvs/parrot/include/parrot/resources.h,v
retrieving revision 1.27
diff -u -r1.27 resources.h
--- include/parrot/resources.h 23 Apr 2002 19:41:15 -0000 1.27
+++ include/parrot/resources.h 29 Apr 2002 14:27:21 -0000
@@ -44,6 +44,8 @@

 void buffer_lives(Buffer *);

+void Parrot_initialize_free_pools(struct Parrot_Interp *);
+
 #define STRING_HEADERS_PER_ALLOC 128
 #define PMC_HEADERS_PER_ALLOC 128
 #define BUFFER_HEADERS_PER_ALLOC 128
@@ -77,6 +79,7 @@
 struct free_pool {
     Buffer pool_buffer;
     size_t entries_in_pool;
+    void (*replenish)(struct Parrot_Interp *, struct free_pool *);
 };

 struct Memory_Pool {
Index: resources.c
===================================================================
RCS file: /home/perlcvs/parrot/resources.c,v
retrieving revision 1.48
diff -u -r1.48 resources.c
--- resources.c 27 Apr 2002 19:31:08 -0000 1.48
+++ resources.c 29 Apr 2002 14:25:19 -0000
@@ -14,39 +14,77 @@

 #include "parrot/parrot.h"

-static void add_header_to_free(struct Parrot_Interp *interpreter,
-                               struct free_pool *pool, void *to_add);
-
-/* Add a string header to the free string header pool */
-static void add_pmc_to_free(struct Parrot_Interp *interpreter,
-                            struct free_pool *pool, void
-                            *to_add) {
-  PMC **temp_ptr;
-  /* First, check and see if there's enough space in the free pool. If
-   we're within the size of a STRING pointer, we make it bigger */
-  if (pool->entries_in_pool * sizeof(PMC *) >=
-      pool->pool_buffer.buflen - sizeof(PMC *)) {
-    /* If not, make the free pool bigger. We enlarge it by 20% */
-    Parrot_reallocate(interpreter,
-                      &pool->pool_buffer,
-                      (UINTVAL)(pool->pool_buffer.buflen * 1.2));
-
-  }
+/* Create a new free pool */
+static struct free_pool *
+new_free_pool(struct Parrot_Interp *interpreter, size_t entries,
+              void (*replenish)(struct Parrot_Interp *, struct free_pool
*)) {
+    struct free_pool *pool;
+    size_t temp_len;
+
+    pool = mem_sys_allocate(sizeof(struct free_pool));
+    temp_len = entries * sizeof(void *);
+    pool->pool_buffer.bufstart = mem_allocate(interpreter, &temp_len);
+    pool->pool_buffer.buflen = temp_len;
+    pool->pool_buffer.flags = BUFFER_live_FLAG;
+    pool->entries_in_pool = 0;
+    pool->replenish = replenish;
+    return pool;
+}
+
+/* Add entry to free pool
+ * Requires that any object-specific processing (eg flag setting,
statistics)
+ * has already been done by the caller
+ */
+static void
+add_to_free_pool(struct Parrot_Interp *interpreter,
+                 struct free_pool *pool, void *to_add) {
+    void **temp_ptr;
+
+    /* First, check and see if there's enough space in the free pool. If
+     we're within the size of a pointer, we make it bigger */
+    if (pool->entries_in_pool * sizeof(void *) >=
+        pool->pool_buffer.buflen - sizeof(void *)) {
+      /* If not, make the free pool bigger. We enlarge it by 20% */
+      Parrot_reallocate(interpreter,
+                        &pool->pool_buffer,
+                        (UINTVAL)(pool->pool_buffer.buflen * 1.2));
+    }
 #ifdef GC_DEBUG
     Parrot_go_collect(interpreter);
 #endif

-  /* Okay, so there's space. Add the header on */
-  ((PMC *)to_add)->flags = PMC_on_free_list_FLAG;
-  temp_ptr = pool->pool_buffer.bufstart;
-  temp_ptr += pool->entries_in_pool;
-  *temp_ptr = to_add;
-  pool->entries_in_pool++;
-
-  /* Note the numbers */
-  interpreter->active_PMCs--;
+    /* Okay, so there's space. Add the header on */
+    temp_ptr = pool->pool_buffer.bufstart;
+    temp_ptr += pool->entries_in_pool;
+    *temp_ptr = to_add;
+    pool->entries_in_pool++;
 }

+/* Get an entity from the specified free pool
+ * If the pool is empty, try a DOD sweep
+ * If the pool is still empty, call the replenishment function
+ */
+static void *
+get_from_free_pool(struct Parrot_Interp *interpreter,
+                   struct free_pool *pool) {
+    void ** ptr;
+
+    if (!pool->entries_in_pool) {
+        Parrot_do_dod_run(interpreter);
+    }
+
+    if (!pool->entries_in_pool) {
+        (*pool->replenish)(interpreter, pool);
+    }
+
+    if (!pool->entries_in_pool) {
+        return NULL;
+    }
+
+    ptr = pool->pool_buffer.bufstart;
+    ptr += --pool->entries_in_pool;
+    return *ptr;
+}

 /* Just go and get more headers unconditionally */
 void
@@ -75,32 +113,21 @@

   /* Note it in our stats */
   interpreter->total_PMCs += PMC_HEADERS_PER_ALLOC;
-  /* Yeah, this is skanky. They're not really active, but
-     add_pmc_to_free assumes that it's adding an active header to
-     the free list */
-  interpreter->active_PMCs += PMC_HEADERS_PER_ALLOC;

   cur_pmc = new_arena->start_PMC;
   for (i = 0; i < PMC_HEADERS_PER_ALLOC; i++) {
-    add_pmc_to_free(interpreter,
-                    interpreter->arena_base->pmc_pool,
-                    cur_pmc++);
+    cur_pmc->flags = PMC_on_free_list_FLAG;
+    add_to_free_pool(interpreter,
+                     interpreter->arena_base->pmc_pool,
+                     cur_pmc++);
   }
 }

 /* We have no more headers on the free header pool. Go allocate more
    and put them on */
-static void alloc_more_pmc_headers(struct Parrot_Interp *interpreter) {
-
-  /* First, try and find some unused headers */
-  Parrot_do_dod_run(interpreter);
+static void alloc_more_pmc_headers(struct Parrot_Interp *interpreter,
+                                   struct free_pool *pool) {

-  /* If we found some, then bail as we don't need to do anything */
-  if (interpreter->arena_base->pmc_pool->entries_in_pool) {
-    return;
-  }
-
-  /* We didn't find any, so go get more */
   Parrot_new_pmc_header_arena(interpreter);
 }

@@ -116,39 +143,17 @@
     return return_me;
   }

-  if (!interpreter->arena_base->pmc_pool->entries_in_pool) {
-    alloc_more_pmc_headers(interpreter);
-  }
-#ifdef GC_DEBUG
-  else {
-    Parrot_do_dod_run(interpreter);
-  }
-#endif
-
-    {
-    /* A stupid temp variable. Our pointer into the pool */
-    PMC **foo;
-    /* Set the pointer to the beginning of the pool */
-    foo =
-      interpreter->arena_base->pmc_pool->pool_buffer.bufstart;
-    /* Decrement the count of entries in the pool */
-    interpreter->arena_base->pmc_pool->entries_in_pool--;
-    /* Add the count of entries in the pool to the base
-       pointer. Conveniently this is both the offset into a zero-based
-       array and the count for the next time */
-    foo +=
-      interpreter->arena_base->pmc_pool->entries_in_pool;
-    /* Dereference the buffer pointer to get the real string pointer */
-    return_me = *foo;
-    /* Count that we've allocated it */
-    interpreter->active_PMCs++;
-    /* Mark it live */
-    return_me->flags = PMC_live_FLAG;
-    /* Don't let it point to garbage memory */
-    return_me->data = NULL;
-    /* Return it */
-    return return_me;
-  }
+  /* Get a PMC from the free pool */
+  return_me = get_from_free_pool(interpreter,
+                                 interpreter->arena_base->pmc_pool);
+  /* Count that we've allocated it */
+  interpreter->active_PMCs++;
+  /* Mark it live */
+  return_me->flags = PMC_live_FLAG;
+  /* Don't let it point to garbage memory */
+  return_me->data = NULL;
+  /* Return it */
+  return return_me;
 }

 void free_pmc(PMC *pmc) {
@@ -166,19 +171,12 @@
 /* We have no more headers on the free header pool. Go allocate more
    and put them on */
 static void
-alloc_more_buffer_headers(struct Parrot_Interp *interpreter) {
+alloc_more_buffer_headers(struct Parrot_Interp *interpreter,
+                                   struct free_pool *pool) {
   struct Buffer_Arena *new_arena;
   Buffer *cur_buffer;
   int i;

-  /* First, try and find some unused headers */
-  Parrot_do_dod_run(interpreter);
-
-  /* If we found some, then bail as we don't need to do anything */
-  if (interpreter->arena_base->buffer_header_pool->entries_in_pool) {
-    return;
-  }
-
   new_arena = mem_sys_allocate(sizeof(struct Buffer_Arena));
   new_arena->start_Buffer = mem_sys_allocate(sizeof(Buffer) *
BUFFER_HEADERS_PER_ALLOC);
   memset(new_arena->start_Buffer, 0, sizeof(Buffer) *
BUFFER_HEADERS_PER_ALLOC);
@@ -197,16 +195,13 @@

   /* Note it in our stats */
   interpreter->total_Buffers += BUFFER_HEADERS_PER_ALLOC;
-  /* Yeah, this is skanky. They're not really active, but
-     add_header_to_free assumes that it's adding an active header to
-     the free list */
-  interpreter->active_Buffers += BUFFER_HEADERS_PER_ALLOC;

   cur_buffer = new_arena->start_Buffer;
   for (i = 0; i < BUFFER_HEADERS_PER_ALLOC; i++) {
-    add_header_to_free(interpreter,
-                       interpreter->arena_base->buffer_header_pool,
-                       cur_buffer++);
+    cur_buffer->flags = BUFFER_on_free_list_FLAG;
+    add_to_free_pool(interpreter,
+                     interpreter->arena_base->buffer_header_pool,
+                     cur_buffer++);
   }
 }

@@ -223,42 +218,17 @@
     return return_me;
   }

-  if (!interpreter->arena_base->buffer_header_pool->entries_in_pool) {
-    alloc_more_buffer_headers(interpreter);
-  }
-#ifdef GC_DEBUG
-  else {
-    Parrot_do_dod_run(interpreter);
-  }
-#endif
-
-  /* Okay, we do this the long, drawn-out, hard way. Otherwise I get
-     really confused and things crash. This, generally, is a Bad
-     Thing. */
-  {
-    /* A stupid temp variable. Our pointer into the pool */
-    Buffer **foo;
-    /* Set the pointer to the beginning of the pool */
-    foo =
-      interpreter->arena_base->buffer_header_pool->pool_buffer.bufstart;
-    /* Decrement the count of entries in the pool */
-    interpreter->arena_base->buffer_header_pool->entries_in_pool--;
-    /* Add the count of entries in the pool to the base
-       pointer. Conveniently this is both the offset into a zero-based
-       array and the count for the next time */
-    foo +=
-      interpreter->arena_base->buffer_header_pool->entries_in_pool;
-    /* Dereference the buffer pointer to get the real string pointer */
-    return_me = *foo;
-    /* Count that we've allocated it */
-    interpreter->active_Buffers++;
-    /* Mark it live */
-    return_me->flags = BUFFER_live_FLAG;
-    /* Don't let it point to garbage memory */
-    return_me->bufstart = NULL;
-    /* Return it */
-    return return_me;
-  }
+  /* get buffer header from the free pool */
+  return_me = get_from_free_pool(interpreter,
+
interpreter->arena_base->buffer_header_pool);
+  /* Count that we've allocated it */
+  interpreter->active_Buffers++;
+  /* Mark it live */
+  return_me->flags = BUFFER_live_FLAG;
+  /* Don't let it point to garbage memory */
+  return_me->bufstart = NULL;
+  /* Return it */
+  return return_me;
 }

 void free_buffer(Buffer *thing) {
@@ -273,36 +243,6 @@

 }

-/* Add a buffer header to a free buffer header pool */
-static void add_header_to_free(struct Parrot_Interp *interpreter,
-                               struct free_pool *pool, void
-                               *to_add) {
-  Buffer **temp_ptr;
-  /* First, check and see if there's enough space in the free pool. If
-   we're within the size of a STRING pointer, we make it bigger */
-  if (pool->entries_in_pool * sizeof(STRING *) >=
-      pool->pool_buffer.buflen - sizeof(STRING *)) {
-    /* If not, make the free pool bigger. We enlarge it by 20% */
-    Parrot_reallocate(interpreter,
-                      &pool->pool_buffer,
-                      (UINTVAL)(pool->pool_buffer.buflen * 1.2));
-
-  }
-#ifdef GC_DEBUG
-  Parrot_go_collect(interpreter);
-#endif
-
-  /* Okay, so there's space. Add the header on */
-  ((Buffer *)to_add)->flags = BUFFER_on_free_list_FLAG;
-  temp_ptr = pool->pool_buffer.bufstart;
-  temp_ptr += pool->entries_in_pool;
-  *temp_ptr = to_add;
-  pool->entries_in_pool++;
-
-  /* Note the numbers */
-  interpreter->active_Buffers--;
-}
-
 /* Mark all the PMCs as not in use.

 */
@@ -544,7 +484,9 @@
       /* If it's not live or on the free list, put it on the free list */
       if (!(pmc_array[i].flags & (PMC_live_FLAG | PMC_immune_FLAG |
                                   PMC_on_free_list_FLAG))) {
-        add_pmc_to_free(interpreter,
+        interpreter->active_PMCs--;
+        pmc_array[i].flags = PMC_on_free_list_FLAG;
+        add_to_free_pool(interpreter,
                         interpreter->arena_base->pmc_pool,
                         &pmc_array[i]);
       }
@@ -568,9 +510,11 @@
       /* If it's not live or on the free list, put it on the free list */
       if (!(string_array[i].flags & (BUFFER_live_FLAG |
                                      BUFFER_on_free_list_FLAG))) {
-        add_header_to_free(interpreter,
-                           interpreter->arena_base->string_header_pool,
-                           &string_array[i]);
+        interpreter->active_Buffers--;
+        string_array[i].flags = BUFFER_on_free_list_FLAG;
+        add_to_free_pool(interpreter,
+                         interpreter->arena_base->string_header_pool,
+                         &string_array[i]);
       }
     }
   }
@@ -611,19 +555,12 @@

 /* We have no more headers on the free header pool. Go allocate more
    and put them on */
-static void alloc_more_string_headers(struct Parrot_Interp *interpreter) {
+static void alloc_more_string_headers(struct Parrot_Interp *interpreter,
+                                   struct free_pool *pool) {
   struct STRING_Arena *new_arena;
   STRING *cur_string;
   int i;

-  /* First, try and find some unused headers */
-  Parrot_do_dod_run(interpreter);
-
-  /* If we found some, then bail as we don't need to do anything */
-  if (interpreter->arena_base->string_header_pool->entries_in_pool) {
-    return;
-  }
-
   new_arena = mem_sys_allocate(sizeof(struct STRING_Arena));
   new_arena->start_STRING = mem_sys_allocate(sizeof(STRING) *
STRING_HEADERS_PER_ALLOC);
   memset(new_arena->start_STRING, 0, sizeof(STRING) *
STRING_HEADERS_PER_ALLOC);
@@ -642,16 +579,13 @@

   /* Note it in our stats */
   interpreter->total_Buffers += STRING_HEADERS_PER_ALLOC;
-  /* Yeah, this is skanky. They're not really active, but
-     add_header_to_free assumes that it's adding an active header to
-     the free list */
-  interpreter->active_Buffers += STRING_HEADERS_PER_ALLOC;

   cur_string = new_arena->start_STRING;
   for (i = 0; i < STRING_HEADERS_PER_ALLOC; i++) {
-    add_header_to_free(interpreter,
-                       interpreter->arena_base->string_header_pool,
-                       cur_string++);
+    cur_string->flags = BUFFER_on_free_list_FLAG;
+    add_to_free_pool(interpreter,
+                     interpreter->arena_base->string_header_pool,
+                     cur_string++);
   }
 }

@@ -667,42 +601,35 @@
     return return_me;
   }

-  if (!interpreter->arena_base->string_header_pool->entries_in_pool) {
-      alloc_more_string_headers(interpreter);
-  }
-#ifdef GC_DEBUG
-  else {
-    Parrot_do_dod_run(interpreter);
-  }
-#endif
-
-  /* Okay, we do this the long, drawn-out, hard way. Otherwise I get
-     really confused and things crash. This, generally, is a Bad
-     Thing. */
-  {
-    /* A stupid temp variable. Our pointer into the pool */
-    STRING **foo;
-    /* Set the pointer to the beginning of the pool */
-    foo =
-      interpreter->arena_base->string_header_pool->pool_buffer.bufstart;
-    /* Decrement the count of entries in the pool */
-    interpreter->arena_base->string_header_pool->entries_in_pool--;
-    /* Add the count of entries in the pool to the base
-       pointer. Conveniently this is both the offset into a zero-based
-       array and the count for the next time */
-    foo +=
-      interpreter->arena_base->string_header_pool->entries_in_pool;
-    /* Dereference the buffer pointer to get the real string pointer */
-    return_me = *foo;
-    /* Count that we've allocated it */
-    interpreter->active_Buffers++;
-    /* Mark it live */
-    return_me->flags = BUFFER_live_FLAG;
-    /* Don't let it point to garbage memory */
-    return_me->bufstart = NULL;
-    /* Return it */
-    return return_me;
-  }
+  /* Get string header from the free pool */
+  return_me = get_from_free_pool(interpreter,
+
interpreter->arena_base->string_header_pool);
+  /* Count that we've allocated it */
+  interpreter->active_Buffers++;
+  /* Mark it live */
+  return_me->flags = BUFFER_live_FLAG;
+  /* Don't let it point to garbage memory */
+  return_me->bufstart = NULL;
+  /* Return it */
+  return return_me;
+}
+
+/* Initialize the free pools for the tracked resources
+ * The 3rd parameter to new_free_pool is the function to be called when
+ *   the pool is empty */
+void
+Parrot_initialize_free_pools(struct Parrot_Interp *interpreter) {
+    /* Init the string header pool */
+    interpreter->arena_base->string_header_pool =
+        new_free_pool(interpreter, 256, alloc_more_string_headers);
+
+    /* Init the buffer header pool */
+    interpreter->arena_base->buffer_header_pool =
+        new_free_pool(interpreter, 256, alloc_more_buffer_headers);
+
+    /* Init the PMC header pool */
+    interpreter->arena_base->pmc_pool =
+        new_free_pool(interpreter, 256, alloc_more_pmc_headers);
 }

 /* Figure out how much memory's been allocated total for buffered
Index: memory.c
===================================================================
RCS file: /home/perlcvs/parrot/memory.c,v
retrieving revision 1.31
diff -u -r1.31 memory.c
--- memory.c 15 Apr 2002 18:05:18 -0000 1.31
+++ memory.c 29 Apr 2002 14:27:02 -0000
@@ -108,37 +108,8 @@
     /* Allocate a first default-sized block of memory */
     Parrot_alloc_new_block(interpreter, 0, 1);

-    /* Init the string header pool */
-    interpreter->arena_base->string_header_pool =
-        mem_sys_allocate(sizeof(struct free_pool));
-    temp_len = 1024;
-    interpreter->arena_base->string_header_pool->pool_buffer.bufstart =
-        mem_allocate(interpreter, &temp_len);
-    interpreter->arena_base->string_header_pool->pool_buffer.flags =
-        BUFFER_live_FLAG;
-    interpreter->arena_base->string_header_pool->pool_buffer.buflen =
temp_len;
-    interpreter->arena_base->string_header_pool->entries_in_pool = 0;
-
-    /* Init the buffer header pool */
-    interpreter->arena_base->buffer_header_pool =
-        mem_sys_allocate(sizeof(struct free_pool));
-    interpreter->arena_base->buffer_header_pool->pool_buffer.bufstart =
-        mem_allocate(interpreter, &temp_len);
-    interpreter->arena_base->buffer_header_pool->pool_buffer.flags =
-        BUFFER_live_FLAG;
-    interpreter->arena_base->buffer_header_pool->pool_buffer.buflen =
temp_len;
-    interpreter->arena_base->buffer_header_pool->entries_in_pool = 0;
-
-    /* Init the PMC header pool */
-    interpreter->arena_base->pmc_pool =
-        mem_sys_allocate(sizeof(struct free_pool));
-    interpreter->arena_base->pmc_pool->pool_buffer.bufstart =
-        mem_allocate(interpreter, &temp_len);
-    interpreter->arena_base->pmc_pool->pool_buffer.flags =
-        BUFFER_live_FLAG;
-    interpreter->arena_base->pmc_pool->pool_buffer.buflen = temp_len;
-    interpreter->arena_base->pmc_pool->entries_in_pool = 0;
-    Parrot_new_pmc_header_arena(interpreter);
+    /* Initialize the free pools */
+    Parrot_initialize_free_pools(interpreter);
 }

 /*



Reply via email to