public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Make per-call statistics to take into account ggc_free
@ 2004-03-05 18:25 Jan Hubicka
  2004-03-19  8:14 ` Jan Hubicka
  2004-09-02 16:12 ` Better memory statistics, take 2 Jan Hubicka
  0 siblings, 2 replies; 16+ messages in thread
From: Jan Hubicka @ 2004-03-05 18:25 UTC (permalink / raw)
  To: gcc-patches, zack, rth

Hi,
this patch improves the per-line statistics by tracking down each allocated
entity to figure out whether it will be freed, garbage collected or leaked.
To rule out ggc_freed values is pretty important as these are much cheaper,
similarly I used leaks to figure out why C++ frontend use that much of memory.

The tracking is implemented via ptr_hash that converts pointers to records and
by forcing one garbage collection run before outputting the data so leaks are
accurate.
For combine.c we now get:

emit-rtl.c:738 (gen_reg_rtx)                         203904: 0.9%     118944:46.1%          0: 0.0%      89376: 7.8%         54
c-decl.c:4338 (grokdeclarator)                        29160: 0.1%          0: 0.0%     316656:10.8%          0: 0.0%       3202
ggc-common.c:192 (ggc_calloc)                        332480: 1.5%          0: 0.0%      51536: 1.7%       4240: 0.4%        575
optabs.c:4913 (new_convert_optab)                         0: 0.0%          0: 0.0%     416916:14.2%     122004:10.7%          9
tree.c:4010 (build_function_type)                    348672: 1.5%          0: 0.0%     135808: 4.6%      75700: 6.6%       3785
convert.c:371 (convert_to_integer)                   517980: 2.3%          0: 0.0%       1480: 0.1%          0: 0.0%      25973
c-parse.y:748 (yyparse)                              521360: 2.3%          0: 0.0%          0: 0.0%          0: 0.0%      26068
c-parse.y:447 (yyparse)                              533540: 2.3%          0: 0.0%        480: 0.0%          0: 0.0%      26701
tree.c:406 (build_int_2_wide)                        529060: 2.3%          0: 0.0%      53020: 1.8%          0: 0.0%      29104
emit-rtl.c:3367 (make_jump_insn_raw)                 643524: 2.8%          0: 0.0%          0: 0.0%     153220:13.4%       7661
genrtl.c:635 (gen_rtx_fmt_i00)                       670160: 2.9%          0: 0.0%       2416: 0.1%          0: 0.0%      42036
genrtl.c:671 (gen_rtx_fmt_e0)                        688752: 3.0%          0: 0.0%      15036: 0.5%          0: 0.0%      58649
stringpool.c:70 (alloc_node)                              0: 0.0%          0: 0.0%     716520:24.3%     170600:15.0%       8530
c-decl.c:4270 (grokdeclarator)                       785160: 3.4%          0: 0.0%      18576: 0.6%          0: 0.0%       7442
varray.c:128 (varray_init)                           793716: 3.5%      15340: 6.0%      10948: 0.4%     211316:18.5%        591
cselib.c:840 (cselib_subst_to_values)                992512: 4.3%          0: 0.0%          0: 0.0%          0: 0.0%      86828
emit-rtl.c:3335 (make_insn_raw)                     1501800: 6.6%          0: 0.0%         40: 0.0%          0: 0.0%      37546
genrtl.c:51 (gen_rtx_fmt_ue)                        1574316: 6.9%          0: 0.0%          0: 0.0%          0: 0.0%     131193
rtl.c:250 (copy_rtx)                                1723556: 7.5%          0: 0.0%        576: 0.0%          0: 0.0%     141955
genrtl.c:33 (gen_rtx_fmt_ee)                        3201600:14.0%          0: 0.0%         36: 0.0%          0: 0.0%     266803
Total                                              22904228           257740          2945464          1139300          1281616
source location                                     Garbage            Freed             Leak         Overhead            Times

So only 257KB is explicitely freed right now.  I will try to add few extra calls.

Honza

Bootstrapped/regtested i686-pc-gnu-linux, OK?
2004-03-05  Jan Hubicka  <jh@suse.cz>
	* ggc-common.c (ggc_force_collect): New global variable.
	(loc_descroptor): New fields freed, collected.
	(ptr_hash): New global variable.
	(ptr_hash_entry): New structure
	(hash_ptr, eq_ptr): New functions.
	(ggc_record_overhead): New argument ptr; record entry to ptr hash.
	(ggc_prune_ptr): New static function.
	(ggc_prune_overhead, ggc_free_overhead): New functions.
	(cmp_statistics): Compare non-freed memory.
	(dump_ggc_loc_statistics): Force collection; print new values.
	* ggc-page.c (ggc_alloc_stat): Update call of
	ggc_recored_overhead.
	(ggc_free): Use ggc_free_overhead.
	(ggc_collect): Obey ggc_force_collect and use
	ggc_prune_overhead_list.
	* ggc.h (ggc_force_collect): Declare.
	(ggc_record_overhead): Update prototype.
	(ggc_free_overhead, ggc_prune_overhead_list): Declare.
Index: ggc-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-common.c,v
retrieving revision 1.83
diff -c -3 -p -r1.83 ggc-common.c
*** ggc-common.c	3 Mar 2004 11:25:47 -0000	1.83
--- ggc-common.c	5 Mar 2004 17:56:21 -0000
*************** Software Foundation, 59 Temple Place - S
*** 60,65 ****
--- 60,68 ----
  #define VALGRIND_DISCARD(x)
  #endif
  
+ /* When set, ggc_collect will do collection.  */
+ bool ggc_force_collect;
+ 
  /* Statistics about the allocation.  */
  static ggc_statistics *ggc_stats;
  
*************** struct loc_descriptor
*** 773,778 ****
--- 776,783 ----
    int times;
    size_t allocated;
    size_t overhead;
+   size_t freed;
+   size_t collected;
  };
  
  /* Hashtable used for statistics.  */
*************** eq_descriptor (const void *p1, const voi
*** 797,802 ****
--- 802,833 ----
  	  && d->function == d2->function);
  }
  
+ /* Hashtable converting address of allocated field to loc descriptor.  */
+ static htab_t ptr_hash;
+ struct ptr_hash_entry
+ {
+   void *ptr;
+   struct loc_descriptor *loc;
+   size_t size;
+ };
+ 
+ /* Hash table helpers functions.  */
+ static hashval_t
+ hash_ptr (const void *p)
+ {
+   const struct ptr_hash_entry *d = p;
+ 
+   return htab_hash_pointer (d->ptr);
+ }
+ 
+ static int
+ eq_ptr (const void *p1, const void *p2)
+ {
+   const struct ptr_hash_entry *p = p1;
+ 
+   return (p->ptr == p2);
+ }
+ 
  /* Return descriptor for given call site, create new one if needed.  */
  static struct loc_descriptor *
  loc_descriptor (const char *name, int line, const char *function)
*************** loc_descriptor (const char *name, int li
*** 821,843 ****
  }
  
  /* Record ALLOCATED and OVERHEAD bytes to descritor NAME:LINE (FUNCTION).  */
! void ggc_record_overhead (size_t allocated, size_t overhead,
  			  const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return (l1->allocated + l1->overhead) - (l2->allocated + l2->overhead);
  }
  
  /* Collect array of the descriptors from hashtable.  */
--- 852,921 ----
  }
  
  /* Record ALLOCATED and OVERHEAD bytes to descritor NAME:LINE (FUNCTION).  */
! void ggc_record_overhead (size_t allocated, size_t overhead, void *ptr,
  			  const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
+   struct ptr_hash_entry *p = xmalloc (sizeof (struct ptr_hash_entry));
+   PTR *slot;
+ 
+   p->ptr = ptr;
+   p->loc = loc;
+   p->size = allocated + overhead;
+   if (!ptr_hash)
+     ptr_hash = htab_create (10, hash_ptr, eq_ptr, NULL);
+   slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr), INSERT);
+   if (*slot)
+     abort ();
+   *slot = p;
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
+ /* Helper function for prune_overhead_list.  See if SLOT is still marked and
+    remove it from hashtable if it is not.  */
+ static int
+ ggc_prune_ptr (void **slot, void *b ATTRIBUTE_UNUSED)
+ {
+   struct ptr_hash_entry *p = *slot;
+   if (!ggc_marked_p (p->ptr))
+     {
+       p->loc->collected += p->size;
+       htab_clear_slot (ptr_hash, slot);
+       free (p);
+     }
+   return 1;
+ }
+ 
+ /* After live values has been marked, walk all recorded pointers and see if
+    they are still live.  */
+ void
+ ggc_prune_overhead_list (void)
+ {
+   htab_traverse (ptr_hash, ggc_prune_ptr, NULL);
+ }
+ 
+ /* Notice that the pointer has been freed.  */
+ void ggc_free_overhead (void *ptr)
+ {
+   PTR *slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr),
+ 					NO_INSERT);
+   struct ptr_hash_entry *p = *slot;
+   p->loc->freed += p->size;
+   htab_clear_slot (ptr_hash, slot);
+   free (p);
+ }
+ 
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return ((l1->allocated + l1->overhead - l1->freed) -
! 	  (l2->allocated + l2->overhead - l1->freed));
  }
  
  /* Collect array of the descriptors from hashtable.  */
*************** void dump_ggc_loc_statistics (void)
*** 858,881 ****
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t count, size, overhead;
    int i;
  
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-60s %10s %10s %10s\n",
! 	   "source location", "Times", "Allocated", "Overhead");
    fprintf (stderr, "-------------------------------------------------------\n");
-   count = 0;
-   size = 0;
-   overhead = 0;
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       size += d->allocated;
!       count += d->times;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
--- 936,961 ----
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t collected = 0, freed = 0, allocated = 0, overhead = 0, times = 0;
    int i;
  
+   ggc_force_collect = true;
+   ggc_collect ();
+ 
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       allocated += d->allocated;
!       times += d->times;
!       freed += d->freed;
!       collected += d->collected;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
*************** void dump_ggc_loc_statistics (void)
*** 888,900 ****
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  fprintf (stderr, "%-60s %10i %10li %10li:%.3f%%\n", s,
! 		   d->times, (long)d->allocated, (long)d->overhead,
! 		   (d->allocated + d->overhead) *100.0 / (size + overhead));
  	}
      }
!   fprintf (stderr, "%-60s %10ld %10ld %10ld\n",
! 	   "Total", (long)count, (long)size, (long)overhead);
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
--- 968,993 ----
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  s[48] = 0;
! 	  fprintf (stderr, "%-48s %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li\n", s,
! 		   (long)d->collected,
! 		   (d->collected) * 100.0 / collected,
! 		   (long)d->freed,
! 		   (d->freed) * 100.0 / freed,
! 		   (long)(d->allocated + d->overhead - d->freed - d->collected),
! 		   (d->allocated + d->overhead - d->freed - d->collected) * 100.0
! 		   / (allocated + overhead - freed - collected),
! 		   (long)d->overhead,
! 		   d->overhead * 100.0 / overhead,
! 		   (long)d->times);
  	}
      }
!   fprintf (stderr, "%-48s %10ld       %10ld       %10ld       %10ld       %10ld\n",
! 	   "Total", (long)collected, (long)freed,
! 	   (long)(allocated + overhead - freed - collected), (long)overhead,
! 	   (long)times);
!   fprintf (stderr, "%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
Index: ggc-page.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-page.c,v
retrieving revision 1.90
diff -c -3 -p -r1.90 ggc-page.c
*** ggc-page.c	3 Mar 2004 11:25:47 -0000	1.90
--- ggc-page.c	5 Mar 2004 17:56:21 -0000
*************** ggc_alloc_stat (size_t size MEM_STAT_DEC
*** 1173,1184 ****
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
- #ifdef GATHER_STATISTICS
-   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size PASS_MEM_STAT);
- #endif
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
--- 1173,1185 ----
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
+ #ifdef GATHER_STATISTICS
+   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size,
+ 		       result PASS_MEM_STAT);
+ #endif
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
*************** ggc_free (void *p)
*** 1327,1332 ****
--- 1328,1337 ----
    size_t order = pe->order;
    size_t size = OBJECT_SIZE (order);
  
