Hi all,

I've attached a small patch to kaffe's gc and would like to
hear your opinions before commiting it.

On the one hand, it provides for a cleaner seperation between
heap management and garbage collection. Therefore, it moves any
logic how one could free some memory out of gc_heap_malloc into
gcMalloc. Second, gc_system_alloc is renamed to gc_heap_grow and
exported so that gcMalloc can call it.

On the other hand, it contains some bugfixes. The memory that's
allocated by the heap manager is no longer aligned at a hard coded
8 byte boundary, but rather at a ALIGNMENT_OF_SIZE(sizeof(jdouble))
boundary. Second, the initialization of a block in gc_small_block
was somewhat wrong (think that's what's causing the SIGBUS on
HPUX).

Comments?

Greetings,
Helmer
Index: kaffe/kaffevm/mem/gc-incremental.c
===================================================================
RCS file: /cvs/kaffe/kaffe/kaffe/kaffevm/mem/gc-incremental.c,v
retrieving revision 1.62
diff -u -r1.62 gc-incremental.c
--- kaffe/kaffevm/mem/gc-incremental.c  11 Mar 2003 08:00:18 -0000      1.62
+++ kaffe/kaffevm/mem/gc-incremental.c  12 Jun 2003 07:58:15 -0000
@@ -860,19 +860,53 @@
        int i;
        size_t bsz;
        int iLockRoot;
+       int times = 0;
 
        assert(gc_init != 0);
        assert(fidx < nrTypes && size != 0);
 
-       unit = gc_heap_malloc(size + sizeof(gc_unit));
-
-       /* keep pointer to object */
-       mem = UTOMEM(unit);
-       if (unit == 0) {
-               return 0;
-       }
+       size += sizeof(gc_unit);
 
        lockStaticMutex(&gc_lock);
+
+       for (unit=0; unit==0;) {
+               times++;
+               unit = gc_heap_malloc(size);
+       
+               /* keep pointer to object */
+               mem = UTOMEM(unit);
+               if (unit == 0) {
+                       switch (times) {
+                       case 1:
+                               /* Try invoking GC if it is available */
+                               if (garbageman != 0) {
+                                       unlockStaticMutex(&gc_lock);
+                                       adviseGC();
+                                       lockStaticMutex(&gc_lock);
+                               }
+                               break;    
+
+                       case 2:
+                               /* Grow the heap */
+                               gc_heap_grow(size);
+                               break;
+
+                       default:
+                               if (DBGEXPR(CATCHOUTOFMEM, true, false)) {
+                                       /*
+                                        * If we ran out of memory, a 
OutOfMemoryException is
+                                        * thrown.  If we fail to allocate memory for 
it, all
+                                        * is lost.
+                                        */
+                                       static int ranout;
+                                       assert (ranout++ == 0 || !!!"Ran out of 
memory!");
+                               }
+                               /* Guess we've really run out */
+                               unlockStaticMutex(&gc_lock);
+                               return (0);
+                       }
+               }
+       }
 
        info = GCMEM2BLOCK(mem);
        i = GCMEM2IDX(info, unit);
Index: kaffe/kaffevm/mem/gc-mem.c
===================================================================
RCS file: /cvs/kaffe/kaffe/kaffe/kaffevm/mem/gc-mem.c,v
retrieving revision 1.43
diff -u -r1.43 gc-mem.c
--- kaffe/kaffevm/mem/gc-mem.c  2 Jun 2003 14:15:47 -0000       1.43
+++ kaffe/kaffevm/mem/gc-mem.c  12 Jun 2003 07:58:16 -0000
@@ -41,7 +41,6 @@
 
 static gc_block* gc_primitive_alloc(size_t);
 void gc_primitive_free(gc_block*);
-static void* gc_system_alloc(size_t);
 
 uintp gc_heap_base;
 uintp gc_block_base;
@@ -206,7 +205,7 @@
         * of powers of two
         */
 #define        OBJSIZE(NR) \
-       ((gc_pgsize-GCBLOCK_OVH-ROUNDUPALIGN(1)-(NR*(2+sizeof(void*))))/NR)
+       ((gc_pgsize-ROUNDUPALIGN(1)-(NR*(2+sizeof(void*))))/NR)
 
        /* For a given number of tiles in a block, work out the size of
         * the allocatable units which'll fit in them and build a translation
@@ -280,11 +279,10 @@
        gc_heap_initial_size = ROUNDUPPAGESIZE(gc_heap_initial_size);
 
        /* allocate heap of initial size from system */
-       gc_system_alloc(gc_heap_initial_size);
+       gc_heap_grow(gc_heap_initial_size);
 }
 
-/*
- * gc_heap_malloc
+/**
  * Allocate a piece of memory.
  */
 void*
