[PATCH] Fix zone collector thinko on my part

[Steven Bosscher]

Hi,

A couple of weeks ago I posted a patch for the zone collector that
allowed us to create and destroy zones dynamically.  The function to
destroy a zone had a check to make sure the zone that was about to
be freed was empty.

I thought a zone would always be empty if nothing would point to
anything in it after a ggc_collect call.  That was a mistake because
we do not always really collect when ggc_collect() is called.  It
depends on some heuristics whether we do collect or we don't.

This wrong assumption makes the existing code almost useless.  No
real issue for mainline, but on tree-ssa I have patches ready that
actually uses this stuff.

I'd like to have this in mainline because it is definitely safe (only
touches unused code) and it allows us to keep ggc-zone.c in sync on
mainline and the tree-ssa branch.

Bootstrapped & tested with the zone collector enabled.  OK?

Gr.
Steven

	* ggc-zone.c (struct alloc_zone): Don't pre-declare, it already
	comes in with ggc.h.  Add a new bool field `dead'.
	(destroy_ggc_zone): Don't destroy a zone at once.  Instead, only
	set the `dead' flag for the dead zone.  Wrap a sanity check in
	ENABLE_CHECKING.
	(ggc_collect_1): Always mark and sweep if a zone has the `dead'
	flag set.
	(ggc_collect): Free dead zones after collecting.

Index: ggc-zone.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-zone.c,v
retrieving revision 2.6
diff -c -3 -p -r2.6 ggc-zone.c
*** ggc-zone.c	1 Dec 2003 22:59:29 -0000	2.6
--- ggc-zone.c	13 Dec 2003 00:32:46 -0000
*************** struct max_alignment {
*** 244,250 ****
  #define LOOKUP_L2(p) \
    (((size_t) (p) >> G.lg_pagesize) & ((1 << PAGE_L2_BITS) - 1))
  
- struct alloc_zone;
  /* A page_entry records the status of an allocation page.  */
  typedef struct page_entry
  {
--- 244,249 ----
*************** struct alloc_zone
*** 382,389 ****
    /* Next zone in the linked list of zones.  */
    struct alloc_zone *next_zone;
  
!   /* Return true if this zone was collected during this collection.  */
    bool was_collected;
  } main_zone;
  
  struct alloc_zone *rtl_zone;
--- 381,391 ----
    /* Next zone in the linked list of zones.  */
    struct alloc_zone *next_zone;
  
!   /* True if this zone was collected during this collection.  */
    bool was_collected;
+ 
+   /* True if this zone should be destroyed after the next collection.  */
+   bool dead;
  } main_zone;
  
  struct alloc_zone *rtl_zone;
*************** destroy_ggc_zone (struct alloc_zone * de
*** 1215,1231 ****
    for (z = G.zones; z && z->next_zone != dead_zone; z = z->next_zone)
      /* Just find that zone.  */ ;
  
!   /* We should have found the zone in the list.  Anything else is
!      fatal.
!      If we did find the zone, we expect this zone to be empty.
!      A ggc_collect should have emptied it before we can destroy it.  */
!   if (!z || dead_zone->allocated != 0)
      abort ();
  
!   /* Unchain the dead zone, release all its pages and free it.  */
!   z->next_zone = z->next_zone->next_zone;
!   release_pages (dead_zone);
!   free (dead_zone);
  }
  
  /* Increment the `GC context'.  Objects allocated in an outer context
--- 1217,1230 ----
    for (z = G.zones; z && z->next_zone != dead_zone; z = z->next_zone)
      /* Just find that zone.  */ ;
  
! #ifdef ENABLE_CHECKING
!   /* We should have found the zone in the list.  Anything else is fatal.  */
!   if (!z)
      abort ();
+ #endif
  
!   /* z is dead, baby. z is dead.  */
!   z->dead= true;
  }
  
  /* Increment the `GC context'.  Objects allocated in an outer context
*************** sweep_pages (struct alloc_zone *zone)
*** 1408,1426 ****
  static bool
  ggc_collect_1 (struct alloc_zone *zone, bool need_marking)
  {
!   /* Avoid frequent unnecessary work by skipping collection if the
!      total allocations haven't expanded much since the last
!      collection.  */
!   float allocated_last_gc =
!     MAX (zone->allocated_last_gc, (size_t)PARAM_VALUE (GGC_MIN_HEAPSIZE) * 1024);
  
!   float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (zone->allocated < allocated_last_gc + min_expand)
!     return false;
  
    if (!quiet_flag)
!     fprintf (stderr, " {%s GC %luk -> ", zone->name, (unsigned long) zone->allocated / 1024);
  
    /* Zero the total allocated bytes.  This will be recalculated in the
       sweep phase.  */
--- 1407,1430 ----
  static bool
  ggc_collect_1 (struct alloc_zone *zone, bool need_marking)
  {
!   if (!zone->dead)
!     {
!       /* Avoid frequent unnecessary work by skipping collection if the
! 	 total allocations haven't expanded much since the last
! 	 collection.  */
!       float allocated_last_gc =
! 	MAX (zone->allocated_last_gc,
! 	     (size_t) PARAM_VALUE (GGC_MIN_HEAPSIZE) * 1024);
  
!       float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!       if (zone->allocated < allocated_last_gc + min_expand)
! 	return false;
!     }
  
    if (!quiet_flag)
!     fprintf (stderr, " {%s GC %luk -> ",
! 	     zone->name, (unsigned long) zone->allocated / 1024);
  
    /* Zero the total allocated bytes.  This will be recalculated in the
       sweep phase.  */
*************** ggc_collect_1 (struct alloc_zone *zone, 
*** 1439,1445 ****
    zone->was_collected = true;
    zone->allocated_last_gc = zone->allocated;
  
- 
    if (!quiet_flag)
      fprintf (stderr, "%luk}", (unsigned long) zone->allocated / 1024);
    return true;
--- 1443,1448 ----
*************** ggc_collect (void)
*** 1586,1591 ****
--- 1589,1615 ----
  	  }
        }
      }
+ 
+   /* Free dead zones.  */
+   for (zone = G.zones; zone && zone->next_zone; zone = zone->next_zone)
+     {
+       if (zone->next_zone->dead)
+ 	{
+ 	  struct alloc_zone *dead_zone = zone->next_zone;
+ 
+ 	  printf ("Zone `%s' is dead and will be freed.\n", dead_zone->name);
+ 
+ 	  /* The zone must be empty.  */
+ 	  if (dead_zone->allocated != 0)
+ 	    abort ();
+ 
+ 	  /* Unchain the dead zone, release all its pages and free it.  */
+ 	  zone->next_zone = zone->next_zone->next_zone;
+ 	  release_pages (dead_zone);
+ 	  free (dead_zone);
+ 	}
+     }
+ 
    timevar_pop (TV_GC);
  }