+ #ifdef GATHER_STATISTICS
+   ggc_free_overhead (p);
+ #endif
+ 
    if (GGC_DEBUG_LEVEL >= 3)
      fprintf (G.debug_file,
  	     "Freeing object, actual size=%lu, at %p on %p\n",
*************** ggc_collect (void)
*** 1971,1977 ****
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand)
      return;
  
    timevar_push (TV_GC);
--- 1976,1982 ----
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand && !ggc_force_collect)
      return;
  
    timevar_push (TV_GC);
*************** ggc_collect (void)
*** 1993,1998 ****
--- 1998,2006 ----
  
    clear_marks ();
    ggc_mark_roots ();
+ #ifdef GATHER_STATISTICS
+   ggc_prune_overhead_list ();
+ #endif
    poison_pages ();
    validate_free_objects ();
    sweep_pages ();
Index: ggc.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc.h,v
retrieving revision 1.66
diff -c -3 -p -r1.66 ggc.h
*** ggc.h	3 Mar 2004 11:25:48 -0000	1.66
--- ggc.h	5 Mar 2004 17:56:21 -0000
*************** extern struct alloc_zone *garbage_zone;
*** 209,214 ****
--- 209,216 ----
  extern struct alloc_zone *rtl_zone;
  /* For regular tree allocations.  */
  extern struct alloc_zone *tree_zone;
+ /* When set, ggc_collect will do collection.  */
+ extern bool ggc_force_collect;
  
  /* The internal primitive.  */
  extern void *ggc_alloc_stat (size_t MEM_STAT_DECL);
*************** extern void *ggc_calloc (size_t, size_t)
*** 233,239 ****
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t MEM_STAT_DECL);
  
  extern void dump_ggc_loc_statistics (void);
  
--- 235,243 ----
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t, void * MEM_STAT_DECL);
! extern void ggc_free_overhead (void *);
! extern void ggc_prune_overhead_list (void);
  
  extern void dump_ggc_loc_statistics (void);
  

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Make per-call statistics to take into account ggc_free
  2004-03-05 18:25 Make per-call statistics to take into account ggc_free Jan Hubicka
@ 2004-03-19  8:14 ` Jan Hubicka
  2004-09-02 16:12 ` Better memory statistics, take 2 Jan Hubicka
  1 sibling, 0 replies; 16+ messages in thread
From: Jan Hubicka @ 2004-03-19  8:14 UTC (permalink / raw)
  To: gcc-patches, zack, rth

Hi,
this patch improves the per-line statistics by tracking down each allocated
entity to figure out whether it will be freed, garbage collected or leaked.
To rule out ggc_freed values is pretty important as these are much cheaper,
similarly I used leaks to figure out why C++ frontend use that much of memory.

The tracking is implemented via ptr_hash that converts pointers to records and
by forcing one garbage collection run before outputting the data so leaks are
accurate.
For combine.c we now get:

emit-rtl.c:738 (gen_reg_rtx)                         203904: 0.9%     118944:46.1%          0: 0.0%      89376: 7.8%         54
c-decl.c:4338 (grokdeclarator)                        29160: 0.1%          0: 0.0%     316656:10.8%          0: 0.0%       3202
ggc-common.c:192 (ggc_calloc)                        332480: 1.5%          0: 0.0%      51536: 1.7%       4240: 0.4%        575
optabs.c:4913 (new_convert_optab)                         0: 0.0%          0: 0.0%     416916:14.2%     122004:10.7%          9
tree.c:4010 (build_function_type)                    348672: 1.5%          0: 0.0%     135808: 4.6%      75700: 6.6%       3785
convert.c:371 (convert_to_integer)                   517980: 2.3%          0: 0.0%       1480: 0.1%          0: 0.0%      25973
c-parse.y:748 (yyparse)                              521360: 2.3%          0: 0.0%          0: 0.0%          0: 0.0%      26068
c-parse.y:447 (yyparse)                              533540: 2.3%          0: 0.0%        480: 0.0%          0: 0.0%      26701
tree.c:406 (build_int_2_wide)                        529060: 2.3%          0: 0.0%      53020: 1.8%          0: 0.0%      29104
emit-rtl.c:3367 (make_jump_insn_raw)                 643524: 2.8%          0: 0.0%          0: 0.0%     153220:13.4%       7661
genrtl.c:635 (gen_rtx_fmt_i00)                       670160: 2.9%          0: 0.0%       2416: 0.1%          0: 0.0%      42036
genrtl.c:671 (gen_rtx_fmt_e0)                        688752: 3.0%          0: 0.0%      15036: 0.5%          0: 0.0%      58649
stringpool.c:70 (alloc_node)                              0: 0.0%          0: 0.0%     716520:24.3%     170600:15.0%       8530
c-decl.c:4270 (grokdeclarator)                       785160: 3.4%          0: 0.0%      18576: 0.6%          0: 0.0%       7442
varray.c:128 (varray_init)                           793716: 3.5%      15340: 6.0%      10948: 0.4%     211316:18.5%        591
cselib.c:840 (cselib_subst_to_values)                992512: 4.3%          0: 0.0%          0: 0.0%          0: 0.0%      86828
emit-rtl.c:3335 (make_insn_raw)                     1501800: 6.6%          0: 0.0%         40: 0.0%          0: 0.0%      37546
genrtl.c:51 (gen_rtx_fmt_ue)                        1574316: 6.9%          0: 0.0%          0: 0.0%          0: 0.0%     131193
rtl.c:250 (copy_rtx)                                1723556: 7.5%          0: 0.0%        576: 0.0%          0: 0.0%     141955
genrtl.c:33 (gen_rtx_fmt_ee)                        3201600:14.0%          0: 0.0%         36: 0.0%          0: 0.0%     266803
Total                                              22904228           257740          2945464          1139300          1281616
source location                                     Garbage            Freed             Leak         Overhead            Times

So only 257KB is explicitely freed right now.  I will try to add few extra calls.

Honza

Bootstrapped/regtested i686-pc-gnu-linux, OK?
2004-03-05  Jan Hubicka  <jh@suse.cz>
	* ggc-common.c (ggc_force_collect): New global variable.
	(loc_descroptor): New fields freed, collected.
	(ptr_hash): New global variable.
	(ptr_hash_entry): New structure
	(hash_ptr, eq_ptr): New functions.
	(ggc_record_overhead): New argument ptr; record entry to ptr hash.
	(ggc_prune_ptr): New static function.
	(ggc_prune_overhead, ggc_free_overhead): New functions.
	(cmp_statistics): Compare non-freed memory.
	(dump_ggc_loc_statistics): Force collection; print new values.
	* ggc-page.c (ggc_alloc_stat): Update call of
	ggc_recored_overhead.
	(ggc_free): Use ggc_free_overhead.
	(ggc_collect): Obey ggc_force_collect and use
	ggc_prune_overhead_list.
	* ggc.h (ggc_force_collect): Declare.
	(ggc_record_overhead): Update prototype.
	(ggc_free_overhead, ggc_prune_overhead_list): Declare.
Index: ggc-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-common.c,v
retrieving revision 1.83
diff -c -3 -p -r1.83 ggc-common.c
*** ggc-common.c	3 Mar 2004 11:25:47 -0000	1.83
--- ggc-common.c	5 Mar 2004 17:56:21 -0000
*************** Software Foundation, 59 Temple Place - S
*** 60,65 ****
--- 60,68 ----
  #define VALGRIND_DISCARD(x)
  #endif
  
+ /* When set, ggc_collect will do collection.  */
+ bool ggc_force_collect;
+ 
  /* Statistics about the allocation.  */
  static ggc_statistics *ggc_stats;
  
*************** struct loc_descriptor
*** 773,778 ****
--- 776,783 ----
    int times;
    size_t allocated;
    size_t overhead;
+   size_t freed;
+   size_t collected;
  };
  
  /* Hashtable used for statistics.  */
*************** eq_descriptor (const void *p1, const voi
*** 797,802 ****
--- 802,833 ----
  	  && d->function == d2->function);
  }
  
+ /* Hashtable converting address of allocated field to loc descriptor.  */
+ static htab_t ptr_hash;
+ struct ptr_hash_entry
+ {
+   void *ptr;
+   struct loc_descriptor *loc;
+   size_t size;
+ };
+ 
+ /* Hash table helpers functions.  */
+ static hashval_t
+ hash_ptr (const void *p)
+ {
+   const struct ptr_hash_entry *d = p;
+ 
+   return htab_hash_pointer (d->ptr);
+ }
+ 
+ static int
+ eq_ptr (const void *p1, const void *p2)
+ {
+   const struct ptr_hash_entry *p = p1;
+ 
+   return (p->ptr == p2);
+ }
+ 
  /* Return descriptor for given call site, create new one if needed.  */
  static struct loc_descriptor *
  loc_descriptor (const char *name, int line, const char *function)
*************** loc_descriptor (const char *name, int li
*** 821,843 ****
  }
  
  /* Record ALLOCATED and OVERHEAD bytes to descritor NAME:LINE (FUNCTION).  */
! void ggc_record_overhead (size_t allocated, size_t overhead,
  			  const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return (l1->allocated + l1->overhead) - (l2->allocated + l2->overhead);
  }
  
  /* Collect array of the descriptors from hashtable.  */
--- 852,921 ----
  }
  
  /* Record ALLOCATED and OVERHEAD bytes to descritor NAME:LINE (FUNCTION).  */
! void ggc_record_overhead (size_t allocated, size_t overhead, void *ptr,
  			  const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
+   struct ptr_hash_entry *p = xmalloc (sizeof (struct ptr_hash_entry));
+   PTR *slot;
+ 
+   p->ptr = ptr;
+   p->loc = loc;
+   p->size = allocated + overhead;
+   if (!ptr_hash)
+     ptr_hash = htab_create (10, hash_ptr, eq_ptr, NULL);
+   slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr), INSERT);
+   if (*slot)
+     abort ();
+   *slot = p;
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
+ /* Helper function for prune_overhead_list.  See if SLOT is still marked and
+    remove it from hashtable if it is not.  */
+ static int
+ ggc_prune_ptr (void **slot, void *b ATTRIBUTE_UNUSED)
+ {
+   struct ptr_hash_entry *p = *slot;
+   if (!ggc_marked_p (p->ptr))
+     {
+       p->loc->collected += p->size;
+       htab_clear_slot (ptr_hash, slot);
+       free (p);
+     }
+   return 1;
+ }
+ 
+ /* After live values has been marked, walk all recorded pointers and see if
+    they are still live.  */
+ void
+ ggc_prune_overhead_list (void)
+ {
+   htab_traverse (ptr_hash, ggc_prune_ptr, NULL);
+ }
+ 
+ /* Notice that the pointer has been freed.  */
+ void ggc_free_overhead (void *ptr)
+ {
+   PTR *slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr),
+ 					NO_INSERT);
+   struct ptr_hash_entry *p = *slot;
+   p->loc->freed += p->size;
+   htab_clear_slot (ptr_hash, slot);
+   free (p);
+ }
+ 
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return ((l1->allocated + l1->overhead - l1->freed) -
! 	  (l2->allocated + l2->overhead - l1->freed));
  }
  
  /* Collect array of the descriptors from hashtable.  */
*************** void dump_ggc_loc_statistics (void)
*** 858,881 ****
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t count, size, overhead;
    int i;
  
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-60s %10s %10s %10s\n",
! 	   "source location", "Times", "Allocated", "Overhead");
    fprintf (stderr, "-------------------------------------------------------\n");
-   count = 0;
-   size = 0;
-   overhead = 0;
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       size += d->allocated;
!       count += d->times;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
--- 936,961 ----
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t collected = 0, freed = 0, allocated = 0, overhead = 0, times = 0;
    int i;
  
+   ggc_force_collect = true;
+   ggc_collect ();
+ 
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       allocated += d->allocated;
!       times += d->times;
!       freed += d->freed;
!       collected += d->collected;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
*************** void dump_ggc_loc_statistics (void)
*** 888,900 ****
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  fprintf (stderr, "%-60s %10i %10li %10li:%.3f%%\n", s,
! 		   d->times, (long)d->allocated, (long)d->overhead,
! 		   (d->allocated + d->overhead) *100.0 / (size + overhead));
  	}
      }