@@ -292,11 +290,10 @@
 {
        static int gc_heap_init = 0;
        size_t lnr;
-       gc_freeobj* mem;
+       gc_freeobj* mem = 0;
        gc_block** mptr;
        gc_block* blk;
        size_t nsz;
-       int times;
        int iLockRoot;
 
        /* Initialise GC heap first time in - we must assume single threaded
@@ -309,8 +306,6 @@
 
        lockStaticMutex(&gc_heap_lock);
 
-       times = 0;
-
 DBG(SLACKANAL,
        if (GC_SMALL_OBJECT(sz)) {
                totalslack += (freelist[sztable[sz].list].sz - sz);
@@ -318,9 +313,6 @@
        }
     )
 
-       rerun:;
-       times++;
-
 DBG(GCDIAG, 
        gc_heap_check();
     )
@@ -342,8 +334,7 @@
                else {
                        blk = gc_small_block(nsz);
                        if (blk == 0) {
-                               nsz = gc_pgsize;
-                               goto nospace;
+                               goto out;
                        }
                        blk->next = *mptr;
                        *mptr = blk;
@@ -378,9 +369,7 @@
                nsz = sz;
                blk = gc_large_block(nsz);
                if (blk == 0) {
-                       nsz = nsz + GCBLOCK_OVH + sizeof(gcFuncs*) + ROUNDUPALIGN(1);
-                       nsz = ROUNDUPPAGESIZE(nsz);
-                       goto nospace;
+                       goto out;
                }
                mem = GCBLOCK2FREE(blk, 0);
                GC_SET_STATE(blk, 0, GC_STATE_NORMAL);
@@ -395,60 +384,13 @@
 
        assert(GC_OBJECT_SIZE(mem) >= sz);
 
+       out:
        unlockStaticMutex(&gc_heap_lock);
 
        return (mem);
-
-       /* --------------------------------------------------------------- */
-       nospace:;
-
-       /* Failed to find space in any freelists. Must try to get the
-        * memory from somewhere.
-        */
-
-       switch (times) {
-       case 1:
-               /* Try invoking GC if it is available */
-               if (garbageman != 0) {
-                       /* The other caller of invokeGC,  Runtime.gc() can't 
-                        * give up this lock on its own, since it does not 
-                        * hold this lock.
-                        */
-                       unlockStaticMutex(&gc_heap_lock);
-                       adviseGC();
-                       lockStaticMutex(&gc_heap_lock);
-               }
-               break;
-
-       case 2:
-               /* Get from the system */
-               if (nsz < gc_heap_allocation_size) {
-                       nsz = gc_heap_allocation_size;
-               }
-               gc_system_alloc(nsz);
-               break;
-
-       default:
-               if (DBGEXPR(CATCHOUTOFMEM, true, false))
-               {
-                       /*
-                        * If we ran out of memory, a OutOfMemoryException is
-                        * thrown.  If we fail to allocate memory for it, all
-                        * is lost.
-                        */
-                       static int ranout;
-                       assert (ranout++ == 0 || !!!"Ran out of memory!");
-               }
-               /* Guess we've really run out */
-               unlockStaticMutex(&gc_heap_lock);
-               return (0);
-       }
-
-       /* Try again */
-       goto rerun;
 }
 
-/*
+/**
  * Free a piece of memory.
  */
 void
@@ -523,7 +465,7 @@
        }
        else {
                /* Calculate true size of block */
-               msz = info->size + GCBLOCK_OVH + ROUNDUPALIGN(1);
+               msz = info->size + ROUNDUPALIGN(1);
                msz = ROUNDUPPAGESIZE(msz);
                info->size = msz;
                gc_primitive_free(info);
@@ -555,7 +497,7 @@
        }
 
        /* Calculate number of objects in this block */
-       nr = (gc_pgsize-GCBLOCK_OVH-ROUNDUPALIGN(1))/(sz+2);
+       nr = (gc_pgsize-ROUNDUPALIGN(1))/(sz+2);
 
        /* Setup the meta-data for the block */
        DBG(GCDIAG, info->magic = GC_MAGIC);
@@ -570,12 +512,14 @@
        DBG(GCDIAG, memset(info->data, 0, sz * nr));
 
        /* Build the objects into a free list */