!   fprintf (stderr, "%-60s %10ld %10ld %10ld\n",
! 	   "Total", (long)count, (long)size, (long)overhead);
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
--- 968,993 ----
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  s[48] = 0;
! 	  fprintf (stderr, "%-48s %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li\n", s,
! 		   (long)d->collected,
! 		   (d->collected) * 100.0 / collected,
! 		   (long)d->freed,
! 		   (d->freed) * 100.0 / freed,
! 		   (long)(d->allocated + d->overhead - d->freed - d->collected),
! 		   (d->allocated + d->overhead - d->freed - d->collected) * 100.0
! 		   / (allocated + overhead - freed - collected),
! 		   (long)d->overhead,
! 		   d->overhead * 100.0 / overhead,
! 		   (long)d->times);
  	}
      }
!   fprintf (stderr, "%-48s %10ld       %10ld       %10ld       %10ld       %10ld\n",
! 	   "Total", (long)collected, (long)freed,
! 	   (long)(allocated + overhead - freed - collected), (long)overhead,
! 	   (long)times);
!   fprintf (stderr, "%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
Index: ggc-page.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-page.c,v
retrieving revision 1.90
diff -c -3 -p -r1.90 ggc-page.c
*** ggc-page.c	3 Mar 2004 11:25:47 -0000	1.90
--- ggc-page.c	5 Mar 2004 17:56:21 -0000
*************** ggc_alloc_stat (size_t size MEM_STAT_DEC
*** 1173,1184 ****
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
- #ifdef GATHER_STATISTICS
-   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size PASS_MEM_STAT);
- #endif
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
--- 1173,1185 ----
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
+ #ifdef GATHER_STATISTICS
+   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size,
+ 		       result PASS_MEM_STAT);
+ #endif
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
*************** ggc_free (void *p)
*** 1327,1332 ****
--- 1328,1337 ----
    size_t order = pe->order;
    size_t size = OBJECT_SIZE (order);
  
+ #ifdef GATHER_STATISTICS
+   ggc_free_overhead (p);
+ #endif
+ 
    if (GGC_DEBUG_LEVEL >= 3)
      fprintf (G.debug_file,
  	     "Freeing object, actual size=%lu, at %p on %p\n",
*************** ggc_collect (void)
*** 1971,1977 ****
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand)
      return;
  
    timevar_push (TV_GC);
--- 1976,1982 ----
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand && !ggc_force_collect)
      return;
  
    timevar_push (TV_GC);
*************** ggc_collect (void)
*** 1993,1998 ****
--- 1998,2006 ----
  
    clear_marks ();
    ggc_mark_roots ();
+ #ifdef GATHER_STATISTICS
+   ggc_prune_overhead_list ();
+ #endif
    poison_pages ();
    validate_free_objects ();
    sweep_pages ();
Index: ggc.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc.h,v
retrieving revision 1.66
diff -c -3 -p -r1.66 ggc.h
*** ggc.h	3 Mar 2004 11:25:48 -0000	1.66
--- ggc.h	5 Mar 2004 17:56:21 -0000
*************** extern struct alloc_zone *garbage_zone;
*** 209,214 ****
--- 209,216 ----
  extern struct alloc_zone *rtl_zone;
  /* For regular tree allocations.  */
  extern struct alloc_zone *tree_zone;
+ /* When set, ggc_collect will do collection.  */
+ extern bool ggc_force_collect;
  
  /* The internal primitive.  */
  extern void *ggc_alloc_stat (size_t MEM_STAT_DECL);
*************** extern void *ggc_calloc (size_t, size_t)
*** 233,239 ****
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t MEM_STAT_DECL);
  
  extern void dump_ggc_loc_statistics (void);
  
--- 235,243 ----
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t, void * MEM_STAT_DECL);
! extern void ggc_free_overhead (void *);
! extern void ggc_prune_overhead_list (void);
  
  extern void dump_ggc_loc_statistics (void);
  

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Better memory statistics, take 2
@ 2004-09-02 16:12 ` Jan Hubicka
  2004-09-02 16:27   ` Zack Weinberg
  2004-09-02 16:54   ` Jan Hubicka
  0 siblings, 2 replies; 16+ messages in thread
From: Jan Hubicka @ 2004-09-02 16:12 UTC (permalink / raw)
  To: gcc-patches, stevenb, rth

Hi,
here is updated version of patch I sent while reducing memory for GCC
3.4, it is quite usefull now again...

Hi,
this patch improves the per-line statistics by tracking down each allocated
entity to figure out whether it will be freed, garbage collected or leaked.
To rule out ggc_freed values is pretty important as these are much cheaper,
similarly I used leaks to figure out why C++ frontend use that much of memory.

The tracking is implemented via ptr_hash that converts pointers to records and
by forcing one garbage collection run before outputting the data so leaks are
accurate.
For combine.c we now get:

emit-rtl.c:738 (gen_reg_rtx)                         203904: 0.9%     118944:46.1%          0: 0.0%      89376: 7.8%         54
c-decl.c:4338 (grokdeclarator)                        29160: 0.1%          0: 0.0%     316656:10.8%          0: 0.0%       3202
ggc-common.c:192 (ggc_calloc)                        332480: 1.5%          0: 0.0%      51536: 1.7%       4240: 0.4%        575
optabs.c:4913 (new_convert_optab)                         0: 0.0%          0: 0.0%     416916:14.2%     122004:10.7%          9
tree.c:4010 (build_function_type)                    348672: 1.5%          0: 0.0%     135808: 4.6%      75700: 6.6%       3785
convert.c:371 (convert_to_integer)                   517980: 2.3%          0: 0.0%       1480: 0.1%          0: 0.0%      25973
c-parse.y:748 (yyparse)                              521360: 2.3%          0: 0.0%          0: 0.0%          0: 0.0%      26068
c-parse.y:447 (yyparse)                              533540: 2.3%          0: 0.0%        480: 0.0%          0: 0.0%      26701
tree.c:406 (build_int_2_wide)                        529060: 2.3%          0: 0.0%      53020: 1.8%          0: 0.0%      29104
emit-rtl.c:3367 (make_jump_insn_raw)                 643524: 2.8%          0: 0.0%          0: 0.0%     153220:13.4%       7661
genrtl.c:635 (gen_rtx_fmt_i00)                       670160: 2.9%          0: 0.0%       2416: 0.1%          0: 0.0%      42036
genrtl.c:671 (gen_rtx_fmt_e0)                        688752: 3.0%          0: 0.0%      15036: 0.5%          0: 0.0%      58649
stringpool.c:70 (alloc_node)                              0: 0.0%          0: 0.0%     716520:24.3%     170600:15.0%       8530
c-decl.c:4270 (grokdeclarator)                       785160: 3.4%          0: 0.0%      18576: 0.6%          0: 0.0%       7442
varray.c:128 (varray_init)                           793716: 3.5%      15340: 6.0%      10948: 0.4%     211316:18.5%        591
cselib.c:840 (cselib_subst_to_values)                992512: 4.3%          0: 0.0%          0: 0.0%          0: 0.0%      86828
emit-rtl.c:3335 (make_insn_raw)                     1501800: 6.6%          0: 0.0%         40: 0.0%          0: 0.0%      37546
genrtl.c:51 (gen_rtx_fmt_ue)                        1574316: 6.9%          0: 0.0%          0: 0.0%          0: 0.0%     131193
rtl.c:250 (copy_rtx)                                1723556: 7.5%          0: 0.0%        576: 0.0%          0: 0.0%     141955
genrtl.c:33 (gen_rtx_fmt_ee)                        3201600:14.0%          0: 0.0%         36: 0.0%          0: 0.0%     266803
Total                                              22904228           257740          2945464          1139300          1281616
source location                                     Garbage            Freed             Leak         Overhead            Times

So only 257KB is explicitely freed right now.  I will try to add few extra calls.

Honza
Index: ggc-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-common.c,v
retrieving revision 1.88
diff -c -3 -p -r1.88 ggc-common.c
*** ggc-common.c	9 Aug 2004 20:19:29 -0000	1.88
--- ggc-common.c	2 Sep 2004 16:08:50 -0000
*************** Software Foundation, 59 Temple Place - S
*** 61,66 ****
--- 61,69 ----
  #define VALGRIND_DISCARD(x)
  #endif
  
+ /* When set, ggc_collect will do collection.  */
+ bool ggc_force_collect;
+ 
  /* Statistics about the allocation.  */
  static ggc_statistics *ggc_stats;
  
*************** struct loc_descriptor
*** 780,785 ****
--- 783,790 ----
    int times;
    size_t allocated;
    size_t overhead;
+   size_t freed;
+   size_t collected;
  };
  
  /* Hashtable used for statistics.  */
*************** eq_descriptor (const void *p1, const voi
*** 804,809 ****
--- 809,840 ----
  	  && d->function == d2->function);
  }
  
+ /* Hashtable converting address of allocated field to loc descriptor.  */
+ static htab_t ptr_hash;
+ struct ptr_hash_entry
+ {
+   void *ptr;
+   struct loc_descriptor *loc;
+   size_t size;
+ };
+ 
+ /* Hash table helpers functions.  */
+ static hashval_t
+ hash_ptr (const void *p)
+ {
+   const struct ptr_hash_entry *d = p;
+ 
+   return htab_hash_pointer (d->ptr);
+ }
+ 
+ static int
+ eq_ptr (const void *p1, const void *p2)
+ {
+   const struct ptr_hash_entry *p = p1;
+ 
+   return (p->ptr == p2);
+ }
+ 
  /* Return descriptor for given call site, create new one if needed.  */
  static struct loc_descriptor *
  loc_descriptor (const char *name, int line, const char *function)
*************** loc_descriptor (const char *name, int li
*** 829,851 ****
  
  /* Record ALLOCATED and OVERHEAD bytes to descriptor NAME:LINE (FUNCTION).  */
  void
! ggc_record_overhead (size_t allocated, size_t overhead,
  		     const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return (l1->allocated + l1->overhead) - (l2->allocated + l2->overhead);
  }
  
  /* Collect array of the descriptors from hashtable.  */
--- 860,929 ----
  
  /* Record ALLOCATED and OVERHEAD bytes to descriptor NAME:LINE (FUNCTION).  */
  void
! ggc_record_overhead (size_t allocated, size_t overhead, void *ptr,
  		     const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
+   struct ptr_hash_entry *p = xmalloc (sizeof (struct ptr_hash_entry));
+   PTR *slot;
+ 
+   p->ptr = ptr;
+   p->loc = loc;
+   p->size = allocated + overhead;
+   if (!ptr_hash)
+     ptr_hash = htab_create (10, hash_ptr, eq_ptr, NULL);
+   slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr), INSERT);
+   if (*slot)
+     abort ();
+   *slot = p;
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
+ /* Helper function for prune_overhead_list.  See if SLOT is still marked and
+    remove it from hashtable if it is not.  */
+ static int
+ ggc_prune_ptr (void **slot, void *b ATTRIBUTE_UNUSED)
+ {
+   struct ptr_hash_entry *p = *slot;
+   if (!ggc_marked_p (p->ptr))
+     {
+       p->loc->collected += p->size;
+       htab_clear_slot (ptr_hash, slot);
+       free (p);
+     }
+   return 1;
+ }
+ 
+ /* After live values has been marked, walk all recorded pointers and see if
+    they are still live.  */
+ void
+ ggc_prune_overhead_list (void)
+ {
+   htab_traverse (ptr_hash, ggc_prune_ptr, NULL);
+ }
+ 
+ /* Notice that the pointer has been freed.  */
+ void ggc_free_overhead (void *ptr)
+ {
+   PTR *slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr),
+ 					NO_INSERT);
+   struct ptr_hash_entry *p = *slot;
+   p->loc->freed += p->size;
+   htab_clear_slot (ptr_hash, slot);
+   free (p);
+ }
+ 
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return ((l1->allocated + l1->overhead - l1->freed) -
! 	  (l2->allocated + l2->overhead - l1->freed));
  }
  
  /* Collect array of the descriptors from hashtable.  */
*************** void dump_ggc_loc_statistics (void)
*** 866,889 ****
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t count, size, overhead;
    int i;
  
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-60s %10s %10s %10s\n",
! 	   "source location", "Times", "Allocated", "Overhead");
    fprintf (stderr, "-------------------------------------------------------\n");
-   count = 0;
-   size = 0;
-   overhead = 0;
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       size += d->allocated;
!       count += d->times;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
--- 944,969 ----
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t collected = 0, freed = 0, allocated = 0, overhead = 0, times = 0;
    int i;
  
+   ggc_force_collect = true;
+   ggc_collect ();
+ 
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       allocated += d->allocated;
!       times += d->times;
!       freed += d->freed;
!       collected += d->collected;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
*************** void dump_ggc_loc_statistics (void)
*** 896,908 ****
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  fprintf (stderr, "%-60s %10i %10li %10li:%.3f%%\n", s,
! 		   d->times, (long)d->allocated, (long)d->overhead,
! 		   (d->allocated + d->overhead) *100.0 / (size + overhead));
  	}
      }
!   fprintf (stderr, "%-60s %10ld %10ld %10ld\n",
! 	   "Total", (long)count, (long)size, (long)overhead);
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
--- 976,1001 ----
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  s[48] = 0;
! 	  fprintf (stderr, "%-48s %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li\n", s,
! 		   (long)d->collected,
! 		   (d->collected) * 100.0 / collected,
! 		   (long)d->freed,
! 		   (d->freed) * 100.0 / freed,
! 		   (long)(d->allocated + d->overhead - d->freed - d->collected),
! 		   (d->allocated + d->overhead - d->freed - d->collected) * 100.0
! 		   / (allocated + overhead - freed - collected),
! 		   (long)d->overhead,
! 		   d->overhead * 100.0 / overhead,
! 		   (long)d->times);
  	}
      }
!   fprintf (stderr, "%-48s %10ld       %10ld       %10ld       %10ld       %10ld\n",
! 	   "Total", (long)collected, (long)freed,
! 	   (long)(allocated + overhead - freed - collected), (long)overhead,
! 	   (long)times);
!   fprintf (stderr, "%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
Index: ggc-page.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-page.c,v
retrieving revision 1.92
diff -c -3 -p -r1.92 ggc-page.c
*** ggc-page.c	22 Mar 2004 02:57:27 -0000	1.92
--- ggc-page.c	2 Sep 2004 16:08:50 -0000
*************** ggc_alloc_stat (size_t size MEM_STAT_DEC
*** 1173,1184 ****
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
- #ifdef GATHER_STATISTICS
-   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size PASS_MEM_STAT);
- #endif
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
--- 1173,1185 ----
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
+ #ifdef GATHER_STATISTICS
+   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size,
+ 		       result PASS_MEM_STAT);
+ #endif
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
*************** ggc_free (void *p)
*** 1327,1332 ****
--- 1328,1337 ----
    size_t order = pe->order;
    size_t size = OBJECT_SIZE (order);
  
+ #ifdef GATHER_STATISTICS
+   ggc_free_overhead (p);
+ #endif
+ 
    if (GGC_DEBUG_LEVEL >= 3)
      fprintf (G.debug_file,
  	     "Freeing object, actual size=%lu, at %p on %p\n",
*************** ggc_collect (void)
*** 1971,1977 ****
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand)
      return;
  
    timevar_push (TV_GC);
--- 1976,1982 ----
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand && !ggc_force_collect)
      return;
  
    timevar_push (TV_GC);
*************** ggc_collect (void)
*** 1993,1998 ****
--- 1998,2006 ----
  
    clear_marks ();
    ggc_mark_roots ();
+ #ifdef GATHER_STATISTICS
+   ggc_prune_overhead_list ();
+ #endif
    poison_pages ();
    validate_free_objects ();
    sweep_pages ();
Index: ggc.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc.h,v
retrieving revision 1.68
diff -c -3 -p -r1.68 ggc.h
*** ggc.h	2 Sep 2004 02:39:15 -0000	1.68
--- ggc.h	2 Sep 2004 16:08:50 -0000
*************** extern struct alloc_zone *garbage_zone;
*** 209,214 ****
--- 209,216 ----
  extern struct alloc_zone *rtl_zone;
  /* For regular tree allocations.  */
  extern struct alloc_zone *tree_zone;
+ /* When set, ggc_collect will do collection.  */
+ extern bool ggc_force_collect;
  
  /* The internal primitive.  */
  extern void *ggc_alloc_stat (size_t MEM_STAT_DECL);
*************** extern void *ggc_calloc (size_t, size_t)
*** 233,239 ****
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t MEM_STAT_DECL);
  
  extern void dump_ggc_loc_statistics (void);
  
--- 235,243 ----
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t, void * MEM_STAT_DECL);
! extern void ggc_free_overhead (void *);
! extern void ggc_prune_overhead_list (void);
  
  extern void dump_ggc_loc_statistics (void);
  
Index: passes.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/passes.c,v
retrieving revision 2.41
diff -c -3 -p -r2.41 passes.c
*** passes.c	1 Sep 2004 20:58:52 -0000	2.41
--- passes.c	2 Sep 2004 16:08:50 -0000
*************** rest_of_clean_state (void)
*** 1685,1690 ****
--- 1685,1691 ----
  
    /* We're done with this function.  Free up memory if we can.  */
    free_after_parsing (cfun);
+   free_after_compilation (cfun);
  }
  \f
  
Index: rtl.def
===================================================================
RCS file: /cvs/gcc/gcc/gcc/rtl.def,v
retrieving revision 1.93
diff -c -3 -p -r1.93 rtl.def
*** rtl.def	24 Aug 2004 17:00:54 -0000	1.93
--- rtl.def	2 Sep 2004 16:08:50 -0000
*************** DEF_RTL_EXPR(HIGH, "high", "e", RTX_CONS
*** 599,643 ****
  /* LO_SUM is the sum of a register and the low-order bits
     of a constant expression.  */
  DEF_RTL_EXPR(LO_SUM, "lo_sum", "ee", RTX_OBJ)
- 
- /* Header for range information.  Operand 0 is the NOTE_INSN_RANGE_BEG insn.
-    Operand 1 is the NOTE_INSN_RANGE_END insn.  Operand 2 is a vector of all of
-    the registers that can be substituted within this range.  Operand 3 is the
-    number of calls in the range.  Operand 4 is the number of insns in the
-    range.  Operand 5 is the unique range number for this range.  Operand 6 is
-    the basic block # of the start of the live range.  Operand 7 is the basic
-    block # of the end of the live range.  Operand 8 is the loop depth.  Operand
-    9 is a bitmap of the registers live at the start of the range.  Operand 10
-    is a bitmap of the registers live at the end of the range.  Operand 11 is
-    marker number for the start of the range.  Operand 12 is the marker number
-    for the end of the range.  */
- DEF_RTL_EXPR(RANGE_INFO, "range_info", "uuEiiiiiibbii", RTX_EXTRA)
- 
- /* Registers that can be substituted within the range.  Operand 0 is the
-    original pseudo register number.  Operand 1 will be filled in with the
-    pseudo register the value is copied for the duration of the range.  Operand
-    2 is the number of references within the range to the register.  Operand 3
-    is the number of sets or clobbers of the register in the range.  Operand 4
-    is the number of deaths the register has.  Operand 5 is the copy flags that
-    give the status of whether a copy is needed from the original register to
-    the new register at the beginning of the range, or whether a copy from the
-    new register back to the original at the end of the range.  Operand 6 is the
-    live length.  Operand 7 is the number of calls that this register is live
-    across.  Operand 8 is the symbol node of the variable if the register is a
-    user variable.  Operand 9 is the block node that the variable is declared
-    in if the register is a user variable.  */
- DEF_RTL_EXPR(RANGE_REG, "range_reg", "iiiiiiiitt", RTX_EXTRA)
- 
- /* Information about a local variable's ranges.  Operand 0 is an EXPR_LIST of
-    the different ranges a variable is in where it is copied to a different
-    pseudo register.  Operand 1 is the block that the variable is declared in.
-    Operand 2 is the number of distinct ranges.  */
- DEF_RTL_EXPR(RANGE_VAR, "range_var", "eti", RTX_EXTRA)
- 
- /* Information about the registers that are live at the current point.  Operand
-    0 is the live bitmap.  Operand 1 is the original block number.  */
- DEF_RTL_EXPR(RANGE_LIVE, "range_live", "bi", RTX_EXTRA)
- 
  /* Describes a merge operation between two vector values.
     Operands 0 and 1 are the vectors to be merged, operand 2 is a bitmask
     that specifies where the parts of the result are taken from.  Set bits
--- 599,604 ----

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:12 ` Better memory statistics, take 2 Jan Hubicka
@ 2004-09-02 16:27   ` Zack Weinberg
  2004-09-02 16:45     ` Matt Austern
  2004-09-02 17:28     ` Daniel Jacobowitz
  2004-09-02 16:54   ` Jan Hubicka
  1 sibling, 2 replies; 16+ messages in thread
From: Zack Weinberg @ 2004-09-02 16:27 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, stevenb, rth

Jan Hubicka <hubicka@ucw.cz> writes:

> Hi,
> here is updated version of patch I sent while reducing memory for GCC
> 3.4, it is quite usefull now again...
>
> Hi, this patch improves the per-line statistics by tracking down
> each allocated entity to figure out whether it will be freed,
> garbage collected or leaked.  To rule out ggc_freed values is pretty
> important as these are much cheaper,
...

Please do timing tests before submitting any changes.  In my
experience ggc_free is *not* cheaper, it is by itself such an
expensive operation that we don't actually gain anything over
letting the garbage collector do its job.

zw

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:27   ` Zack Weinberg
@ 2004-09-02 16:45     ` Matt Austern
  2004-09-02 16:47       ` Jan Hubicka
  2004-09-02 17:28     ` Daniel Jacobowitz
  1 sibling, 1 reply; 16+ messages in thread
From: Matt Austern @ 2004-09-02 16:45 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Jan Hubicka, gcc-patches, stevenb, rth

On Sep 2, 2004, at 9:23 AM, Zack Weinberg wrote:

> Jan Hubicka <hubicka@ucw.cz> writes:
>
>> Hi,
>> here is updated version of patch I sent while reducing memory for GCC
>> 3.4, it is quite usefull now again...
>>
>> Hi, this patch improves the per-line statistics by tracking down
>> each allocated entity to figure out whether it will be freed,
>> garbage collected or leaked.  To rule out ggc_freed values is pretty
>> important as these are much cheaper,
> ...
>
> Please do timing tests before submitting any changes.  In my
> experience ggc_free is *not* cheaper, it is by itself such an
> expensive operation that we don't actually gain anything over
> letting the garbage collector do its job.

What this does mean is that if we know a data structure will be 
temporary, we shouldn't use ggc_free or ggc_anything.

			--Matt

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:45     ` Matt Austern
@ 2004-09-02 16:47       ` Jan Hubicka
  2004-09-02 17:15         ` Zack Weinberg
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Hubicka @ 2004-09-02 16:47 UTC (permalink / raw)
  To: Matt Austern; +Cc: Zack Weinberg, Jan Hubicka, gcc-patches, stevenb, rth

> On Sep 2, 2004, at 9:23 AM, Zack Weinberg wrote:
> 
> >Jan Hubicka <hubicka@ucw.cz> writes:
> >
> >>Hi,
> >>here is updated version of patch I sent while reducing memory for GCC
> >>3.4, it is quite usefull now again...
> >>
> >>Hi, this patch improves the per-line statistics by tracking down
> >>each allocated entity to figure out whether it will be freed,
> >>garbage collected or leaked.  To rule out ggc_freed values is pretty
> >>important as these are much cheaper,
> >...
> >
> >Please do timing tests before submitting any changes.  In my
> >experience ggc_free is *not* cheaper, it is by itself such an
> >expensive operation that we don't actually gain anything over
> >letting the garbage collector do its job.

My experience is that you need to free quite a lot of memory to see gain
(ie you need to avoid at leat one GGC run).  For Gerald's testcase I
actually measured slight change in execution time with both changes (ie
one GGC run, but it is because my memory setup is definitly on the
corner between 22 and 23 GGC runs)

This patch is not adding any ggc_free, just making analysis prettier.  I
have some extra patches dependent on the framework (ie I use it to
detect leaks where we keep pointer to something local accidentally and
it was usefull enought to reduce peak memory usage of combine.c
compilation from 12MB to 2.8MB.)
> 
> What this does mean is that if we know a data structure will be 
> temporary, we shouldn't use ggc_free or ggc_anything.

Yes, where convenient, I am trying to get out of GGC completely.

Honza
> 
> 			--Matt

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:12 ` Better memory statistics, take 2 Jan Hubicka
  2004-09-02 16:27   ` Zack Weinberg
@ 2004-09-02 16:54   ` Jan Hubicka
  2004-09-02 17:18     ` Richard Henderson
  2021-08-17  9:17     ` Thomas Schwinge
  1 sibling, 2 replies; 16+ messages in thread