-       for (i = nr-1; i >= 0; i--) {
+       for (i = nr-1; i-- > 0;) {
                GCBLOCK2FREE(info, i)->next = GCBLOCK2FREE(info, i+1);
                GC_SET_COLOUR(info, i, GC_COLOUR_FREE);
                GC_SET_STATE(info, i, GC_STATE_NORMAL);
        }
        GCBLOCK2FREE(info, nr-1)->next = 0;
+       GC_SET_COLOUR(info, nr-1, GC_COLOUR_FREE);
+       GC_SET_STATE(info, nr-1, GC_STATE_NORMAL);
        info->free = GCBLOCK2FREE(info, 0);
 DBG(SLACKANAL,
        int slack = ((void *)info) 
@@ -596,7 +540,7 @@
        size_t msz;
 
        /* Add in management overhead */
-       msz = sz+GCBLOCK_OVH+2+ROUNDUPALIGN(1);
+       msz = sz+2+ROUNDUPALIGN(1);
        /* Round size up to a number of pages */
        msz = ROUNDUPPAGESIZE(msz);
 
@@ -1005,17 +949,33 @@
        return GCMEM2BLOCK(heap_addr);
 }
 
-static
-void*
-gc_system_alloc(size_t sz)
+/**
+ * Grows the heap.
+ *
+ * @param sz minimum number of bytes to grow.
+ * @return 0 in case of an error, otherwise != 0
+ */
+void *
+gc_heap_grow(size_t sz)
 {
        gc_block* blk;
 
+       if (GC_SMALL_OBJECT(sz)) {
+               sz = gc_pgsize;
+       } else {
+               sz = sz + 2 + ROUNDUPALIGN(1);
+               sz = ROUNDUPPAGESIZE(sz);
+       }
+
+       if (sz < gc_heap_allocation_size) {
+               sz = gc_heap_allocation_size;
+       }
+
        assert(sz % gc_pgsize == 0);
 
        if (gc_heap_total == gc_heap_limit) {
                return (0);
-       } else  if (gc_heap_total + sz > gc_heap_limit) {
+       } else if (gc_heap_total + sz > gc_heap_limit) {
                /* take as much memory as we can */
                sz = gc_heap_limit - gc_heap_total;
                assert(sz % gc_pgsize == 0);
@@ -1026,13 +986,14 @@
 #endif
 
        blk = gc_block_alloc(sz);
-       
-DBG(GCSYSALLOC,
-       dprintf("gc_system_alloc: %ld byte at %p\n", (long) sz, blk);           )
+
+       DBG(GCSYSALLOC,
+           dprintf("gc_system_alloc: %ld byte at %p\n", (long) sz, blk); )
 
        if (blk == 0) {
                return (0);
        }
+
        gc_heap_total += sz;
        assert(gc_heap_total <= gc_heap_limit);
 
Index: kaffe/kaffevm/mem/gc-mem.h
===================================================================
RCS file: /cvs/kaffe/kaffe/kaffe/kaffevm/mem/gc-mem.h,v
retrieving revision 1.15
diff -u -r1.15 gc-mem.h
--- kaffe/kaffevm/mem/gc-mem.h  2 Jun 2003 14:15:47 -0000       1.15
+++ kaffe/kaffevm/mem/gc-mem.h  12 Jun 2003 07:58:16 -0000
@@ -14,6 +14,12 @@
 #ifndef __gc_mem_h
 #define        __gc_mem_h
 
+#include "md.h"
+
+#if !defined(ALIGNMENT_OF_SIZE)
+#define ALIGNMENT_OF_SIZE(S)   (S)
+#endif
+
 #ifndef gc_pgsize
 extern size_t gc_pgsize;
 extern int gc_pgbits;
@@ -44,7 +50,7 @@
  * Alignment for gc_blocks
  *
  */
-#define        MEMALIGN                8
+#define        MEMALIGN                (ALIGNMENT_OF_SIZE(sizeof(jdouble)))
 
 /**
  * rounds @V up to the next MEMALIGN boundary. 
@@ -69,6 +75,8 @@
 extern void*   gc_heap_malloc(size_t);    
 extern void    gc_heap_free(void*);
 
+extern void*   gc_heap_grow(size_t);
+
 /**
  * Evaluates to the size of the object that contains address @M.
  *
@@ -154,14 +162,12 @@
  * Evaluates to the first usable address in gc_block @B.
  *
  */ 
-#define GCBLOCK2BASE(B)        (((char *)gc_heap_base)         \
-                        + gc_pgsize * ((B) - GC_BLOCKS))
-
+#define GCBLOCK2BASE(B)                (((char *)gc_heap_base) \
+                                        + gc_pgsize * ((B) - GC_BLOCKS))
 
 /* This is OK, gc_prim_(alloc|free) never assume GCBLOCKEND is really
    a valid block */
 #define GCBLOCKEND(B)          ((B) + (((B)->size+gc_pgsize-1)>>gc_pgbits))
-#define GCBLOCK_OVH            0
 
 #define ASSERT_ONBLOCK(OBJ, BLK) assert(GCMEM2BLOCK(OBJ) == BLK)
 

Reply via email to