From: Jan Hubicka @ 2004-09-02 16:54 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, stevenb, rth

> Hi,
> here is updated version of patch I sent while reducing memory for GCC
> 3.4, it is quite usefull now again...
> 
> Hi,
> this patch improves the per-line statistics by tracking down each allocated
> entity to figure out whether it will be freed, garbage collected or leaked.
> To rule out ggc_freed values is pretty important as these are much cheaper,
> similarly I used leaks to figure out why C++ frontend use that much of memory.
> 
> The tracking is implemented via ptr_hash that converts pointers to records and
> by forcing one garbage collection run before outputting the data so leaks are
> accurate.
> For combine.c we now get:
> 
> emit-rtl.c:738 (gen_reg_rtx)                         203904: 0.9%     118944:46.1%          0: 0.0%      89376: 7.8%         54
> c-decl.c:4338 (grokdeclarator)                        29160: 0.1%          0: 0.0%     316656:10.8%          0: 0.0%       3202
> ggc-common.c:192 (ggc_calloc)                        332480: 1.5%          0: 0.0%      51536: 1.7%       4240: 0.4%        575
> optabs.c:4913 (new_convert_optab)                         0: 0.0%          0: 0.0%     416916:14.2%     122004:10.7%          9
> tree.c:4010 (build_function_type)                    348672: 1.5%          0: 0.0%     135808: 4.6%      75700: 6.6%       3785
> convert.c:371 (convert_to_integer)                   517980: 2.3%          0: 0.0%       1480: 0.1%          0: 0.0%      25973
> c-parse.y:748 (yyparse)                              521360: 2.3%          0: 0.0%          0: 0.0%          0: 0.0%      26068
> c-parse.y:447 (yyparse)                              533540: 2.3%          0: 0.0%        480: 0.0%          0: 0.0%      26701
> tree.c:406 (build_int_2_wide)                        529060: 2.3%          0: 0.0%      53020: 1.8%          0: 0.0%      29104
> emit-rtl.c:3367 (make_jump_insn_raw)                 643524: 2.8%          0: 0.0%          0: 0.0%     153220:13.4%       7661
> genrtl.c:635 (gen_rtx_fmt_i00)                       670160: 2.9%          0: 0.0%       2416: 0.1%          0: 0.0%      42036
> genrtl.c:671 (gen_rtx_fmt_e0)                        688752: 3.0%          0: 0.0%      15036: 0.5%          0: 0.0%      58649
> stringpool.c:70 (alloc_node)                              0: 0.0%          0: 0.0%     716520:24.3%     170600:15.0%       8530
> c-decl.c:4270 (grokdeclarator)                       785160: 3.4%          0: 0.0%      18576: 0.6%          0: 0.0%       7442
> varray.c:128 (varray_init)                           793716: 3.5%      15340: 6.0%      10948: 0.4%     211316:18.5%        591
> cselib.c:840 (cselib_subst_to_values)                992512: 4.3%          0: 0.0%          0: 0.0%          0: 0.0%      86828
> emit-rtl.c:3335 (make_insn_raw)                     1501800: 6.6%          0: 0.0%         40: 0.0%          0: 0.0%      37546
> genrtl.c:51 (gen_rtx_fmt_ue)                        1574316: 6.9%          0: 0.0%          0: 0.0%          0: 0.0%     131193
> rtl.c:250 (copy_rtx)                                1723556: 7.5%          0: 0.0%        576: 0.0%          0: 0.0%     141955
> genrtl.c:33 (gen_rtx_fmt_ee)                        3201600:14.0%          0: 0.0%         36: 0.0%          0: 0.0%     266803
> Total                                              22904228           257740          2945464          1139300          1281616
> source location                                     Garbage            Freed             Leak         Overhead            Times
> 
> So only 257KB is explicitely freed right now.  I will try to add few extra calls.

Sorry, I managed to melt two changes together, here is proper patch and
I am just re-testing it.

Honza
Index: ggc-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-common.c,v
retrieving revision 1.88
diff -c -3 -p -r1.88 ggc-common.c
*** ggc-common.c	9 Aug 2004 20:19:29 -0000	1.88
--- ggc-common.c	2 Sep 2004 16:08:50 -0000
*************** Software Foundation, 59 Temple Place - S
*** 61,66 ****
--- 61,69 ----
  #define VALGRIND_DISCARD(x)
  #endif
  
+ /* When set, ggc_collect will do collection.  */
+ bool ggc_force_collect;
+ 
  /* Statistics about the allocation.  */
  static ggc_statistics *ggc_stats;
  
*************** struct loc_descriptor
*** 780,785 ****
--- 783,790 ----
    int times;
    size_t allocated;
    size_t overhead;
+   size_t freed;
+   size_t collected;
  };
  
  /* Hashtable used for statistics.  */
*************** eq_descriptor (const void *p1, const voi
*** 804,809 ****
--- 809,840 ----
  	  && d->function == d2->function);
  }
  
+ /* Hashtable converting address of allocated field to loc descriptor.  */
+ static htab_t ptr_hash;
+ struct ptr_hash_entry
+ {
+   void *ptr;
+   struct loc_descriptor *loc;
+   size_t size;
+ };
+ 
+ /* Hash table helpers functions.  */
+ static hashval_t
+ hash_ptr (const void *p)
+ {
+   const struct ptr_hash_entry *d = p;
+ 
+   return htab_hash_pointer (d->ptr);
+ }
+ 
+ static int
+ eq_ptr (const void *p1, const void *p2)
+ {
+   const struct ptr_hash_entry *p = p1;
+ 
+   return (p->ptr == p2);
+ }
+ 
  /* Return descriptor for given call site, create new one if needed.  */
  static struct loc_descriptor *
  loc_descriptor (const char *name, int line, const char *function)
*************** loc_descriptor (const char *name, int li
*** 829,851 ****
  
  /* Record ALLOCATED and OVERHEAD bytes to descriptor NAME:LINE (FUNCTION).  */
  void
! ggc_record_overhead (size_t allocated, size_t overhead,
  		     const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return (l1->allocated + l1->overhead) - (l2->allocated + l2->overhead);
  }
  
  /* Collect array of the descriptors from hashtable.  */
--- 860,929 ----
  
  /* Record ALLOCATED and OVERHEAD bytes to descriptor NAME:LINE (FUNCTION).  */
  void
! ggc_record_overhead (size_t allocated, size_t overhead, void *ptr,
  		     const char *name, int line, const char *function)
  {
    struct loc_descriptor *loc = loc_descriptor (name, line, function);
+   struct ptr_hash_entry *p = xmalloc (sizeof (struct ptr_hash_entry));
+   PTR *slot;
+ 
+   p->ptr = ptr;
+   p->loc = loc;
+   p->size = allocated + overhead;
+   if (!ptr_hash)
+     ptr_hash = htab_create (10, hash_ptr, eq_ptr, NULL);
+   slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr), INSERT);
+   if (*slot)
+     abort ();
+   *slot = p;
  
    loc->times++;
    loc->allocated+=allocated;
    loc->overhead+=overhead;
  }
  
+ /* Helper function for prune_overhead_list.  See if SLOT is still marked and
+    remove it from hashtable if it is not.  */
+ static int
+ ggc_prune_ptr (void **slot, void *b ATTRIBUTE_UNUSED)
+ {
+   struct ptr_hash_entry *p = *slot;
+   if (!ggc_marked_p (p->ptr))
+     {
+       p->loc->collected += p->size;
+       htab_clear_slot (ptr_hash, slot);
+       free (p);
+     }
+   return 1;
+ }
+ 
+ /* After live values has been marked, walk all recorded pointers and see if
+    they are still live.  */
+ void
+ ggc_prune_overhead_list (void)
+ {
+   htab_traverse (ptr_hash, ggc_prune_ptr, NULL);
+ }
+ 
+ /* Notice that the pointer has been freed.  */
+ void ggc_free_overhead (void *ptr)
+ {
+   PTR *slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr),
+ 					NO_INSERT);
+   struct ptr_hash_entry *p = *slot;
+   p->loc->freed += p->size;
+   htab_clear_slot (ptr_hash, slot);
+   free (p);
+ }
+ 
  /* Helper for qsort; sort descriptors by amount of memory consumed.  */
  static int
  cmp_statistic (const void *loc1, const void *loc2)
  {
    struct loc_descriptor *l1 = *(struct loc_descriptor **) loc1;
    struct loc_descriptor *l2 = *(struct loc_descriptor **) loc2;
!   return ((l1->allocated + l1->overhead - l1->freed) -
! 	  (l2->allocated + l2->overhead - l1->freed));
  }
  
  /* Collect array of the descriptors from hashtable.  */
*************** void dump_ggc_loc_statistics (void)
*** 866,889 ****
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t count, size, overhead;
    int i;
  
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-60s %10s %10s %10s\n",
! 	   "source location", "Times", "Allocated", "Overhead");
    fprintf (stderr, "-------------------------------------------------------\n");
-   count = 0;
-   size = 0;
-   overhead = 0;
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       size += d->allocated;
!       count += d->times;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
--- 944,969 ----
  #ifdef GATHER_STATISTICS
    int nentries = 0;
    char s[4096];
!   size_t collected = 0, freed = 0, allocated = 0, overhead = 0, times = 0;
    int i;
  
+   ggc_force_collect = true;
+   ggc_collect ();
+ 
    loc_array = xcalloc (sizeof (*loc_array), loc_hash->n_elements);
    fprintf (stderr, "-------------------------------------------------------\n");
!   fprintf (stderr, "\n%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
    htab_traverse (loc_hash, add_statistics, &nentries);
    qsort (loc_array, nentries, sizeof (*loc_array), cmp_statistic);
    for (i = 0; i < nentries; i++)
      {
        struct loc_descriptor *d = loc_array[i];
!       allocated += d->allocated;
!       times += d->times;
!       freed += d->freed;
!       collected += d->collected;
        overhead += d->overhead;
      }
    for (i = 0; i < nentries; i++)
*************** void dump_ggc_loc_statistics (void)
*** 896,908 ****
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  fprintf (stderr, "%-60s %10i %10li %10li:%.3f%%\n", s,
! 		   d->times, (long)d->allocated, (long)d->overhead,
! 		   (d->allocated + d->overhead) *100.0 / (size + overhead));
  	}
      }
!   fprintf (stderr, "%-60s %10ld %10ld %10ld\n",
! 	   "Total", (long)count, (long)size, (long)overhead);
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
--- 976,1001 ----
  	  while ((s2 = strstr (s1, "gcc/")))
  	    s1 = s2 + 4;
  	  sprintf (s, "%s:%i (%s)", s1, d->line, d->function);
! 	  s[48] = 0;
! 	  fprintf (stderr, "%-48s %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li:%4.1f%% %10li\n", s,
! 		   (long)d->collected,
! 		   (d->collected) * 100.0 / collected,
! 		   (long)d->freed,
! 		   (d->freed) * 100.0 / freed,
! 		   (long)(d->allocated + d->overhead - d->freed - d->collected),
! 		   (d->allocated + d->overhead - d->freed - d->collected) * 100.0
! 		   / (allocated + overhead - freed - collected),
! 		   (long)d->overhead,
! 		   d->overhead * 100.0 / overhead,
! 		   (long)d->times);
  	}
      }
!   fprintf (stderr, "%-48s %10ld       %10ld       %10ld       %10ld       %10ld\n",
! 	   "Total", (long)collected, (long)freed,
! 	   (long)(allocated + overhead - freed - collected), (long)overhead,
! 	   (long)times);
!   fprintf (stderr, "%-48s %10s       %10s       %10s       %10s       %10s\n",
! 	   "source location", "Garbage", "Freed", "Leak", "Overhead", "Times");
    fprintf (stderr, "-------------------------------------------------------\n");
  #endif
  }
Index: ggc-page.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-page.c,v
retrieving revision 1.92
diff -c -3 -p -r1.92 ggc-page.c
*** ggc-page.c	22 Mar 2004 02:57:27 -0000	1.92
--- ggc-page.c	2 Sep 2004 16:08:50 -0000
*************** ggc_alloc_stat (size_t size MEM_STAT_DEC
*** 1173,1184 ****
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
- #ifdef GATHER_STATISTICS
-   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size PASS_MEM_STAT);
- #endif
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
--- 1173,1185 ----
        G.page_tails[order]->next = entry;
        G.page_tails[order] = entry;
      }
  
    /* Calculate the object's address.  */
    result = entry->page + object_offset;
+ #ifdef GATHER_STATISTICS
+   ggc_record_overhead (OBJECT_SIZE (order), OBJECT_SIZE (order) - size,
+ 		       result PASS_MEM_STAT);
+ #endif
  
  #ifdef ENABLE_GC_CHECKING
    /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
*************** ggc_free (void *p)
*** 1327,1332 ****
--- 1328,1337 ----
    size_t order = pe->order;
    size_t size = OBJECT_SIZE (order);
  
+ #ifdef GATHER_STATISTICS
+   ggc_free_overhead (p);
+ #endif
+ 
    if (GGC_DEBUG_LEVEL >= 3)
      fprintf (G.debug_file,
  	     "Freeing object, actual size=%lu, at %p on %p\n",
*************** ggc_collect (void)
*** 1971,1977 ****
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand)
      return;
  
    timevar_push (TV_GC);
--- 1976,1982 ----
  
    float min_expand = allocated_last_gc * PARAM_VALUE (GGC_MIN_EXPAND) / 100;
  
!   if (G.allocated < allocated_last_gc + min_expand && !ggc_force_collect)
      return;
  
    timevar_push (TV_GC);
*************** ggc_collect (void)
*** 1993,1998 ****
--- 1998,2006 ----
  
    clear_marks ();
    ggc_mark_roots ();
+ #ifdef GATHER_STATISTICS
+   ggc_prune_overhead_list ();
+ #endif
    poison_pages ();
    validate_free_objects ();
    sweep_pages ();
Index: ggc.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc.h,v
retrieving revision 1.68
diff -c -3 -p -r1.68 ggc.h
*** ggc.h	2 Sep 2004 02:39:15 -0000	1.68
--- ggc.h	2 Sep 2004 16:08:50 -0000
*************** extern struct alloc_zone *garbage_zone;
*** 209,214 ****
--- 209,216 ----
  extern struct alloc_zone *rtl_zone;
  /* For regular tree allocations.  */
  extern struct alloc_zone *tree_zone;
+ /* When set, ggc_collect will do collection.  */
+ extern bool ggc_force_collect;
  
  /* The internal primitive.  */
  extern void *ggc_alloc_stat (size_t MEM_STAT_DECL);
*************** extern void *ggc_calloc (size_t, size_t)
*** 233,239 ****
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t MEM_STAT_DECL);
  
  extern void dump_ggc_loc_statistics (void);
  
--- 235,243 ----
  /* Free a block.  To be used when known for certain it's not reachable.  */
  extern void ggc_free (void *);
   
! extern void ggc_record_overhead (size_t, size_t, void * MEM_STAT_DECL);
! extern void ggc_free_overhead (void *);
! extern void ggc_prune_overhead_list (void);
  
  extern void dump_ggc_loc_statistics (void);
  

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:47       ` Jan Hubicka
@ 2004-09-02 17:15         ` Zack Weinberg
  2004-09-02 17:23           ` Jan Hubicka
  0 siblings, 1 reply; 16+ messages in thread
From: Zack Weinberg @ 2004-09-02 17:15 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: Matt Austern, gcc-patches, stevenb, rth

Jan Hubicka <hubicka@ucw.cz> writes:

>> On Sep 2, 2004, at 9:23 AM, Zack Weinberg wrote:
>> 
>> >Jan Hubicka <hubicka@ucw.cz> writes:
>> >
>> >>Hi,
>> >>here is updated version of patch I sent while reducing memory for GCC
>> >>3.4, it is quite usefull now again...
>> >>
>> >>Hi, this patch improves the per-line statistics by tracking down
>> >>each allocated entity to figure out whether it will be freed,
>> >>garbage collected or leaked.  To rule out ggc_freed values is pretty
>> >>important as these are much cheaper,
>> >...
>> >
>> >Please do timing tests before submitting any changes.  In my
>> >experience ggc_free is *not* cheaper, it is by itself such an
>> >expensive operation that we don't actually gain anything over
>> >letting the garbage collector do its job.
>
> My experience is that you need to free quite a lot of memory to see gain
> (ie you need to avoid at leat one GGC run).  For Gerald's testcase I
> actually measured slight change in execution time with both changes (ie
> one GGC run, but it is because my memory setup is definitly on the
> corner between 22 and 23 GGC runs)

That makes sense.  Note though that the current memory-pressure
heuristics don't run the collector at all for small to medium-sized
input.

> This patch is not adding any ggc_free, just making analysis prettier.  I
> have some extra patches dependent on the framework (ie I use it to
> detect leaks where we keep pointer to something local accidentally and
> it was usefull enought to reduce peak memory usage of combine.c
> compilation from 12MB to 2.8MB.)

Yes.  I wasn't clear enough: I am not objecting to this patch, only
cautioning you not to assume that sprinkling ggc_free calls through
the compiler will speed anything up.

>> What this does mean is that if we know a data structure will be 
>> temporary, we shouldn't use ggc_free or ggc_anything.
>
> Yes, where convenient, I am trying to get out of GGC completely.

Agree, use of xmalloc or obstacks for data with a well-defined
lifetime is a good move.

zw

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:54   ` Jan Hubicka
@ 2004-09-02 17:18     ` Richard Henderson
  2021-08-17  9:17     ` Thomas Schwinge
  1 sibling, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2004-09-02 17:18 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, stevenb

On Thu, Sep 02, 2004 at 06:47:01PM +0200, Jan Hubicka wrote:
> Sorry, I managed to melt two changes together, here is proper patch and
> I am just re-testing it.

Ok.

> +   slot = htab_find_slot_with_hash (ptr_hash, ptr, htab_hash_pointer (ptr), INSERT);

Careful with the line length.


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 17:15         ` Zack Weinberg
@ 2004-09-02 17:23           ` Jan Hubicka
  0 siblings, 0 replies; 16+ messages in thread
From: Jan Hubicka @ 2004-09-02 17:23 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Jan Hubicka, Matt Austern, gcc-patches, stevenb, rth

> Jan Hubicka <hubicka@ucw.cz> writes:
> 
> >> On Sep 2, 2004, at 9:23 AM, Zack Weinberg wrote:
> >> 
> >> >Jan Hubicka <hubicka@ucw.cz> writes:
> >> >
> >> >>Hi,
> >> >>here is updated version of patch I sent while reducing memory for GCC
> >> >>3.4, it is quite usefull now again...
> >> >>
> >> >>Hi, this patch improves the per-line statistics by tracking down
> >> >>each allocated entity to figure out whether it will be freed,
> >> >>garbage collected or leaked.  To rule out ggc_freed values is pretty
> >> >>important as these are much cheaper,
> >> >...
> >> >
> >> >Please do timing tests before submitting any changes.  In my
> >> >experience ggc_free is *not* cheaper, it is by itself such an
> >> >expensive operation that we don't actually gain anything over
> >> >letting the garbage collector do its job.
> >
> > My experience is that you need to free quite a lot of memory to see gain
> > (ie you need to avoid at leat one GGC run).  For Gerald's testcase I
> > actually measured slight change in execution time with both changes (ie
> > one GGC run, but it is because my memory setup is definitly on the
> > corner between 22 and 23 GGC runs)
> 
> That makes sense.  Note though that the current memory-pressure
> heuristics don't run the collector at all for small to medium-sized
> input.

Yes, I do two tests always - Geralds testcase and GCC modules that
should cover both cases.  I will try to post numbers more consistenly.
> 
> > This patch is not adding any ggc_free, just making analysis prettier.  I
> > have some extra patches dependent on the framework (ie I use it to
> > detect leaks where we keep pointer to something local accidentally and
> > it was usefull enought to reduce peak memory usage of combine.c
> > compilation from 12MB to 2.8MB.)
> 
> Yes.  I wasn't clear enough: I am not objecting to this patch, only
> cautioning you not to assume that sprinkling ggc_free calls through
> the compiler will speed anything up.

Yes, I will try to be curefull.  I am experimenting with explicit
allocation/deallocation of gimple nodes, so that will likely show how
well/badly ggc_free scale.
> 
> >> What this does mean is that if we know a data structure will be 
> >> temporary, we shouldn't use ggc_free or ggc_anything.
> >
> > Yes, where convenient, I am trying to get out of GGC completely.
> 
> Agree, use of xmalloc or obstacks for data with a well-defined
> lifetime is a good move.

don't forget about allocpool, it is prettier than obstack ;)

Honza
> 
> zw

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:27   ` Zack Weinberg
  2004-09-02 16:45     ` Matt Austern
@ 2004-09-02 17:28     ` Daniel Jacobowitz
  1 sibling, 0 replies; 16+ messages in thread
From: Daniel Jacobowitz @ 2004-09-02 17:28 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Jan Hubicka, gcc-patches, stevenb, rth

On Thu, Sep 02, 2004 at 09:23:29AM -0700, Zack Weinberg wrote:
> Jan Hubicka <hubicka@ucw.cz> writes:
> 
> > Hi,
> > here is updated version of patch I sent while reducing memory for GCC
> > 3.4, it is quite usefull now again...
> >
> > Hi, this patch improves the per-line statistics by tracking down
> > each allocated entity to figure out whether it will be freed,
> > garbage collected or leaked.  To rule out ggc_freed values is pretty
> > important as these are much cheaper,
> ...
> 
> Please do timing tests before submitting any changes.  In my
> experience ggc_free is *not* cheaper, it is by itself such an
> expensive operation that we don't actually gain anything over
> letting the garbage collector do its job.

Just FYI, in the revamped zone allocator that I've been working on,
ggc_free will be very cheap.  If it's not for the page allocator, then
maybe it's a good idea to have it not do anything for !ENABLE_CHECKING.

-- 
Daniel Jacobowitz

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2004-09-02 16:54   ` Jan Hubicka
  2004-09-02 17:18     ` Richard Henderson
@ 2021-08-17  9:17     ` Thomas Schwinge
  2021-08-17 10:21       ` Richard Biener
  2021-08-17 13:27       ` David Malcolm
  1 sibling, 2 replies; 16+ messages in thread
From: Thomas Schwinge @ 2021-08-17  9:17 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka, David Malcolm

[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]

Hi!

On 2004-09-02T18:47:01+0200, Jan Hubicka <hubicka@ucw.cz> wrote:
> *** ggc-common.c      9 Aug 2004 20:19:29 -0000       1.88
> --- ggc-common.c      2 Sep 2004 16:08:50 -0000

> + /* When set, ggc_collect will do collection.  */
> + bool ggc_force_collect;

> *************** void dump_ggc_loc_statistics (void)

> +   ggc_force_collect = true;
> +   ggc_collect ();

> *************** ggc_collect (void)

> !   if (G.allocated < allocated_last_gc + min_expand)

> !   if (G.allocated < allocated_last_gc + min_expand && !ggc_force_collect)

> *** ggc.h     2 Sep 2004 02:39:15 -0000       1.68
> --- ggc.h     2 Sep 2004 16:08:50 -0000

> + /* When set, ggc_collect will do collection.  */
> + extern bool ggc_force_collect;

This has later acquired another use in the GCC selftests.

I wonder if we shouldn't simplify the interface per the attached "Turn
global 'ggc_force_collect' variable into 'force_collect' parameter to
'ggc_collect'"?  OK to push to master branch after bootstrap testing?


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Turn-global-ggc_force_collect-variable-into-force_co.patch --]
[-- Type: text/x-diff, Size: 9796 bytes --]

From 58e7dc524f65593166102c553a2e6e11e6b20b60 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 17 Aug 2021 10:47:02 +0200
Subject: [PATCH] Turn global 'ggc_force_collect' variable into 'force_collect'
 parameter to 'ggc_collect'

This simplifies the interface and gets us rid of a global variable.
No change in behavior.

Clean-up for 2004-09-02 CVS commit (Subversion r86974,
Git commit 0772402279c0161fe41784911b52c77e12803c42)
"Better memory statistics, take 2".

	gcc/
	* ggc.h (ggc_collect): Add 'force_collect' parameter.
	* ggc-page.c (ggc_collect): Use that one instead of global
	'ggc_force_collect'.  Adjust all users.
	* doc/gty.texi (Invoking the garbage collector): Update.
	* ggc-internal.h (ggc_force_collect): Remove.
	* ggc-common.c (ggc_force_collect): Likewise.
	* selftest.h (forcibly_ggc_collect): Remove.
	* ggc-tests.c (selftest::forcibly_ggc_collect): Likewise.
	* read-rtl-function.c (test_loading_labels): Adjust.
	* selftest-run-tests.c (run_tests): Likewise.
---
 gcc/doc/gty.texi         |  5 ++++-
 gcc/ggc-common.c         |  8 +-------
 gcc/ggc-internal.h       |  3 ---
 gcc/ggc-page.c           |  4 ++--
 gcc/ggc-tests.c          | 29 +++++++++--------------------
 gcc/ggc.h                |  6 ++++--
 gcc/read-rtl-function.c  |  2 +-
 gcc/selftest-run-tests.c |  2 +-
 gcc/selftest.h           |  5 -----
 9 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/gcc/doc/gty.texi b/gcc/doc/gty.texi
index cf070c1f7f7..b667d1d19ba 100644
--- a/gcc/doc/gty.texi
+++ b/gcc/doc/gty.texi
@@ -654,7 +654,10 @@ The GCC garbage collector GGC is only invoked explicitly. In contrast
 with many other garbage collectors, it is not implicitly invoked by
 allocation routines when a lot of memory has been consumed. So the
 only way to have GGC reclaim storage is to call the @code{ggc_collect}
-function explicitly.  This call is an expensive operation, as it may
+function explicitly.
+When the @var{force_collect} parameter is set or otherwise an internal
+heuristic decides whether to actually collect, this call is
+potentially an expensive operation, as it may
 have to scan the entire heap.  Beware that local variables (on the GCC
 call stack) are not followed by such an invocation (as many other
 garbage collectors do): you should reference all your data from static
diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index 357bda13f97..f38e4d5020d 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -31,9 +31,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "plugin.h"
 #include "options.h"
 
-/* When set, ggc_collect will do collection.  */
-bool ggc_force_collect;
-
 /* When true, protect the contents of the identifier hash table.  */
 bool ggc_protect_identifiers = true;
 
@@ -965,12 +962,9 @@ dump_ggc_loc_statistics ()
   if (! GATHER_STATISTICS)
     return;
 
-  ggc_force_collect = true;
-  ggc_collect ();
+  ggc_collect (true);
 
   ggc_mem_desc.dump (GGC_ORIGIN);
-
-  ggc_force_collect = false;
 }
 
 /* Record ALLOCATED and OVERHEAD bytes to descriptor NAME:LINE (FUNCTION).  */
diff --git a/gcc/ggc-internal.h b/gcc/ggc-internal.h
index 39850cd6230..4dcfb4c008c 100644
--- a/gcc/ggc-internal.h
+++ b/gcc/ggc-internal.h
@@ -88,9 +88,6 @@ extern void ggc_pch_read (FILE *, void *);
 
 /* Allocation and collection.  */
 
-/* When set, ggc_collect will do collection.  */
-extern bool ggc_force_collect;
-
 extern void ggc_record_overhead (size_t, size_t, void * FINAL_MEM_STAT_DECL);
 
 extern void ggc_free_overhead (void *);
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index 1b09f0da94f..a6fbecaa1d8 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -2184,7 +2184,7 @@ validate_free_objects (void)
 /* Top level mark-and-sweep routine.  */
 
 void
-ggc_collect (void)
+ggc_collect (bool force_collect)
 {
   /* Avoid frequent unnecessary work by skipping collection if the
      total allocations haven't expanded much since the last
@@ -2196,7 +2196,7 @@ ggc_collect (void)
   memory_block_pool::trim ();
 
   float min_expand = allocated_last_gc * param_ggc_min_expand / 100;
-  if (G.allocated < allocated_last_gc + min_expand && !ggc_force_collect)
+  if (G.allocated < allocated_last_gc + min_expand && !force_collect)
     return;
 
   timevar_push (TV_GC);
diff --git a/gcc/ggc-tests.c b/gcc/ggc-tests.c
index 4ee95506625..2891c20ceac 100644
--- a/gcc/ggc-tests.c
+++ b/gcc/ggc-tests.c
@@ -22,21 +22,10 @@ along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "tree-core.h"
 #include "tree.h"
-#include "ggc-internal.h" /* (for ggc_force_collect).  */
 #include "selftest.h"
 
 #if CHECKING_P
 
-/* A helper function for writing ggc tests.  */
-
-void
-selftest::forcibly_ggc_collect ()
-{
-  ggc_force_collect = true;
-  ggc_collect ();
-  ggc_force_collect = false;
-}
-
 /* The various GTY markers must be outside of a namespace to be seen by
    gengtype, so we don't put this file within the selftest namespace.  */
 
@@ -58,7 +47,7 @@ test_basic_struct ()
   root_test_struct = ggc_cleared_alloc <test_struct> ();
   root_test_struct->other = ggc_cleared_alloc <test_struct> ();
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   ASSERT_TRUE (ggc_marked_p (root_test_struct));
   ASSERT_TRUE (ggc_marked_p (root_test_struct->other));
@@ -88,7 +77,7 @@ test_length ()
   for (int i = 0; i < count; i++)
     root_test_of_length->elem[i] = ggc_cleared_alloc <test_of_length> ();
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   ASSERT_TRUE (ggc_marked_p (root_test_of_length));
   for (int i = 0; i < count; i++)
@@ -162,7 +151,7 @@ test_union ()
   test_struct *referenced_by_other = ggc_cleared_alloc <test_struct> ();
   other->m_ptr = referenced_by_other;
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   ASSERT_TRUE (ggc_marked_p (root_test_of_union_1));
   ASSERT_TRUE (ggc_marked_p (ts));
@@ -203,7 +192,7 @@ test_finalization ()
 
   test_struct_with_dtor::dtor_call_count = 0;
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   /* Verify that the destructor was run for each instance.  */
   ASSERT_EQ (count, test_struct_with_dtor::dtor_call_count);
@@ -221,7 +210,7 @@ test_deletable_global ()
   test_of_deletable = ggc_cleared_alloc <test_struct> ();
   ASSERT_TRUE (test_of_deletable != NULL);
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   ASSERT_EQ (NULL, test_of_deletable);
 }
@@ -294,7 +283,7 @@ test_inheritance ()
   test_some_subclass_as_base_ptr = new some_subclass ();
   test_some_other_subclass_as_base_ptr = new some_other_subclass ();
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   /* Verify that the roots and everything referenced by them got marked
      (both for fields in the base class and those in subclasses).  */
@@ -373,7 +362,7 @@ test_chain_next ()
       tail_node = new_node;
     }
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   /* If we got here, we survived.  */
 
@@ -440,7 +429,7 @@ test_user_struct ()
 
   num_calls_to_user_gt_ggc_mx = 0;
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   ASSERT_TRUE (ggc_marked_p (root_user_struct_ptr));
   ASSERT_TRUE (ggc_marked_p (referenced));
@@ -458,7 +447,7 @@ test_tree_marking ()
 {
   dummy_unittesting_tree = build_int_cst (integer_type_node, 1066);
 
-  selftest::forcibly_ggc_collect ();
+  ggc_collect (true);
 
   ASSERT_TRUE (ggc_marked_p (dummy_unittesting_tree));
 }
diff --git a/gcc/ggc.h b/gcc/ggc.h
index 92884717f5c..0f640b255e8 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -262,8 +262,10 @@ extern const char *ggc_alloc_string (const char *contents, int length
 #define ggc_strdup(S) ggc_alloc_string ((S), -1 MEM_STAT_INFO)
 
 /* Invoke the collector.  Garbage collection occurs only when this
-   function is called, not during allocations.  */
-extern void ggc_collect	(void);
+   function is called, not during allocations.
+   Unless FORCE_COLLECT, an internal heuristic decides whether to actually
+   collect.  */
+extern void ggc_collect (bool force_collect = false);
 
 /* Return unused memory pages to the system.  */
 extern void ggc_trim (void);
diff --git a/gcc/read-rtl-function.c b/gcc/read-rtl-function.c
index ca580d3f71e..0badfb98f9d 100644
--- a/gcc/read-rtl-function.c
+++ b/gcc/read-rtl-function.c
@@ -1861,7 +1861,7 @@ test_loading_labels ()
 
   /* Ensure that label names read from a dump are GC-managed
      and are found through the insn.  */
-  forcibly_ggc_collect ();
+  ggc_collect (true);
   ASSERT_TRUE (ggc_marked_p (insn_200));
   ASSERT_TRUE (ggc_marked_p (LABEL_NAME (insn_200)));
 }
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 1b5583e476a..10881fce230 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -128,7 +128,7 @@ selftest::run_tests ()
      issues.  For example, if any GC-managed items have buggy (or missing)
      finalizers, this last collection will ensure that things that were
      failed to be finalized can be detected by valgrind.  */
-  forcibly_ggc_collect ();
+  ggc_collect (true);
 
   /* Finished running tests; the test_runner dtor will print a summary.  */
 }
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 80459d63a39..58d8d383cee 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -190,11 +190,6 @@ for_each_line_table_case (void (*testcase) (const line_table_case &));
 
 extern char *read_file (const location &loc, const char *path);
 
-/* A helper function for writing tests that interact with the
-   garbage collector.  */
-
-extern void forcibly_ggc_collect ();
-
 /* Convert a path relative to SRCDIR/gcc/testsuite/selftests
    to a real path (either absolute, or relative to pwd).
    The result should be freed by the caller.  */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2021-08-17  9:17     ` Thomas Schwinge
@ 2021-08-17 10:21       ` Richard Biener
  2021-08-17 13:27       ` David Malcolm
  1 sibling, 0 replies; 16+ messages in thread
From: Richard Biener @ 2021-08-17 10:21 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: GCC Patches, Jan Hubicka

On Tue, Aug 17, 2021 at 11:18 AM Thomas Schwinge
<thomas@codesourcery.com> wrote:
>
> Hi!
>
> On 2004-09-02T18:47:01+0200, Jan Hubicka <hubicka@ucw.cz> wrote:
> > *** ggc-common.c      9 Aug 2004 20:19:29 -0000       1.88
> > --- ggc-common.c      2 Sep 2004 16:08:50 -0000
>
> > + /* When set, ggc_collect will do collection.  */
> > + bool ggc_force_collect;
>
> > *************** void dump_ggc_loc_statistics (void)
>
> > +   ggc_force_collect = true;
> > +   ggc_collect ();
>
> > *************** ggc_collect (void)
>
> > !   if (G.allocated < allocated_last_gc + min_expand)
>
> > !   if (G.allocated < allocated_last_gc + min_expand && !ggc_force_collect)
>
> > *** ggc.h     2 Sep 2004 02:39:15 -0000       1.68
> > --- ggc.h     2 Sep 2004 16:08:50 -0000
>
> > + /* When set, ggc_collect will do collection.  */
> > + extern bool ggc_force_collect;
>
> This has later acquired another use in the GCC selftests.
>
> I wonder if we shouldn't simplify the interface per the attached "Turn
> global 'ggc_force_collect' variable into 'force_collect' parameter to
> 'ggc_collect'"?  OK to push to master branch after bootstrap testing?

OK.

Thanks,
Richard.

>
> Grüße
>  Thomas
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2021-08-17  9:17     ` Thomas Schwinge
  2021-08-17 10:21       ` Richard Biener
@ 2021-08-17 13:27       ` David Malcolm
  2021-08-17 19:31         ` Thomas Schwinge
  1 sibling, 1 reply; 16+ messages in thread
From: David Malcolm @ 2021-08-17 13:27 UTC (permalink / raw)
  To: Thomas Schwinge, gcc-patches; +Cc: Jan Hubicka

On Tue, 2021-08-17 at 11:17 +0200, Thomas Schwinge wrote:
> Hi!
> 
> On 2004-09-02T18:47:01+0200, Jan Hubicka <hubicka@ucw.cz> wrote:
> > *** ggc-common.c      9 Aug 2004 20:19:29 -0000       1.88
> > --- ggc-common.c      2 Sep 2004 16:08:50 -0000
> 
> > + /* When set, ggc_collect will do collection.  */
> > + bool ggc_force_collect;
> 
> > *************** void dump_ggc_loc_statistics (void)
> 
> > +   ggc_force_collect = true;
> > +   ggc_collect ();
> 
> > *************** ggc_collect (void)
> 
> > !   if (G.allocated < allocated_last_gc + min_expand)
> 
> > !   if (G.allocated < allocated_last_gc + min_expand &&
> > !ggc_force_collect)
> 
> > *** ggc.h     2 Sep 2004 02:39:15 -0000       1.68
> > --- ggc.h     2 Sep 2004 16:08:50 -0000
> 
> > + /* When set, ggc_collect will do collection.  */
> > + extern bool ggc_force_collect;
> 
> This has later acquired another use in the GCC selftests.
> 
> I wonder if we shouldn't simplify the interface per the attached
> "Turn
> global 'ggc_force_collect' variable into 'force_collect' parameter to
> 'ggc_collect'"?  OK to push to master branch after bootstrap testing?

Looks good to me, but bool params can be unclear - maybe introduce an
enum to make the meaning more explicit to the reader of the code?

e.g.

enum gcc_collect_when
{
  GGC_COLLECT_UNDER_MEMORY_PRESSURE,
  GGC_COLLECT_ALWAYS
};

or somesuch???

Dave



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2021-08-17 13:27       ` David Malcolm
@ 2021-08-17 19:31         ` Thomas Schwinge
  2021-08-18  7:02           ` Richard Biener
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Schwinge @ 2021-08-17 19:31 UTC (permalink / raw)
  To: David Malcolm, gcc-patches; +Cc: Jan Hubicka, Richard Biener

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

Hi!

On 2021-08-17T09:27:46-0400, David Malcolm via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> On Tue, 2021-08-17 at 11:17 +0200, Thomas Schwinge wrote:
>> "Turn
>> global 'ggc_force_collect' variable into 'force_collect' parameter to
>> 'ggc_collect'"

> Looks good to me, but bool params can be unclear - maybe introduce an
> enum to make the meaning more explicit to the reader of the code?

I actually had contemplated that, but then went for the simpler 'bool'
variant...  ;-) But yes, it's a good suggestion, thanks.  OK to push the
attached "Turn 'bool force_collect' parameter to 'ggc_collect' into an
'enum ggc_collect mode'"?


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Turn-bool-force_collect-parameter-to-ggc_collect-int.patch --]
[-- Type: text/x-diff, Size: 7689 bytes --]

From 73e4b9869e2cc515ee3393bffa220e775bbbcd45 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 17 Aug 2021 21:15:46 +0200
Subject: [PATCH] Turn 'bool force_collect' parameter to 'ggc_collect' into an
 'enum ggc_collect mode'

... to make the meaning more explicit to the reader of the code.

Follow-up to recent commit 0edf2e81bb02cba43b649b3f6e7258b68a779ac0
"Turn global 'ggc_force_collect' variable into 'force_collect' parameter to
'ggc_collect'".

	gcc/
	* ggc.h (enum ggc_collect): New.
	(ggc_collect): Use it.
	* ggc-page.c: Adjust.
	* ggc-common.c: Likewise.
	* ggc-tests.c: Likewise.
	* read-rtl-function.c: Likewise.
	* selftest-run-tests.c: Likewise.
	* doc/gty.texi (Invoking the garbage collector): Likewise.

Suggested-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/doc/gty.texi         |  6 +++---
 gcc/ggc-common.c         |  2 +-
 gcc/ggc-page.c           |  5 +++--
 gcc/ggc-tests.c          | 18 +++++++++---------
 gcc/ggc.h                | 10 ++++++----
 gcc/read-rtl-function.c  |  2 +-
 gcc/selftest-run-tests.c |  2 +-
 7 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/gcc/doc/gty.texi b/gcc/doc/gty.texi
index b667d1d19ba..2ad7793191b 100644
--- a/gcc/doc/gty.texi
+++ b/gcc/doc/gty.texi
@@ -655,9 +655,9 @@ with many other garbage collectors, it is not implicitly invoked by
 allocation routines when a lot of memory has been consumed. So the
 only way to have GGC reclaim storage is to call the @code{ggc_collect}
 function explicitly.
-When the @var{force_collect} parameter is set or otherwise an internal
-heuristic decides whether to actually collect, this call is
-potentially an expensive operation, as it may
+With @var{mode} @code{GGC_COLLECT_FORCE} or otherwise (default
+@code{GGC_COLLECT_HEURISTIC}) when the internal heuristic decides to
+collect, this call is potentially an expensive operation, as it may
 have to scan the entire heap.  Beware that local variables (on the GCC
 call stack) are not followed by such an invocation (as many other
 garbage collectors do): you should reference all your data from static
diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index f38e4d5020d..32ba5be42b2 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -962,7 +962,7 @@ dump_ggc_loc_statistics ()
   if (! GATHER_STATISTICS)
     return;
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   ggc_mem_desc.dump (GGC_ORIGIN);
 }
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index a6fbecaa1d8..1c49643e7e7 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -2184,7 +2184,7 @@ validate_free_objects (void)
 /* Top level mark-and-sweep routine.  */
 
 void
-ggc_collect (bool force_collect)
+ggc_collect (enum ggc_collect mode)
 {
   /* Avoid frequent unnecessary work by skipping collection if the
      total allocations haven't expanded much since the last
@@ -2196,7 +2196,8 @@ ggc_collect (bool force_collect)
   memory_block_pool::trim ();
 
   float min_expand = allocated_last_gc * param_ggc_min_expand / 100;
-  if (G.allocated < allocated_last_gc + min_expand && !force_collect)
+  if (mode == GGC_COLLECT_HEURISTIC
+      && G.allocated < allocated_last_gc + min_expand)
     return;
 
   timevar_push (TV_GC);
diff --git a/gcc/ggc-tests.c b/gcc/ggc-tests.c
index 2891c20ceac..e83f7019863 100644
--- a/gcc/ggc-tests.c
+++ b/gcc/ggc-tests.c
@@ -47,7 +47,7 @@ test_basic_struct ()
   root_test_struct = ggc_cleared_alloc <test_struct> ();
   root_test_struct->other = ggc_cleared_alloc <test_struct> ();
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   ASSERT_TRUE (ggc_marked_p (root_test_struct));
   ASSERT_TRUE (ggc_marked_p (root_test_struct->other));
@@ -77,7 +77,7 @@ test_length ()
   for (int i = 0; i < count; i++)
     root_test_of_length->elem[i] = ggc_cleared_alloc <test_of_length> ();
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   ASSERT_TRUE (ggc_marked_p (root_test_of_length));
   for (int i = 0; i < count; i++)
@@ -151,7 +151,7 @@ test_union ()
   test_struct *referenced_by_other = ggc_cleared_alloc <test_struct> ();
   other->m_ptr = referenced_by_other;
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   ASSERT_TRUE (ggc_marked_p (root_test_of_union_1));
   ASSERT_TRUE (ggc_marked_p (ts));
@@ -192,7 +192,7 @@ test_finalization ()
 
   test_struct_with_dtor::dtor_call_count = 0;
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   /* Verify that the destructor was run for each instance.  */
   ASSERT_EQ (count, test_struct_with_dtor::dtor_call_count);
@@ -210,7 +210,7 @@ test_deletable_global ()
   test_of_deletable = ggc_cleared_alloc <test_struct> ();
   ASSERT_TRUE (test_of_deletable != NULL);
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   ASSERT_EQ (NULL, test_of_deletable);
 }
@@ -283,7 +283,7 @@ test_inheritance ()
   test_some_subclass_as_base_ptr = new some_subclass ();
   test_some_other_subclass_as_base_ptr = new some_other_subclass ();
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   /* Verify that the roots and everything referenced by them got marked
      (both for fields in the base class and those in subclasses).  */
@@ -362,7 +362,7 @@ test_chain_next ()
       tail_node = new_node;
     }
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   /* If we got here, we survived.  */
 
@@ -429,7 +429,7 @@ test_user_struct ()
 
   num_calls_to_user_gt_ggc_mx = 0;
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   ASSERT_TRUE (ggc_marked_p (root_user_struct_ptr));
   ASSERT_TRUE (ggc_marked_p (referenced));
@@ -447,7 +447,7 @@ test_tree_marking ()
 {
   dummy_unittesting_tree = build_int_cst (integer_type_node, 1066);
 
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   ASSERT_TRUE (ggc_marked_p (dummy_unittesting_tree));
 }
diff --git a/gcc/ggc.h b/gcc/ggc.h
index 0f640b255e8..5e921d957fd 100644
--- a/gcc/ggc.h
+++ b/gcc/ggc.h
@@ -262,10 +262,12 @@ extern const char *ggc_alloc_string (const char *contents, int length
 #define ggc_strdup(S) ggc_alloc_string ((S), -1 MEM_STAT_INFO)
 
 /* Invoke the collector.  Garbage collection occurs only when this
-   function is called, not during allocations.
-   Unless FORCE_COLLECT, an internal heuristic decides whether to actually
-   collect.  */
-extern void ggc_collect (bool force_collect = false);
+   function is called, not during allocations.  */
+enum ggc_collect {
+  GGC_COLLECT_HEURISTIC,
+  GGC_COLLECT_FORCE
+};
+extern void ggc_collect (enum ggc_collect mode = GGC_COLLECT_HEURISTIC);
 
 /* Return unused memory pages to the system.  */
 extern void ggc_trim (void);
diff --git a/gcc/read-rtl-function.c b/gcc/read-rtl-function.c
index 0badfb98f9d..941d1e158a3 100644
--- a/gcc/read-rtl-function.c
+++ b/gcc/read-rtl-function.c
@@ -1861,7 +1861,7 @@ test_loading_labels ()
 
   /* Ensure that label names read from a dump are GC-managed
      and are found through the insn.  */
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
   ASSERT_TRUE (ggc_marked_p (insn_200));
   ASSERT_TRUE (ggc_marked_p (LABEL_NAME (insn_200)));
 }
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 10881fce230..6a8f291f5dd 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -128,7 +128,7 @@ selftest::run_tests ()
      issues.  For example, if any GC-managed items have buggy (or missing)
      finalizers, this last collection will ensure that things that were
      failed to be finalized can be detected by valgrind.  */
-  ggc_collect (true);
+  ggc_collect (GGC_COLLECT_FORCE);
 
   /* Finished running tests; the test_runner dtor will print a summary.  */
 }
-- 
2.30.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Better memory statistics, take 2
  2021-08-17 19:31         ` Thomas Schwinge
@ 2021-08-18  7:02           ` Richard Biener
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Biener @ 2021-08-18  7:02 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: David Malcolm, GCC Patches, Jan Hubicka

On Tue, Aug 17, 2021 at 9:31 PM Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> Hi!
>
> On 2021-08-17T09:27:46-0400, David Malcolm via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> > On Tue, 2021-08-17 at 11:17 +0200, Thomas Schwinge wrote:
> >> "Turn
> >> global 'ggc_force_collect' variable into 'force_collect' parameter to
> >> 'ggc_collect'"
>
> > Looks good to me, but bool params can be unclear - maybe introduce an
> > enum to make the meaning more explicit to the reader of the code?
>
> I actually had contemplated that, but then went for the simpler 'bool'
> variant...  ;-) But yes, it's a good suggestion, thanks.  OK to push the
> attached "Turn 'bool force_collect' parameter to 'ggc_collect' into an
> 'enum ggc_collect mode'"?

OK.

>
> Grüße
>  Thomas
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-08-18  7:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-03-05 18:25 Make per-call statistics to take into account ggc_free Jan Hubicka
2004-03-19  8:14 ` Jan Hubicka
2004-09-02 16:12 ` Better memory statistics, take 2 Jan Hubicka
2004-09-02 16:27   ` Zack Weinberg
2004-09-02 16:45     ` Matt Austern
2004-09-02 16:47       ` Jan Hubicka
2004-09-02 17:15         ` Zack Weinberg
2004-09-02 17:23           ` Jan Hubicka
2004-09-02 17:28     ` Daniel Jacobowitz
2004-09-02 16:54   ` Jan Hubicka
2004-09-02 17:18     ` Richard Henderson
2021-08-17  9:17     ` Thomas Schwinge
2021-08-17 10:21       ` Richard Biener
2021-08-17 13:27       ` David Malcolm
2021-08-17 19:31         ` Thomas Schwinge
2021-08-18  7:02           ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).