public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Memory statistics enhancement.
@ 2015-06-02 11:14 mliska
  2015-06-02 11:18 ` [PATCH 2/2] Fix memory report layout at various places mliska
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: mliska @ 2015-06-02 11:14 UTC (permalink / raw)
  To: gcc-patches

Hi.

Following 2 patches improve memory statistics infrastructure. First one
ports pool allocator to the new infrastructure. And the second one makes
column alignment properly.

Both can bootstrap on x86_64-linux-pc and survive regression tests.

Ready for trunk?
Thank you,
Martin

Port pool-allocator memory stats to a new infrastructure.

gcc/ChangeLog:

2015-06-02  Martin Liska  <mliska@suse.cz>

	* alloc-pool.c (allocate_pool_descriptor): Remove.
	(struct pool_output_info): Likewise.
	(print_alloc_pool_statistics): Likewise.
	(dump_alloc_pool_statistics): Likewise.
	* alloc-pool.h (struct pool_usage): New struct.
	(pool_allocator::initialize): Change usage of memory statistics
	to a new interface.
	(pool_allocator::release): Likewise.
	(pool_allocator::allocate): Likewise.
	(pool_allocator::remove): Likewise.
	* mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
	for a pool allocator.
	* mem-stats.h (struct mem_location): Add new ctor.
	(struct mem_usage): Add counter for number of
	instances.
	(mem_alloc_description::register_descriptor): New overload of
	the function.
---
 gcc/alloc-pool.c       |  60 +----------------------------
 gcc/alloc-pool.h       | 102 +++++++++++++++++++++++++++++++++++++++----------
 gcc/mem-stats-traits.h |   3 +-
 gcc/mem-stats.h        |  69 ++++++++++++++++++++++-----------
 4 files changed, 132 insertions(+), 102 deletions(-)

diff --git a/gcc/alloc-pool.c b/gcc/alloc-pool.c
index e9fdc86..601c2b7 100644
--- a/gcc/alloc-pool.c
+++ b/gcc/alloc-pool.c
@@ -26,70 +26,14 @@ along with GCC; see the file COPYING3.  If not see
 #include "hash-map.h"
 
 ALLOC_POOL_ID_TYPE last_id;
-
-/* Hashtable mapping alloc_pool names to descriptors.  */
-hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
-
-struct alloc_pool_descriptor *
-allocate_pool_descriptor (const char *name)
-{
-  if (!alloc_pool_hash)
-    alloc_pool_hash = new hash_map<const char *, alloc_pool_descriptor> (10,
-									 false,
-									 false);
-
-  return &alloc_pool_hash->get_or_insert (name);
-}
-
-/* Output per-alloc_pool statistics.  */
-
-/* Used to accumulate statistics about alloc_pool sizes.  */
-struct pool_output_info
-{
-  unsigned long total_created;
-  unsigned long total_allocated;
-};
-
-/* Called via hash_map.traverse.  Output alloc_pool descriptor pointed out by
-   SLOT and update statistics.  */
-bool
-print_alloc_pool_statistics (const char *const &name,
-			     const alloc_pool_descriptor &d,
-			     struct pool_output_info *i)
-{
-  if (d.allocated)
-    {
-      fprintf (stderr,
-	       "%-22s %6d %10lu %10lu(%10lu) %10lu(%10lu) %10lu(%10lu)\n",
-	       name, d.elt_size, d.created, d.allocated,
-	       d.allocated / d.elt_size, d.peak, d.peak / d.elt_size,
-	       d.current, d.current / d.elt_size);
-      i->total_allocated += d.allocated;
-      i->total_created += d.created;
-    }
-  return 1;
-}
+mem_alloc_description<pool_usage> pool_allocator_usage;
 
 /* Output per-alloc_pool memory usage statistics.  */
 void
 dump_alloc_pool_statistics (void)
 {
-  struct pool_output_info info;
-
   if (! GATHER_STATISTICS)
     return;
 
-  if (!alloc_pool_hash)
-    return;
-
-  fprintf (stderr, "\nAlloc-pool Kind         Elt size  Pools  Allocated (elts)            Peak (elts)            Leak (elts)\n");
-  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
-  info.total_created = 0;
-  info.total_allocated = 0;
-  alloc_pool_hash->traverse <struct pool_output_info *,
-			     print_alloc_pool_statistics> (&info);
-  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
-  fprintf (stderr, "%-22s           %7lu %10lu\n",
-	   "Total", info.total_created, info.total_allocated);
-  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
+  pool_allocator_usage.dump (ALLOC_POOL);
 }
diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
index 96a1342..a1727ce 100644
--- a/gcc/alloc-pool.h
+++ b/gcc/alloc-pool.h
@@ -26,6 +26,71 @@ extern void dump_alloc_pool_statistics (void);
 
 typedef unsigned long ALLOC_POOL_ID_TYPE;
 
+/* Pool allocator memory usage.  */
+struct pool_usage: public mem_usage
+{
+  /* Default contructor.  */
+  pool_usage (): m_element_size (0), m_pool_name ("") {}
+  /* Constructor.  */
+  pool_usage (size_t allocated, size_t times, size_t peak,
+	      size_t instances, size_t element_size,
+	      const char *pool_name)
+    : mem_usage (allocated, times, peak, instances),
+      m_element_size (element_size),
+      m_pool_name (pool_name) {}
+
+  /* Sum the usage with SECOND usage.  */
+  pool_usage operator+ (const pool_usage &second)
+  {
+    return pool_usage (m_allocated + second.m_allocated,
+			     m_times + second.m_times,
+			     m_peak + second.m_peak,
+			     m_instances + second.m_instances,
+			     m_element_size, m_pool_name);
+  }
+
+  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
+  inline void dump (mem_location *loc, mem_usage &total) const
+  {
+    char s[4096];
+    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
+	     loc->m_line, loc->m_function);
+
+    s[48] = '\0';
+
+    fprintf (stderr, "%-32s%-48s %6li%10li:%5.1f%%%10li%10li:%5.1f%%%12li\n",
+	     m_pool_name, s, (long)m_instances,
+	     (long)m_allocated, get_percent (m_allocated, total.m_allocated),
+	     (long)m_peak, (long)m_times,
+	     get_percent (m_times, total.m_times),
+	     (long)m_element_size);
+  }
+
+  /* Dump header with NAME.  */
+  static inline void dump_header (const char *name)
+  {
+    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
+	     "Pools", "Leak", "Peak", "Times", "Elt size");
+    print_dash_line ();
+  }
+
+  /* Dump footer.  */
+  inline void dump_footer ()
+  {
+    print_dash_line ();
+    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
+	     (long)m_allocated);
+    print_dash_line ();
+  }
+
+  /* Element size.  */
+  size_t m_element_size;
+  /* Pool name.  */
+  const char *m_pool_name;
+};
+
+extern mem_alloc_description<pool_usage> pool_allocator_usage;
+
 /* Type based memory pool allocator.  */
 template <typename T>
 class pool_allocator
@@ -35,7 +100,7 @@ public:
      has NUM elements.  The allocator support EXTRA_SIZE and can
      potentially IGNORE_TYPE_SIZE.  */
   pool_allocator (const char *name, size_t num, size_t extra_size = 0,
-		  bool ignore_type_size = false);
+		  bool ignore_type_size = false CXX_MEM_STAT_INFO);
   ~pool_allocator ();
   void release ();
   void release_if_empty ();
@@ -122,6 +187,8 @@ private:
   size_t m_extra_size;
   /* Flag if a pool allocator is initialized.  */
   bool m_initialized;
+  /* Memory allocation location.  */
+  mem_location m_location;
 };
 
 /* Last used ID.  */
@@ -151,19 +218,16 @@ struct alloc_pool_descriptor
 /* Hashtable mapping alloc_pool names to descriptors.  */
 extern hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
 
-/* For given name, return descriptor, create new if needed.  */
-alloc_pool_descriptor *
-allocate_pool_descriptor (const char *name);
-
 template <typename T>
 inline
 pool_allocator<T>::pool_allocator (const char *name, size_t num,
-				   size_t extra_size, bool ignore_type_size):
+				   size_t extra_size, bool ignore_type_size
+				   MEM_STAT_DECL):
   m_name (name), m_elts_per_block (num), m_returned_free_list (NULL),
   m_virgin_free_list (NULL), m_virgin_elts_remaining (0), m_elts_allocated (0),
   m_elts_free (0), m_blocks_allocated (0), m_block_list (NULL),
   m_ignore_type_size (ignore_type_size), m_extra_size (extra_size),
-  m_initialized (false) {}
+  m_initialized (false), m_location (ALLOC_POOL, false PASS_MEM_STAT) {}
 
 /* Initialize a pool allocator.  */
 
@@ -196,9 +260,11 @@ pool_allocator<T>::initialize ()
 
   if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-      desc->elt_size = size;
-      desc->created++;
+      pool_usage *u = pool_allocator_usage.register_descriptor
+	(this, new mem_location (m_location));
+
+      u->m_element_size = m_elt_size;
+      u->m_pool_name = m_name;
     }
 
   /* List header size should be a multiple of 8.  */
@@ -235,10 +301,10 @@ pool_allocator<T>::release ()
       free (block);
     }
 
-  if (GATHER_STATISTICS && false)
+  if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-      desc->current -= (m_elts_allocated - m_elts_free) * m_elt_size;
+      pool_allocator_usage.release_instance_overhead (this,
+	(m_elts_allocated - m_elts_free) * m_elt_size);
     }
 
   m_returned_free_list = NULL;
@@ -279,12 +345,7 @@ pool_allocator<T>::allocate ()
 
   if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-
-      desc->allocated += m_elt_size;
-      desc->current += m_elt_size;
-      if (desc->peak < desc->current)
-	desc->peak = desc->current;
+      pool_allocator_usage.register_instance_overhead (m_elt_size, this);
     }
 
 #ifdef ENABLE_VALGRIND_ANNOTATIONS
@@ -383,8 +444,7 @@ pool_allocator<T>::remove (T *object)
 
   if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-      desc->current -= m_elt_size;
+      pool_allocator_usage.release_instance_overhead (this, m_elt_size);
     }
 }
 
diff --git a/gcc/mem-stats-traits.h b/gcc/mem-stats-traits.h
index de1614e..f7843f2 100644
--- a/gcc/mem-stats-traits.h
+++ b/gcc/mem-stats-traits.h
@@ -10,11 +10,12 @@ enum mem_alloc_origin
   VEC,
   BITMAP,
   GGC,
+  ALLOC_POOL,
   MEM_ALLOC_ORIGIN_LENGTH
 };
 
 /* Verbose names of the memory allocation origin.  */
 static const char * mem_alloc_origin_names[] = { "Hash tables", "Hash maps",
-  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory" };
+  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory", "Allocation pool" };
 
 #endif // GCC_MEM_STATS_TRAITS_H
diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index ac47231..fea68fa 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -18,11 +18,16 @@ struct mem_location
   inline mem_location () {}
 
   /* Constructor.  */
-  inline mem_location (const char *filename, const char *function, int line,
-		mem_alloc_origin origin, bool ggc):
+  inline mem_location (mem_alloc_origin origin, bool ggc,
+		const char *filename = NULL, int line = 0, const char *function = NULL):
     m_filename (filename), m_function (function), m_line (line), m_origin
     (origin), m_ggc (ggc) {}
 
+  /* Copy constructor.  */
+  inline mem_location (mem_location &other): m_filename (other.m_filename),
+    m_function (other.m_function), m_line (other.m_line),
+    m_origin (other.m_origin), m_ggc (other.m_ggc) {}
+
   /* Compute hash value based on file name, function name and line in
      source code. As there is just a single pointer registered for every
      constant that points to e.g. the same file name, we can use hash
@@ -79,11 +84,12 @@ struct mem_location
 struct mem_usage
 {
   /* Default constructor.  */
-  mem_usage (): m_allocated (0), m_times (0), m_peak (0) {}
+  mem_usage (): m_allocated (0), m_times (0), m_peak (0), m_instances (1) {}
 
   /* Constructor.  */
-  mem_usage (size_t allocated, size_t times, size_t peak):
-    m_allocated (allocated), m_times (times), m_peak (peak) {}
+  mem_usage (size_t allocated, size_t times, size_t peak, size_t instances = 0):
+    m_allocated (allocated), m_times (times), m_peak (peak),
+    m_instances (instances) {}
 
   /* Register overhead of SIZE bytes.  */
   inline void register_overhead (size_t size)
@@ -108,7 +114,8 @@ struct mem_usage
   {
     return mem_usage (m_allocated + second.m_allocated,
 		      m_times + second.m_times,
-		      m_peak + second.m_peak);
+		      m_peak + second.m_peak,
+		      m_instances + second.m_instances);
   }
 
   /* Comparison operator.  */
@@ -163,7 +170,7 @@ struct mem_usage
   /* Print line made of dashes.  */
   static inline void print_dash_line ()
   {
-    fprintf (stderr, "%s\n", std::string (128, '-').c_str ());
+    fprintf (stderr, "%s\n", std::string (140, '-').c_str ());
   }
 
   /* Dump header with NAME.  */
@@ -180,6 +187,8 @@ struct mem_usage
   size_t m_times;
   /* Peak allocation in bytes.  */
   size_t m_peak;
+  /* Number of container instances.  */
+  size_t m_instances;
 };
 
 /* Memory usage pair that connectes memory usage and number
@@ -241,9 +250,13 @@ public:
   /* Return descriptor for instance PTR.  */
   T *get_descriptor_for_instance (const void *ptr);
 
-  /* Register memory allocation descriptor for container PTR. ORIGIN identifies
+  /* Register memory allocation descriptor for container PTR which is
+     described by a memory LOCATION.  */
+  T *register_descriptor (const void *ptr, mem_location *location);
+
+  /* Register memory allocation descriptor for container PTR.  ORIGIN identifies
      type of container and GGC identifes if the allocation is handled in GGC
-     memory. Each location is identified by file NAME, LINE in source code and
+     memory.  Each location is identified by file NAME, LINE in source code and
      FUNCTION name.  */
   T *register_descriptor (const void *ptr, mem_alloc_origin origin,
 			  bool ggc, const char *name, int line,
@@ -321,33 +334,27 @@ mem_alloc_description<T>::get_descriptor_for_instance (const void *ptr)
   return m_reverse_map->get (ptr) ? (*m_reverse_map->get (ptr)).usage : NULL;
 }
 
-/* Register memory allocation descriptor for container PTR. ORIGIN identifies
-   type of container and GGC identifes if the allocation is handled in GGC
-   memory. Each location is identified by file NAME, LINE in source code and
-   FUNCTION name.  */
 
+  /* Register memory allocation descriptor for container PTR which is
+     described by a memory LOCATION.  */
 template <class T>
 inline T*
 mem_alloc_description<T>::register_descriptor (const void *ptr,
-					       mem_alloc_origin origin,
-					       bool ggc,
-					       const char *filename,
-					       int line,
-					       const char *function)
+					       mem_location *location)
 {
-  mem_location *l = new mem_location (filename, function, line, origin, ggc);
   T *usage = NULL;
 
-  T **slot = m_map->get (l);
+  T **slot = m_map->get (location);
   if (slot)
     {
-      delete l;
+      delete location;
       usage = *slot;
+      usage->m_instances++;
     }
   else
     {
       usage = new T ();
-      m_map->put (l, usage);
+      m_map->put (location, usage);
     }
 
   if (!m_reverse_map->get (ptr))
@@ -356,6 +363,24 @@ mem_alloc_description<T>::register_descriptor (const void *ptr,
   return usage;
 }
 
+/* Register memory allocation descriptor for container PTR.  ORIGIN identifies
+   type of container and GGC identifes if the allocation is handled in GGC
+   memory.  Each location is identified by file NAME, LINE in source code and
+   FUNCTION name.  */
+
+template <class T>
+inline T*
+mem_alloc_description<T>::register_descriptor (const void *ptr,
+					       mem_alloc_origin origin,
+					       bool ggc,
+					       const char *filename,
+					       int line,
+					       const char *function)
+{
+  mem_location *l = new mem_location (origin, ggc, filename, line, function);
+  return register_descriptor (ptr, l);
+}
+
 /* Register instance overhead identified by PTR pointer. Allocation takes
    SIZE bytes.  */
 
-- 
2.1.4


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

* [PATCH 2/2] Fix memory report layout at various places.
  2015-06-02 11:14 [PATCH 1/2] Memory statistics enhancement mliska
@ 2015-06-02 11:18 ` mliska
  2015-06-02 14:11   ` Jeff Law
  2015-06-02 14:00 ` [PATCH 1/2] Memory statistics enhancement Jeff Law
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: mliska @ 2015-06-02 11:18 UTC (permalink / raw)
  To: gcc-patches

gcc/ChangeLog:

2015-06-02  Martin Liska  <mliska@suse.cz>

	* alloc-pool.h (struct pool_usage): Correct space padding.
	* ggc-page.c (ggc_print_statistics): Align columns in a report.
	* mem-stats.h (struct mem_usage): Add argument to print_dash_line.
	* tree.c (dump_tree_statistics): Align columns in a report.
---
 gcc/alloc-pool.h |  2 +-
 gcc/ggc-page.c   | 46 +++++++++++++++++++++++++---------------------
 gcc/mem-stats.h  |  4 ++--
 gcc/tree.c       | 19 ++++++++++++-------
 4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
index a1727ce..6f1bbed 100644
--- a/gcc/alloc-pool.h
+++ b/gcc/alloc-pool.h
@@ -78,7 +78,7 @@ struct pool_usage: public mem_usage
   inline void dump_footer ()
   {
     print_dash_line ();
-    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
+    fprintf (stderr, "%s%82li%10li\n", "Total", (long)m_instances,
 	     (long)m_allocated);
     print_dash_line ();
   }
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c
index 158156a..7fceeba 100644
--- a/gcc/ggc-page.c
+++ b/gcc/ggc-page.c
@@ -2268,7 +2268,7 @@ ggc_print_statistics (void)
      allocation.  */
   fprintf (stderr,
            "Memory still allocated at the end of the compilation process\n");
-  fprintf (stderr, "%-5s %10s  %10s  %10s\n",
+  fprintf (stderr, "%-8s %10s  %10s  %10s\n",
 	   "Size", "Allocated", "Used", "Overhead");
   for (i = 0; i < NUM_ORDERS; ++i)
     {
@@ -2295,47 +2295,51 @@ ggc_print_statistics (void)
 	  overhead += (sizeof (page_entry) - sizeof (long)
 		       + BITMAP_SIZE (OBJECTS_IN_PAGE (p) + 1));
 	}
-      fprintf (stderr, "%-5lu %10lu%c %10lu%c %10lu%c\n",
+      fprintf (stderr, "%-8lu %10lu%c %10lu%c %10lu%c\n",
 	       (unsigned long) OBJECT_SIZE (i),
 	       SCALE (allocated), STAT_LABEL (allocated),
 	       SCALE (in_use), STAT_LABEL (in_use),
 	       SCALE (overhead), STAT_LABEL (overhead));
       total_overhead += overhead;
     }
-  fprintf (stderr, "%-5s %10lu%c %10lu%c %10lu%c\n", "Total",
+  fprintf (stderr, "%-8s %10lu%c %10lu%c %10lu%c\n", "Total",
 	   SCALE (G.bytes_mapped), STAT_LABEL (G.bytes_mapped),
 	   SCALE (G.allocated), STAT_LABEL (G.allocated),
 	   SCALE (total_overhead), STAT_LABEL (total_overhead));
 
   if (GATHER_STATISTICS)
     {
-      fprintf (stderr, "\nTotal allocations and overheads during the compilation process\n");
+      fprintf (stderr, "\nTotal allocations and overheads during "
+	       "the compilation process\n");
 
-      fprintf (stderr, "Total Overhead:                        %10" HOST_LONG_LONG_FORMAT "d\n",
-	       G.stats.total_overhead);
-      fprintf (stderr, "Total Allocated:                       %10" HOST_LONG_LONG_FORMAT "d\n",
+      fprintf (stderr, "Total Overhead:                          %10"
+	       HOST_LONG_LONG_FORMAT "d\n", G.stats.total_overhead);
+      fprintf (stderr, "Total Allocated:                         %10"
+	       HOST_LONG_LONG_FORMAT "d\n",
 	       G.stats.total_allocated);
 
-      fprintf (stderr, "Total Overhead  under  32B:            %10" HOST_LONG_LONG_FORMAT "d\n",
-	       G.stats.total_overhead_under32);
-      fprintf (stderr, "Total Allocated under  32B:            %10" HOST_LONG_LONG_FORMAT "d\n",
-	       G.stats.total_allocated_under32);
-      fprintf (stderr, "Total Overhead  under  64B:            %10" HOST_LONG_LONG_FORMAT "d\n",
-	       G.stats.total_overhead_under64);
-      fprintf (stderr, "Total Allocated under  64B:            %10" HOST_LONG_LONG_FORMAT "d\n",
-	       G.stats.total_allocated_under64);
-      fprintf (stderr, "Total Overhead  under 128B:            %10" HOST_LONG_LONG_FORMAT "d\n",
-	       G.stats.total_overhead_under128);
-      fprintf (stderr, "Total Allocated under 128B:            %10" HOST_LONG_LONG_FORMAT "d\n",
-	       G.stats.total_allocated_under128);
+      fprintf (stderr, "Total Overhead  under  32B:              %10"
+	       HOST_LONG_LONG_FORMAT "d\n", G.stats.total_overhead_under32);
+      fprintf (stderr, "Total Allocated under  32B:              %10"
+	       HOST_LONG_LONG_FORMAT "d\n", G.stats.total_allocated_under32);
+      fprintf (stderr, "Total Overhead  under  64B:              %10"
+	       HOST_LONG_LONG_FORMAT "d\n", G.stats.total_overhead_under64);
+      fprintf (stderr, "Total Allocated under  64B:              %10"
+	       HOST_LONG_LONG_FORMAT "d\n", G.stats.total_allocated_under64);
+      fprintf (stderr, "Total Overhead  under 128B:              %10"
+	       HOST_LONG_LONG_FORMAT "d\n", G.stats.total_overhead_under128);
+      fprintf (stderr, "Total Allocated under 128B:              %10"
+	       HOST_LONG_LONG_FORMAT "d\n", G.stats.total_allocated_under128);
 
       for (i = 0; i < NUM_ORDERS; i++)
 	if (G.stats.total_allocated_per_order[i])
 	  {
-	    fprintf (stderr, "Total Overhead  page size %7lu:     %10" HOST_LONG_LONG_FORMAT "d\n",
+	    fprintf (stderr, "Total Overhead  page size %9lu:     %10"
+		     HOST_LONG_LONG_FORMAT "d\n",
 		     (unsigned long) OBJECT_SIZE (i),
 		     G.stats.total_overhead_per_order[i]);
-	    fprintf (stderr, "Total Allocated page size %7lu:     %10" HOST_LONG_LONG_FORMAT "d\n",
+	    fprintf (stderr, "Total Allocated page size %9lu:     %10"
+		     HOST_LONG_LONG_FORMAT "d\n",
 		     (unsigned long) OBJECT_SIZE (i),
 		     G.stats.total_allocated_per_order[i]);
 	  }
diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index fea68fa..9914425 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -168,9 +168,9 @@ struct mem_usage
   }
 
   /* Print line made of dashes.  */
-  static inline void print_dash_line ()
+  static inline void print_dash_line (size_t count = 140)
   {
-    fprintf (stderr, "%s\n", std::string (140, '-').c_str ());
+    fprintf (stderr, "%s\n", std::string (count, '-').c_str ());
   }
 
   /* Dump header with NAME.  */
diff --git a/gcc/tree.c b/gcc/tree.c
index ca48c60..441ab3a 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -9125,6 +9125,8 @@ get_callee_fndecl (const_tree call)
   return NULL_TREE;
 }
 
+#define TREE_MEM_USAGE_SPACES 40
+
 /* Print debugging information about tree nodes generated during the compile,
    and any language-specific information.  */
 
@@ -9135,8 +9137,8 @@ dump_tree_statistics (void)
     {
       int i;
       int total_nodes, total_bytes;
-      fprintf (stderr, "Kind                   Nodes      Bytes\n");
-      fprintf (stderr, "---------------------------------------\n");
+      fprintf (stderr, "\nKind                   Nodes      Bytes\n");
+      mem_usage::print_dash_line (TREE_MEM_USAGE_SPACES);
       total_nodes = total_bytes = 0;
       for (i = 0; i < (int) all_kinds; i++)
 	{
@@ -9145,17 +9147,20 @@ dump_tree_statistics (void)
 	  total_nodes += tree_node_counts[i];
 	  total_bytes += tree_node_sizes[i];
 	}
-      fprintf (stderr, "---------------------------------------\n");
+      mem_usage::print_dash_line (TREE_MEM_USAGE_SPACES);
       fprintf (stderr, "%-20s %7d %10d\n", "Total", total_nodes, total_bytes);
-      fprintf (stderr, "---------------------------------------\n");
+      mem_usage::print_dash_line (TREE_MEM_USAGE_SPACES);
       fprintf (stderr, "Code                   Nodes\n");
-      fprintf (stderr, "----------------------------\n");
+      mem_usage::print_dash_line (TREE_MEM_USAGE_SPACES);
       for (i = 0; i < (int) MAX_TREE_CODES; i++)
-	fprintf (stderr, "%-20s %7d\n", get_tree_code_name ((enum tree_code) i),
+	fprintf (stderr, "%-32s %7d\n", get_tree_code_name ((enum tree_code) i),
                  tree_code_counts[i]);
-      fprintf (stderr, "----------------------------\n");
+      mem_usage::print_dash_line (TREE_MEM_USAGE_SPACES);
+      fprintf (stderr, "\n");
       ssanames_print_statistics ();
+      fprintf (stderr, "\n");
       phinodes_print_statistics ();
+      fprintf (stderr, "\n");
     }
   else
     fprintf (stderr, "(No per-node statistics)\n");
-- 
2.1.4

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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-02 11:14 [PATCH 1/2] Memory statistics enhancement mliska
  2015-06-02 11:18 ` [PATCH 2/2] Fix memory report layout at various places mliska
@ 2015-06-02 14:00 ` Jeff Law
  2015-06-02 15:38   ` Martin Liška
  2015-06-03  9:47 ` Martin Liška
  2015-06-08 15:02 ` Martin Liška
  3 siblings, 1 reply; 14+ messages in thread
From: Jeff Law @ 2015-06-02 14:00 UTC (permalink / raw)
  To: mliska, gcc-patches

On 06/01/2015 10:16 AM, mliska wrote:
> Hi.
>
> Following 2 patches improve memory statistics infrastructure. First one
> ports pool allocator to the new infrastructure. And the second one makes
> column alignment properly.
>
> Both can bootstrap on x86_64-linux-pc and survive regression tests.
>
> Ready for trunk?
> Thank you,
> Martin
>
> Port pool-allocator memory stats to a new infrastructure.
>
> gcc/ChangeLog:
>
> 2015-06-02  Martin Liska  <mliska@suse.cz>
>
> 	* alloc-pool.c (allocate_pool_descriptor): Remove.
> 	(struct pool_output_info): Likewise.
> 	(print_alloc_pool_statistics): Likewise.
> 	(dump_alloc_pool_statistics): Likewise.
> 	* alloc-pool.h (struct pool_usage): New struct.
> 	(pool_allocator::initialize): Change usage of memory statistics
> 	to a new interface.
> 	(pool_allocator::release): Likewise.
> 	(pool_allocator::allocate): Likewise.
> 	(pool_allocator::remove): Likewise.
> 	* mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
> 	for a pool allocator.
> 	* mem-stats.h (struct mem_location): Add new ctor.
> 	(struct mem_usage): Add counter for number of
> 	instances.
> 	(mem_alloc_description::register_descriptor): New overload of
> 	the function.
  -

> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
> index 96a1342..a1727ce 100644
> --- a/gcc/alloc-pool.h
> +++ b/gcc/alloc-pool.h

> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
> +  inline void dump (mem_location *loc, mem_usage &total) const
> +  {
> +    char s[4096];
> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
> +	     loc->m_line, loc->m_function);
Static sized buffer used in a sprintf where the strings are potentially 
user controlled.   Not good, even in dumping code, still not good.

> +
> +    s[48] = '\0';
?!?  Presumably you're just truncating the output line here for the 
subsequent fprintf call.  Consider using a const with a symbolic name 
rather than the magic "48".  I say "consider" because there's magic 
constants all over the place in the dumping code. So it may not be worth 
the effort.  Your call.

  +
> +  /* Dump header with NAME.  */
> +  static inline void dump_header (const char *name)
> +  {
> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
> +	     "Pools", "Leak", "Peak", "Times", "Elt size");
> +    print_dash_line ();
> +  }
> +
> +  /* Dump footer.  */
> +  inline void dump_footer ()
> +  {
> +    print_dash_line ();
> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
> +	     (long)m_allocated);
> +    print_dash_line ();
> +  }
Note the header is static inline, footer is just inline.  Please try to 
make them consistent.

  @@ -235,10 +301,10 @@ pool_allocator<T>::release ()
>         free (block);
>       }
>
> -  if (GATHER_STATISTICS && false)
> +  if (GATHER_STATISTICS)
>       {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -      desc->current -= (m_elts_allocated - m_elts_free) * m_elt_size;
> +      pool_allocator_usage.release_instance_overhead (this,
> +	(m_elts_allocated - m_elts_free) * m_elt_size);
Looks like line wrapping needs to be fixed.


Clearly the biggest issue is that static sized buffer used to hold the 
results of sprintf...  Once that and the smaller issues are fixed, this 
is OK.

jeff

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

* Re: [PATCH 2/2] Fix memory report layout at various places.
  2015-06-02 11:18 ` [PATCH 2/2] Fix memory report layout at various places mliska
@ 2015-06-02 14:11   ` Jeff Law
  0 siblings, 0 replies; 14+ messages in thread
From: Jeff Law @ 2015-06-02 14:11 UTC (permalink / raw)
  To: mliska, gcc-patches

On 06/01/2015 10:36 AM, mliska wrote:
> gcc/ChangeLog:
>
> 2015-06-02  Martin Liska  <mliska@suse.cz>
>
> 	* alloc-pool.h (struct pool_usage): Correct space padding.
> 	* ggc-page.c (ggc_print_statistics): Align columns in a report.
> 	* mem-stats.h (struct mem_usage): Add argument to print_dash_line.
> 	* tree.c (dump_tree_statistics): Align columns in a report.
OK.  I'm inclined to say that subsequent patches that do similar things 
should be considered pre-approved.

Jeff

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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-02 14:00 ` [PATCH 1/2] Memory statistics enhancement Jeff Law
@ 2015-06-02 15:38   ` Martin Liška
  2015-06-02 17:33     ` Jeff Law
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2015-06-02 15:38 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

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

On 06/02/2015 03:58 PM, Jeff Law wrote:
> On 06/01/2015 10:16 AM, mliska wrote:
>> Hi.
>>
>> Following 2 patches improve memory statistics infrastructure. First one
>> ports pool allocator to the new infrastructure. And the second one makes
>> column alignment properly.
>>
>> Both can bootstrap on x86_64-linux-pc and survive regression tests.
>>
>> Ready for trunk?
>> Thank you,
>> Martin
>>
>> Port pool-allocator memory stats to a new infrastructure.
>>
>> gcc/ChangeLog:
>>
>> 2015-06-02  Martin Liska  <mliska@suse.cz>
>>
>>     * alloc-pool.c (allocate_pool_descriptor): Remove.
>>     (struct pool_output_info): Likewise.
>>     (print_alloc_pool_statistics): Likewise.
>>     (dump_alloc_pool_statistics): Likewise.
>>     * alloc-pool.h (struct pool_usage): New struct.
>>     (pool_allocator::initialize): Change usage of memory statistics
>>     to a new interface.
>>     (pool_allocator::release): Likewise.
>>     (pool_allocator::allocate): Likewise.
>>     (pool_allocator::remove): Likewise.
>>     * mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
>>     for a pool allocator.
>>     * mem-stats.h (struct mem_location): Add new ctor.
>>     (struct mem_usage): Add counter for number of
>>     instances.
>>     (mem_alloc_description::register_descriptor): New overload of
>>     the function.
>  -
> 
>> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
>> index 96a1342..a1727ce 100644
>> --- a/gcc/alloc-pool.h
>> +++ b/gcc/alloc-pool.h
> 
>> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
>> +  inline void dump (mem_location *loc, mem_usage &total) const
>> +  {
>> +    char s[4096];
>> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
>> +         loc->m_line, loc->m_function);
> Static sized buffer used in a sprintf where the strings are potentially user controlled.   Not good, even in dumping code, still not good.
> 
>> +
>> +    s[48] = '\0';
> ?!?  Presumably you're just truncating the output line here for the subsequent fprintf call.  Consider using a const with a symbolic name rather than the magic "48".  I say "consider" because there's magic constants all over the place in the dumping code. So it may not be worth the effort.  Your call.
> 
>  +
>> +  /* Dump header with NAME.  */
>> +  static inline void dump_header (const char *name)
>> +  {
>> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
>> +         "Pools", "Leak", "Peak", "Times", "Elt size");
>> +    print_dash_line ();
>> +  }
>> +
>> +  /* Dump footer.  */
>> +  inline void dump_footer ()
>> +  {
>> +    print_dash_line ();
>> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
>> +         (long)m_allocated);
>> +    print_dash_line ();
>> +  }
> Note the header is static inline, footer is just inline.  Please try to make them consistent.
> 
>  @@ -235,10 +301,10 @@ pool_allocator<T>::release ()
>>         free (block);
>>       }
>>
>> -  if (GATHER_STATISTICS && false)
>> +  if (GATHER_STATISTICS)
>>       {
>> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
>> -      desc->current -= (m_elts_allocated - m_elts_free) * m_elt_size;
>> +      pool_allocator_usage.release_instance_overhead (this,
>> +    (m_elts_allocated - m_elts_free) * m_elt_size);
> Looks like line wrapping needs to be fixed.
> 
> 
> Clearly the biggest issue is that static sized buffer used to hold the results of sprintf...  Once that and the smaller issues are fixed, this is OK.
> 
> jeff
> 

Hi Jeff.

This patch improves the patch in suggested way. I verified it by valgrind that the allocated string buffer
is used correctly.

Ready for trunk?
Thanks,
Martin

[-- Attachment #2: 0001-Port-pool-allocator-memory-stats-to-a-new-infrastruc.patch --]
[-- Type: text/x-patch, Size: 19003 bytes --]

From 3373344b681f343ebe038e4c75abc8fe4c39ebea Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Mon, 1 Jun 2015 18:16:52 +0200
Subject: [PATCH 1/2] Port pool-allocator memory stats to a new infrastructure.

gcc/ChangeLog:

2015-06-02  Martin Liska  <mliska@suse.cz>

	* alloc-pool.c (allocate_pool_descriptor): Remove.
	(struct pool_output_info): Likewise.
	(print_alloc_pool_statistics): Likewise.
	(dump_alloc_pool_statistics): Likewise.
	* alloc-pool.h (struct pool_usage): New struct.
	(pool_allocator::initialize): Change usage of memory statistics
	to a new interface.
	(pool_allocator::release): Likewise.
	(pool_allocator::allocate): Likewise.
	(pool_allocator::remove): Likewise.
	* mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
	for a pool allocator.
	* mem-stats.h (struct mem_location): Add new ctor.
	(struct mem_usage): Add counter for number of
	instances.
	(mem_alloc_description::register_descriptor): New overload of
	* mem-stats.h (mem_location::to_string): New function.
	* bitmap.h (struct bitmap_usage): Use this new function.
	* ggc-common.c (struct ggc_usage): Likewise.
	the function.
---
 gcc/alloc-pool.c       |  60 +----------------------------
 gcc/alloc-pool.h       | 100 ++++++++++++++++++++++++++++++++++++++-----------
 gcc/bitmap.h           |  11 +++---
 gcc/ggc-common.c       |   9 ++---
 gcc/mem-stats-traits.h |   3 +-
 gcc/mem-stats.h        | 100 +++++++++++++++++++++++++++++++++++--------------
 6 files changed, 163 insertions(+), 120 deletions(-)

diff --git a/gcc/alloc-pool.c b/gcc/alloc-pool.c
index e9fdc86..601c2b7 100644
--- a/gcc/alloc-pool.c
+++ b/gcc/alloc-pool.c
@@ -26,70 +26,14 @@ along with GCC; see the file COPYING3.  If not see
 #include "hash-map.h"
 
 ALLOC_POOL_ID_TYPE last_id;
-
-/* Hashtable mapping alloc_pool names to descriptors.  */
-hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
-
-struct alloc_pool_descriptor *
-allocate_pool_descriptor (const char *name)
-{
-  if (!alloc_pool_hash)
-    alloc_pool_hash = new hash_map<const char *, alloc_pool_descriptor> (10,
-									 false,
-									 false);
-
-  return &alloc_pool_hash->get_or_insert (name);
-}
-
-/* Output per-alloc_pool statistics.  */
-
-/* Used to accumulate statistics about alloc_pool sizes.  */
-struct pool_output_info
-{
-  unsigned long total_created;
-  unsigned long total_allocated;
-};
-
-/* Called via hash_map.traverse.  Output alloc_pool descriptor pointed out by
-   SLOT and update statistics.  */
-bool
-print_alloc_pool_statistics (const char *const &name,
-			     const alloc_pool_descriptor &d,
-			     struct pool_output_info *i)
-{
-  if (d.allocated)
-    {
-      fprintf (stderr,
-	       "%-22s %6d %10lu %10lu(%10lu) %10lu(%10lu) %10lu(%10lu)\n",
-	       name, d.elt_size, d.created, d.allocated,
-	       d.allocated / d.elt_size, d.peak, d.peak / d.elt_size,
-	       d.current, d.current / d.elt_size);
-      i->total_allocated += d.allocated;
-      i->total_created += d.created;
-    }
-  return 1;
-}
+mem_alloc_description<pool_usage> pool_allocator_usage;
 
 /* Output per-alloc_pool memory usage statistics.  */
 void
 dump_alloc_pool_statistics (void)
 {
-  struct pool_output_info info;
-
   if (! GATHER_STATISTICS)
     return;
 
-  if (!alloc_pool_hash)
-    return;
-
-  fprintf (stderr, "\nAlloc-pool Kind         Elt size  Pools  Allocated (elts)            Peak (elts)            Leak (elts)\n");
-  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
-  info.total_created = 0;
-  info.total_allocated = 0;
-  alloc_pool_hash->traverse <struct pool_output_info *,
-			     print_alloc_pool_statistics> (&info);
-  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
-  fprintf (stderr, "%-22s           %7lu %10lu\n",
-	   "Total", info.total_created, info.total_allocated);
-  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
+  pool_allocator_usage.dump (ALLOC_POOL);
 }
diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
index 96a1342..895e12f 100644
--- a/gcc/alloc-pool.h
+++ b/gcc/alloc-pool.h
@@ -26,6 +26,69 @@ extern void dump_alloc_pool_statistics (void);
 
 typedef unsigned long ALLOC_POOL_ID_TYPE;
 
+/* Pool allocator memory usage.  */
+struct pool_usage: public mem_usage
+{
+  /* Default contructor.  */
+  pool_usage (): m_element_size (0), m_pool_name ("") {}
+  /* Constructor.  */
+  pool_usage (size_t allocated, size_t times, size_t peak,
+	      size_t instances, size_t element_size,
+	      const char *pool_name)
+    : mem_usage (allocated, times, peak, instances),
+      m_element_size (element_size),
+      m_pool_name (pool_name) {}
+
+  /* Sum the usage with SECOND usage.  */
+  pool_usage operator+ (const pool_usage &second)
+  {
+    return pool_usage (m_allocated + second.m_allocated,
+			     m_times + second.m_times,
+			     m_peak + second.m_peak,
+			     m_instances + second.m_instances,
+			     m_element_size, m_pool_name);
+  }
+
+  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
+  inline void dump (mem_location *loc, mem_usage &total) const
+  {
+    char *location_string = loc->to_string ();
+
+    fprintf (stderr, "%-32s%-48s %6li%10li:%5.1f%%%10li%10li:%5.1f%%%12li\n",
+	     m_pool_name, location_string, (long)m_instances,
+	     (long)m_allocated, get_percent (m_allocated, total.m_allocated),
+	     (long)m_peak, (long)m_times,
+	     get_percent (m_times, total.m_times),
+	     (long)m_element_size);
+
+    free (location_string);
+  }
+
+  /* Dump header with NAME.  */
+  static inline void dump_header (const char *name)
+  {
+    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
+	     "Pools", "Leak", "Peak", "Times", "Elt size");
+    print_dash_line ();
+  }
+
+  /* Dump footer.  */
+  inline void dump_footer ()
+  {
+    print_dash_line ();
+    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
+	     (long)m_allocated);
+    print_dash_line ();
+  }
+
+  /* Element size.  */
+  size_t m_element_size;
+  /* Pool name.  */
+  const char *m_pool_name;
+};
+
+extern mem_alloc_description<pool_usage> pool_allocator_usage;
+
 /* Type based memory pool allocator.  */
 template <typename T>
 class pool_allocator
@@ -35,7 +98,7 @@ public:
      has NUM elements.  The allocator support EXTRA_SIZE and can
      potentially IGNORE_TYPE_SIZE.  */
   pool_allocator (const char *name, size_t num, size_t extra_size = 0,
-		  bool ignore_type_size = false);
+		  bool ignore_type_size = false CXX_MEM_STAT_INFO);
   ~pool_allocator ();
   void release ();
   void release_if_empty ();
@@ -122,6 +185,8 @@ private:
   size_t m_extra_size;
   /* Flag if a pool allocator is initialized.  */
   bool m_initialized;
+  /* Memory allocation location.  */
+  mem_location m_location;
 };
 
 /* Last used ID.  */
@@ -151,19 +216,16 @@ struct alloc_pool_descriptor
 /* Hashtable mapping alloc_pool names to descriptors.  */
 extern hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
 
-/* For given name, return descriptor, create new if needed.  */
-alloc_pool_descriptor *
-allocate_pool_descriptor (const char *name);
-
 template <typename T>
 inline
 pool_allocator<T>::pool_allocator (const char *name, size_t num,
-				   size_t extra_size, bool ignore_type_size):
+				   size_t extra_size, bool ignore_type_size
+				   MEM_STAT_DECL):
   m_name (name), m_elts_per_block (num), m_returned_free_list (NULL),
   m_virgin_free_list (NULL), m_virgin_elts_remaining (0), m_elts_allocated (0),
   m_elts_free (0), m_blocks_allocated (0), m_block_list (NULL),
   m_ignore_type_size (ignore_type_size), m_extra_size (extra_size),
-  m_initialized (false) {}
+  m_initialized (false), m_location (ALLOC_POOL, false PASS_MEM_STAT) {}
 
 /* Initialize a pool allocator.  */
 
@@ -196,9 +258,11 @@ pool_allocator<T>::initialize ()
 
   if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-      desc->elt_size = size;
-      desc->created++;
+      pool_usage *u = pool_allocator_usage.register_descriptor
+	(this, new mem_location (m_location));
+
+      u->m_element_size = m_elt_size;
+      u->m_pool_name = m_name;
     }
 
   /* List header size should be a multiple of 8.  */
@@ -235,10 +299,10 @@ pool_allocator<T>::release ()
       free (block);
     }
 
-  if (GATHER_STATISTICS && false)
+  if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-      desc->current -= (m_elts_allocated - m_elts_free) * m_elt_size;
+      pool_allocator_usage.release_instance_overhead
+	(this, (m_elts_allocated - m_elts_free) * m_elt_size);
     }
 
   m_returned_free_list = NULL;
@@ -279,12 +343,7 @@ pool_allocator<T>::allocate ()
 
   if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-
-      desc->allocated += m_elt_size;
-      desc->current += m_elt_size;
-      if (desc->peak < desc->current)
-	desc->peak = desc->current;
+      pool_allocator_usage.register_instance_overhead (m_elt_size, this);
     }
 
 #ifdef ENABLE_VALGRIND_ANNOTATIONS
@@ -383,8 +442,7 @@ pool_allocator<T>::remove (T *object)
 
   if (GATHER_STATISTICS)
     {
-      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
-      desc->current -= m_elt_size;
+      pool_allocator_usage.release_instance_overhead (this, m_elt_size);
     }
 }
 
diff --git a/gcc/bitmap.h b/gcc/bitmap.h
index 40562f6..4309f6d 100644
--- a/gcc/bitmap.h
+++ b/gcc/bitmap.h
@@ -156,18 +156,17 @@ struct bitmap_usage: public mem_usage
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
   inline void dump (mem_location *loc, mem_usage &total) const
   {
-    char s[4096];
-    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
-	     loc->m_line, loc->m_function);
+    char *location_string = loc->to_string ();
 
-    s[48] = '\0';
-
-    fprintf (stderr, "%-48s %10li:%5.1f%%%10li%10li:%5.1f%%%12li%12li%10s\n", s,
+    fprintf (stderr, "%-48s %10li:%5.1f%%%10li%10li:%5.1f%%%12li%12li%10s\n",
+	     location_string,
 	     (long)m_allocated, get_percent (m_allocated, total.m_allocated),
 	     (long)m_peak, (long)m_times,
 	     get_percent (m_times, total.m_times),
 	     (long)m_nsearches, (long)m_search_iter,
 	     loc->m_ggc ? "ggc" : "heap");
+
+    free (location_string);
   }
 
   /* Dump header with NAME.  */
diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index 2e94ca4..43ccc0b 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -894,12 +894,11 @@ struct ggc_usage: public mem_usage
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
   inline void dump (mem_location *loc, ggc_usage &total) const
   {
-    char s[4096];
-    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
-	     loc->m_line, loc->m_function);
-    s[48] = '\0';
+    char *location_string = loc->to_string ();
 
-    dump (s, total);
+    dump (location_string, total);
+
+    free (location_string);
   }
 
   /* Dump footer.  */
diff --git a/gcc/mem-stats-traits.h b/gcc/mem-stats-traits.h
index de1614e..f7843f2 100644
--- a/gcc/mem-stats-traits.h
+++ b/gcc/mem-stats-traits.h
@@ -10,11 +10,12 @@ enum mem_alloc_origin
   VEC,
   BITMAP,
   GGC,
+  ALLOC_POOL,
   MEM_ALLOC_ORIGIN_LENGTH
 };
 
 /* Verbose names of the memory allocation origin.  */
 static const char * mem_alloc_origin_names[] = { "Hash tables", "Hash maps",
-  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory" };
+  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory", "Allocation pool" };
 
 #endif // GCC_MEM_STATS_TRAITS_H
diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index ac47231..092bfd3 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -11,6 +11,9 @@ template<typename Key, typename Value,
 	 typename Traits = default_hashmap_traits>
 class hash_map;
 
+#define LOCATION_LINE_EXTRA_SPACE 30
+#define LOCATION_LINE_WIDTH	  48
+
 /* Memory allocation location.  */
 struct mem_location
 {
@@ -18,11 +21,17 @@ struct mem_location
   inline mem_location () {}
 
   /* Constructor.  */
-  inline mem_location (const char *filename, const char *function, int line,
-		mem_alloc_origin origin, bool ggc):
+  inline mem_location (mem_alloc_origin origin, bool ggc,
+		const char *filename = NULL, int line = 0,
+		const char *function = NULL):
     m_filename (filename), m_function (function), m_line (line), m_origin
     (origin), m_ggc (ggc) {}
 
+  /* Copy constructor.  */
+  inline mem_location (mem_location &other): m_filename (other.m_filename),
+    m_function (other.m_function), m_line (other.m_line),
+    m_origin (other.m_origin), m_ggc (other.m_ggc) {}
+
   /* Compute hash value based on file name, function name and line in
      source code. As there is just a single pointer registered for every
      constant that points to e.g. the same file name, we can use hash
@@ -57,6 +66,20 @@ struct mem_location
     return s1;
   }
 
+  inline char *to_string ()
+  {
+    unsigned l = strlen (get_trimmed_filename ()) + strlen (m_function)
+      + LOCATION_LINE_EXTRA_SPACE;
+
+    char *s = XNEWVEC (char, l);
+    sprintf (s, "%s:%i (%s)", get_trimmed_filename (),
+	     m_line, m_function);
+
+    s[MIN (LOCATION_LINE_WIDTH, l - 1)] = '\0';
+
+    return s;
+  }
+
   /* Return display name associated to ORIGIN type.  */
   static const char *get_origin_name (mem_alloc_origin origin)
   {
@@ -79,11 +102,12 @@ struct mem_location
 struct mem_usage
 {
   /* Default constructor.  */
-  mem_usage (): m_allocated (0), m_times (0), m_peak (0) {}
+  mem_usage (): m_allocated (0), m_times (0), m_peak (0), m_instances (1) {}
 
   /* Constructor.  */
-  mem_usage (size_t allocated, size_t times, size_t peak):
-    m_allocated (allocated), m_times (times), m_peak (peak) {}
+  mem_usage (size_t allocated, size_t times, size_t peak, size_t instances = 0):
+    m_allocated (allocated), m_times (times), m_peak (peak),
+    m_instances (instances) {}
 
   /* Register overhead of SIZE bytes.  */
   inline void register_overhead (size_t size)
@@ -108,7 +132,8 @@ struct mem_usage
   {
     return mem_usage (m_allocated + second.m_allocated,
 		      m_times + second.m_times,
-		      m_peak + second.m_peak);
+		      m_peak + second.m_peak,
+		      m_instances + second.m_instances);
   }
 
   /* Comparison operator.  */
@@ -133,20 +158,19 @@ struct mem_usage
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
   inline void dump (mem_location *loc, mem_usage &total) const
   {
-    char s[4096];
-    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
-	     loc->m_line, loc->m_function);
+    char *location_string = loc->to_string ();
 
-    s[48] = '\0';
-
-    fprintf (stderr, "%-48s %10li:%5.1f%%%10li%10li:%5.1f%%%10s\n", s,
+    fprintf (stderr, "%-48s %10li:%5.1f%%%10li%10li:%5.1f%%%10s\n",
+	     location_string,
 	     (long)m_allocated, get_percent (m_allocated, total.m_allocated),
 	     (long)m_peak, (long)m_times,
 	     get_percent (m_times, total.m_times), loc->m_ggc ? "ggc" : "heap");
+
+    free (location_string);
   }
 
   /* Dump footer.  */
-  inline void dump_footer ()
+  inline void dump_footer () const
   {
     print_dash_line ();
     fprintf (stderr, "%s%54li%27li\n", "Total", (long)m_allocated,
@@ -163,7 +187,7 @@ struct mem_usage
   /* Print line made of dashes.  */
   static inline void print_dash_line ()
   {
-    fprintf (stderr, "%s\n", std::string (128, '-').c_str ());
+    fprintf (stderr, "%s\n", std::string (140, '-').c_str ());
   }
 
   /* Dump header with NAME.  */
@@ -180,6 +204,8 @@ struct mem_usage
   size_t m_times;
   /* Peak allocation in bytes.  */
   size_t m_peak;
+  /* Number of container instances.  */
+  size_t m_instances;
 };
 
 /* Memory usage pair that connectes memory usage and number
@@ -241,9 +267,13 @@ public:
   /* Return descriptor for instance PTR.  */
   T *get_descriptor_for_instance (const void *ptr);
 
-  /* Register memory allocation descriptor for container PTR. ORIGIN identifies
+  /* Register memory allocation descriptor for container PTR which is
+     described by a memory LOCATION.  */
+  T *register_descriptor (const void *ptr, mem_location *location);
+
+  /* Register memory allocation descriptor for container PTR.  ORIGIN identifies
      type of container and GGC identifes if the allocation is handled in GGC
-     memory. Each location is identified by file NAME, LINE in source code and
+     memory.  Each location is identified by file NAME, LINE in source code and
      FUNCTION name.  */
   T *register_descriptor (const void *ptr, mem_alloc_origin origin,
 			  bool ggc, const char *name, int line,
@@ -321,33 +351,27 @@ mem_alloc_description<T>::get_descriptor_for_instance (const void *ptr)
   return m_reverse_map->get (ptr) ? (*m_reverse_map->get (ptr)).usage : NULL;
 }
 
-/* Register memory allocation descriptor for container PTR. ORIGIN identifies
-   type of container and GGC identifes if the allocation is handled in GGC
-   memory. Each location is identified by file NAME, LINE in source code and
-   FUNCTION name.  */
 
+  /* Register memory allocation descriptor for container PTR which is
+     described by a memory LOCATION.  */
 template <class T>
 inline T*
 mem_alloc_description<T>::register_descriptor (const void *ptr,
-					       mem_alloc_origin origin,
-					       bool ggc,
-					       const char *filename,
-					       int line,
-					       const char *function)
+					       mem_location *location)
 {
-  mem_location *l = new mem_location (filename, function, line, origin, ggc);
   T *usage = NULL;
 
-  T **slot = m_map->get (l);
+  T **slot = m_map->get (location);
   if (slot)
     {
-      delete l;
+      delete location;
       usage = *slot;
+      usage->m_instances++;
     }
   else
     {
       usage = new T ();
-      m_map->put (l, usage);
+      m_map->put (location, usage);
     }
 
   if (!m_reverse_map->get (ptr))
@@ -356,6 +380,24 @@ mem_alloc_description<T>::register_descriptor (const void *ptr,
   return usage;
 }
 
+/* Register memory allocation descriptor for container PTR.  ORIGIN identifies
+   type of container and GGC identifes if the allocation is handled in GGC
+   memory.  Each location is identified by file NAME, LINE in source code and
+   FUNCTION name.  */
+
+template <class T>
+inline T*
+mem_alloc_description<T>::register_descriptor (const void *ptr,
+					       mem_alloc_origin origin,
+					       bool ggc,
+					       const char *filename,
+					       int line,
+					       const char *function)
+{
+  mem_location *l = new mem_location (origin, ggc, filename, line, function);
+  return register_descriptor (ptr, l);
+}
+
 /* Register instance overhead identified by PTR pointer. Allocation takes
    SIZE bytes.  */
 
-- 
2.1.4


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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-02 15:38   ` Martin Liška
@ 2015-06-02 17:33     ` Jeff Law
  2015-06-03  9:16       ` Martin Liška
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff Law @ 2015-06-02 17:33 UTC (permalink / raw)
  To: Martin Liška, gcc-patches

On 06/02/2015 09:05 AM, Martin Liška wrote:
> On 06/02/2015 03:58 PM, Jeff Law wrote:
>> On 06/01/2015 10:16 AM, mliska wrote:
>>> Hi.
>>>
>>> Following 2 patches improve memory statistics infrastructure. First one
>>> ports pool allocator to the new infrastructure. And the second one makes
>>> column alignment properly.
>>>
>>> Both can bootstrap on x86_64-linux-pc and survive regression tests.
>>>
>>> Ready for trunk?
>>> Thank you,
>>> Martin
>>>
>>> Port pool-allocator memory stats to a new infrastructure.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-06-02  Martin Liska  <mliska@suse.cz>
>>>
>>>      * alloc-pool.c (allocate_pool_descriptor): Remove.
>>>      (struct pool_output_info): Likewise.
>>>      (print_alloc_pool_statistics): Likewise.
>>>      (dump_alloc_pool_statistics): Likewise.
>>>      * alloc-pool.h (struct pool_usage): New struct.
>>>      (pool_allocator::initialize): Change usage of memory statistics
>>>      to a new interface.
>>>      (pool_allocator::release): Likewise.
>>>      (pool_allocator::allocate): Likewise.
>>>      (pool_allocator::remove): Likewise.
>>>      * mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
>>>      for a pool allocator.
>>>      * mem-stats.h (struct mem_location): Add new ctor.
>>>      (struct mem_usage): Add counter for number of
>>>      instances.
>>>      (mem_alloc_description::register_descriptor): New overload of
>>>      the function.
>>   -
>>
>>> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
>>> index 96a1342..a1727ce 100644
>>> --- a/gcc/alloc-pool.h
>>> +++ b/gcc/alloc-pool.h
>>
>>> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
>>> +  inline void dump (mem_location *loc, mem_usage &total) const
>>> +  {
>>> +    char s[4096];
>>> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
>>> +         loc->m_line, loc->m_function);
>> Static sized buffer used in a sprintf where the strings are potentially user controlled.   Not good, even in dumping code, still not good.
>>
>>> +
>>> +    s[48] = '\0';
>> ?!?  Presumably you're just truncating the output line here for the subsequent fprintf call.  Consider using a const with a symbolic name rather than the magic "48".  I say "consider" because there's magic constants all over the place in the dumping code. So it may not be worth the effort.  Your call.
>>
>>   +
>>> +  /* Dump header with NAME.  */
>>> +  static inline void dump_header (const char *name)
>>> +  {
>>> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
>>> +         "Pools", "Leak", "Peak", "Times", "Elt size");
>>> +    print_dash_line ();
>>> +  }
>>> +
>>> +  /* Dump footer.  */
>>> +  inline void dump_footer ()
>>> +  {
>>> +    print_dash_line ();
>>> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
>>> +         (long)m_allocated);
>>> +    print_dash_line ();
>>> +  }
>> Note the header is static inline, footer is just inline.  Please try to make them consistent.
It doesn't look like you did anything with this.  Is there a reason that 
the dump_header and dump_footer have different linkage?  Also the 
linkage/return type for dump_header should be on its own line.

With that fixed, this is OK for the trunk.

jeff

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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-02 17:33     ` Jeff Law
@ 2015-06-03  9:16       ` Martin Liška
  2015-06-03 13:19         ` Jeff Law
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2015-06-03  9:16 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

On 06/02/2015 07:17 PM, Jeff Law wrote:
> On 06/02/2015 09:05 AM, Martin Liška wrote:
>> On 06/02/2015 03:58 PM, Jeff Law wrote:
>>> On 06/01/2015 10:16 AM, mliska wrote:
>>>> Hi.
>>>>
>>>> Following 2 patches improve memory statistics infrastructure. First one
>>>> ports pool allocator to the new infrastructure. And the second one makes
>>>> column alignment properly.
>>>>
>>>> Both can bootstrap on x86_64-linux-pc and survive regression tests.
>>>>
>>>> Ready for trunk?
>>>> Thank you,
>>>> Martin
>>>>
>>>> Port pool-allocator memory stats to a new infrastructure.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2015-06-02  Martin Liska  <mliska@suse.cz>
>>>>
>>>>      * alloc-pool.c (allocate_pool_descriptor): Remove.
>>>>      (struct pool_output_info): Likewise.
>>>>      (print_alloc_pool_statistics): Likewise.
>>>>      (dump_alloc_pool_statistics): Likewise.
>>>>      * alloc-pool.h (struct pool_usage): New struct.
>>>>      (pool_allocator::initialize): Change usage of memory statistics
>>>>      to a new interface.
>>>>      (pool_allocator::release): Likewise.
>>>>      (pool_allocator::allocate): Likewise.
>>>>      (pool_allocator::remove): Likewise.
>>>>      * mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
>>>>      for a pool allocator.
>>>>      * mem-stats.h (struct mem_location): Add new ctor.
>>>>      (struct mem_usage): Add counter for number of
>>>>      instances.
>>>>      (mem_alloc_description::register_descriptor): New overload of
>>>>      the function.
>>>   -
>>>
>>>> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
>>>> index 96a1342..a1727ce 100644
>>>> --- a/gcc/alloc-pool.h
>>>> +++ b/gcc/alloc-pool.h
>>>
>>>> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
>>>> +  inline void dump (mem_location *loc, mem_usage &total) const
>>>> +  {
>>>> +    char s[4096];
>>>> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
>>>> +         loc->m_line, loc->m_function);
>>> Static sized buffer used in a sprintf where the strings are potentially user controlled.   Not good, even in dumping code, still not good.
>>>
>>>> +
>>>> +    s[48] = '\0';
>>> ?!?  Presumably you're just truncating the output line here for the subsequent fprintf call.  Consider using a const with a symbolic name rather than the magic "48".  I say "consider" because there's magic constants all over the place in the dumping code. So it may not be worth the effort.  Your call.
>>>
>>>   +
>>>> +  /* Dump header with NAME.  */
>>>> +  static inline void dump_header (const char *name)
>>>> +  {
>>>> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
>>>> +         "Pools", "Leak", "Peak", "Times", "Elt size");
>>>> +    print_dash_line ();
>>>> +  }
>>>> +
>>>> +  /* Dump footer.  */
>>>> +  inline void dump_footer ()
>>>> +  {
>>>> +    print_dash_line ();
>>>> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
>>>> +         (long)m_allocated);
>>>> +    print_dash_line ();
>>>> +  }
>>> Note the header is static inline, footer is just inline.  Please try to make them consistent.
> It doesn't look like you did anything with this.  Is there a reason that the dump_header and dump_footer have different linkage?  Also the linkage/return type for dump_header should be on its own line.
> 
> With that fixed, this is OK for the trunk.
> 
> jeff

Hi Jeff.

Different linkage is because of dump_header is just a generic header unrelated to any real numbers.
On the other hand, dump_footer is called on a total instance.

I've just identified that the whole memory statistics infrastructure lacks correct GNU formatting
in case of C++ member functions. I'm going to fix it in a different patch.

Thanks,
Martin


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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-02 11:14 [PATCH 1/2] Memory statistics enhancement mliska
  2015-06-02 11:18 ` [PATCH 2/2] Fix memory report layout at various places mliska
  2015-06-02 14:00 ` [PATCH 1/2] Memory statistics enhancement Jeff Law
@ 2015-06-03  9:47 ` Martin Liška
  2015-06-03 13:27   ` Jeff Law
  2015-06-08 15:02 ` Martin Liška
  3 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2015-06-03  9:47 UTC (permalink / raw)
  To: gcc-patches

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

On 06/01/2015 06:16 PM, mliska wrote:
> Hi.
> 
> Following 2 patches improve memory statistics infrastructure. First one
> ports pool allocator to the new infrastructure. And the second one makes
> column alignment properly.
> 
> Both can bootstrap on x86_64-linux-pc and survive regression tests.
> 
> Ready for trunk?
> Thank you,
> Martin
> 
> Port pool-allocator memory stats to a new infrastructure.
> 
> gcc/ChangeLog:
> 
> 2015-06-02  Martin Liska  <mliska@suse.cz>
> 
> 	* alloc-pool.c (allocate_pool_descriptor): Remove.
> 	(struct pool_output_info): Likewise.
> 	(print_alloc_pool_statistics): Likewise.
> 	(dump_alloc_pool_statistics): Likewise.
> 	* alloc-pool.h (struct pool_usage): New struct.
> 	(pool_allocator::initialize): Change usage of memory statistics
> 	to a new interface.
> 	(pool_allocator::release): Likewise.
> 	(pool_allocator::allocate): Likewise.
> 	(pool_allocator::remove): Likewise.
> 	* mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
> 	for a pool allocator.
> 	* mem-stats.h (struct mem_location): Add new ctor.
> 	(struct mem_usage): Add counter for number of
> 	instances.
> 	(mem_alloc_description::register_descriptor): New overload of
> 	the function.
> ---
>  gcc/alloc-pool.c       |  60 +----------------------------
>  gcc/alloc-pool.h       | 102 +++++++++++++++++++++++++++++++++++++++----------
>  gcc/mem-stats-traits.h |   3 +-
>  gcc/mem-stats.h        |  69 ++++++++++++++++++++++-----------
>  4 files changed, 132 insertions(+), 102 deletions(-)
> 
> diff --git a/gcc/alloc-pool.c b/gcc/alloc-pool.c
> index e9fdc86..601c2b7 100644
> --- a/gcc/alloc-pool.c
> +++ b/gcc/alloc-pool.c
> @@ -26,70 +26,14 @@ along with GCC; see the file COPYING3.  If not see
>  #include "hash-map.h"
>  
>  ALLOC_POOL_ID_TYPE last_id;
> -
> -/* Hashtable mapping alloc_pool names to descriptors.  */
> -hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
> -
> -struct alloc_pool_descriptor *
> -allocate_pool_descriptor (const char *name)
> -{
> -  if (!alloc_pool_hash)
> -    alloc_pool_hash = new hash_map<const char *, alloc_pool_descriptor> (10,
> -									 false,
> -									 false);
> -
> -  return &alloc_pool_hash->get_or_insert (name);
> -}
> -
> -/* Output per-alloc_pool statistics.  */
> -
> -/* Used to accumulate statistics about alloc_pool sizes.  */
> -struct pool_output_info
> -{
> -  unsigned long total_created;
> -  unsigned long total_allocated;
> -};
> -
> -/* Called via hash_map.traverse.  Output alloc_pool descriptor pointed out by
> -   SLOT and update statistics.  */
> -bool
> -print_alloc_pool_statistics (const char *const &name,
> -			     const alloc_pool_descriptor &d,
> -			     struct pool_output_info *i)
> -{
> -  if (d.allocated)
> -    {
> -      fprintf (stderr,
> -	       "%-22s %6d %10lu %10lu(%10lu) %10lu(%10lu) %10lu(%10lu)\n",
> -	       name, d.elt_size, d.created, d.allocated,
> -	       d.allocated / d.elt_size, d.peak, d.peak / d.elt_size,
> -	       d.current, d.current / d.elt_size);
> -      i->total_allocated += d.allocated;
> -      i->total_created += d.created;
> -    }
> -  return 1;
> -}
> +mem_alloc_description<pool_usage> pool_allocator_usage;
>  
>  /* Output per-alloc_pool memory usage statistics.  */
>  void
>  dump_alloc_pool_statistics (void)
>  {
> -  struct pool_output_info info;
> -
>    if (! GATHER_STATISTICS)
>      return;
>  
> -  if (!alloc_pool_hash)
> -    return;
> -
> -  fprintf (stderr, "\nAlloc-pool Kind         Elt size  Pools  Allocated (elts)            Peak (elts)            Leak (elts)\n");
> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
> -  info.total_created = 0;
> -  info.total_allocated = 0;
> -  alloc_pool_hash->traverse <struct pool_output_info *,
> -			     print_alloc_pool_statistics> (&info);
> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
> -  fprintf (stderr, "%-22s           %7lu %10lu\n",
> -	   "Total", info.total_created, info.total_allocated);
> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
> +  pool_allocator_usage.dump (ALLOC_POOL);
>  }
> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
> index 96a1342..a1727ce 100644
> --- a/gcc/alloc-pool.h
> +++ b/gcc/alloc-pool.h
> @@ -26,6 +26,71 @@ extern void dump_alloc_pool_statistics (void);
>  
>  typedef unsigned long ALLOC_POOL_ID_TYPE;
>  
> +/* Pool allocator memory usage.  */
> +struct pool_usage: public mem_usage
> +{
> +  /* Default contructor.  */
> +  pool_usage (): m_element_size (0), m_pool_name ("") {}
> +  /* Constructor.  */
> +  pool_usage (size_t allocated, size_t times, size_t peak,
> +	      size_t instances, size_t element_size,
> +	      const char *pool_name)
> +    : mem_usage (allocated, times, peak, instances),
> +      m_element_size (element_size),
> +      m_pool_name (pool_name) {}
> +
> +  /* Sum the usage with SECOND usage.  */
> +  pool_usage operator+ (const pool_usage &second)
> +  {
> +    return pool_usage (m_allocated + second.m_allocated,
> +			     m_times + second.m_times,
> +			     m_peak + second.m_peak,
> +			     m_instances + second.m_instances,
> +			     m_element_size, m_pool_name);
> +  }
> +
> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
> +  inline void dump (mem_location *loc, mem_usage &total) const
> +  {
> +    char s[4096];
> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
> +	     loc->m_line, loc->m_function);
> +
> +    s[48] = '\0';
> +
> +    fprintf (stderr, "%-32s%-48s %6li%10li:%5.1f%%%10li%10li:%5.1f%%%12li\n",
> +	     m_pool_name, s, (long)m_instances,
> +	     (long)m_allocated, get_percent (m_allocated, total.m_allocated),
> +	     (long)m_peak, (long)m_times,
> +	     get_percent (m_times, total.m_times),
> +	     (long)m_element_size);
> +  }
> +
> +  /* Dump header with NAME.  */
> +  static inline void dump_header (const char *name)
> +  {
> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
> +	     "Pools", "Leak", "Peak", "Times", "Elt size");
> +    print_dash_line ();
> +  }
> +
> +  /* Dump footer.  */
> +  inline void dump_footer ()
> +  {
> +    print_dash_line ();
> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
> +	     (long)m_allocated);
> +    print_dash_line ();
> +  }
> +
> +  /* Element size.  */
> +  size_t m_element_size;
> +  /* Pool name.  */
> +  const char *m_pool_name;
> +};
> +
> +extern mem_alloc_description<pool_usage> pool_allocator_usage;
> +
>  /* Type based memory pool allocator.  */
>  template <typename T>
>  class pool_allocator
> @@ -35,7 +100,7 @@ public:
>       has NUM elements.  The allocator support EXTRA_SIZE and can
>       potentially IGNORE_TYPE_SIZE.  */
>    pool_allocator (const char *name, size_t num, size_t extra_size = 0,
> -		  bool ignore_type_size = false);
> +		  bool ignore_type_size = false CXX_MEM_STAT_INFO);
>    ~pool_allocator ();
>    void release ();
>    void release_if_empty ();
> @@ -122,6 +187,8 @@ private:
>    size_t m_extra_size;
>    /* Flag if a pool allocator is initialized.  */
>    bool m_initialized;
> +  /* Memory allocation location.  */
> +  mem_location m_location;
>  };
>  
>  /* Last used ID.  */
> @@ -151,19 +218,16 @@ struct alloc_pool_descriptor
>  /* Hashtable mapping alloc_pool names to descriptors.  */
>  extern hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
>  
> -/* For given name, return descriptor, create new if needed.  */
> -alloc_pool_descriptor *
> -allocate_pool_descriptor (const char *name);
> -
>  template <typename T>
>  inline
>  pool_allocator<T>::pool_allocator (const char *name, size_t num,
> -				   size_t extra_size, bool ignore_type_size):
> +				   size_t extra_size, bool ignore_type_size
> +				   MEM_STAT_DECL):
>    m_name (name), m_elts_per_block (num), m_returned_free_list (NULL),
>    m_virgin_free_list (NULL), m_virgin_elts_remaining (0), m_elts_allocated (0),
>    m_elts_free (0), m_blocks_allocated (0), m_block_list (NULL),
>    m_ignore_type_size (ignore_type_size), m_extra_size (extra_size),
> -  m_initialized (false) {}
> +  m_initialized (false), m_location (ALLOC_POOL, false PASS_MEM_STAT) {}
>  
>  /* Initialize a pool allocator.  */
>  
> @@ -196,9 +260,11 @@ pool_allocator<T>::initialize ()
>  
>    if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -      desc->elt_size = size;
> -      desc->created++;
> +      pool_usage *u = pool_allocator_usage.register_descriptor
> +	(this, new mem_location (m_location));
> +
> +      u->m_element_size = m_elt_size;
> +      u->m_pool_name = m_name;
>      }
>  
>    /* List header size should be a multiple of 8.  */
> @@ -235,10 +301,10 @@ pool_allocator<T>::release ()
>        free (block);
>      }
>  
> -  if (GATHER_STATISTICS && false)
> +  if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -      desc->current -= (m_elts_allocated - m_elts_free) * m_elt_size;
> +      pool_allocator_usage.release_instance_overhead (this,
> +	(m_elts_allocated - m_elts_free) * m_elt_size);
>      }
>  
>    m_returned_free_list = NULL;
> @@ -279,12 +345,7 @@ pool_allocator<T>::allocate ()
>  
>    if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -
> -      desc->allocated += m_elt_size;
> -      desc->current += m_elt_size;
> -      if (desc->peak < desc->current)
> -	desc->peak = desc->current;
> +      pool_allocator_usage.register_instance_overhead (m_elt_size, this);
>      }
>  
>  #ifdef ENABLE_VALGRIND_ANNOTATIONS
> @@ -383,8 +444,7 @@ pool_allocator<T>::remove (T *object)
>  
>    if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -      desc->current -= m_elt_size;
> +      pool_allocator_usage.release_instance_overhead (this, m_elt_size);
>      }
>  }
>  
> diff --git a/gcc/mem-stats-traits.h b/gcc/mem-stats-traits.h
> index de1614e..f7843f2 100644
> --- a/gcc/mem-stats-traits.h
> +++ b/gcc/mem-stats-traits.h
> @@ -10,11 +10,12 @@ enum mem_alloc_origin
>    VEC,
>    BITMAP,
>    GGC,
> +  ALLOC_POOL,
>    MEM_ALLOC_ORIGIN_LENGTH
>  };
>  
>  /* Verbose names of the memory allocation origin.  */
>  static const char * mem_alloc_origin_names[] = { "Hash tables", "Hash maps",
> -  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory" };
> +  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory", "Allocation pool" };
>  
>  #endif // GCC_MEM_STATS_TRAITS_H
> diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
> index ac47231..fea68fa 100644
> --- a/gcc/mem-stats.h
> +++ b/gcc/mem-stats.h
> @@ -18,11 +18,16 @@ struct mem_location
>    inline mem_location () {}
>  
>    /* Constructor.  */
> -  inline mem_location (const char *filename, const char *function, int line,
> -		mem_alloc_origin origin, bool ggc):
> +  inline mem_location (mem_alloc_origin origin, bool ggc,
> +		const char *filename = NULL, int line = 0, const char *function = NULL):
>      m_filename (filename), m_function (function), m_line (line), m_origin
>      (origin), m_ggc (ggc) {}
>  
> +  /* Copy constructor.  */
> +  inline mem_location (mem_location &other): m_filename (other.m_filename),
> +    m_function (other.m_function), m_line (other.m_line),
> +    m_origin (other.m_origin), m_ggc (other.m_ggc) {}
> +
>    /* Compute hash value based on file name, function name and line in
>       source code. As there is just a single pointer registered for every
>       constant that points to e.g. the same file name, we can use hash
> @@ -79,11 +84,12 @@ struct mem_location
>  struct mem_usage
>  {
>    /* Default constructor.  */
> -  mem_usage (): m_allocated (0), m_times (0), m_peak (0) {}
> +  mem_usage (): m_allocated (0), m_times (0), m_peak (0), m_instances (1) {}
>  
>    /* Constructor.  */
> -  mem_usage (size_t allocated, size_t times, size_t peak):
> -    m_allocated (allocated), m_times (times), m_peak (peak) {}
> +  mem_usage (size_t allocated, size_t times, size_t peak, size_t instances = 0):
> +    m_allocated (allocated), m_times (times), m_peak (peak),
> +    m_instances (instances) {}
>  
>    /* Register overhead of SIZE bytes.  */
>    inline void register_overhead (size_t size)
> @@ -108,7 +114,8 @@ struct mem_usage
>    {
>      return mem_usage (m_allocated + second.m_allocated,
>  		      m_times + second.m_times,
> -		      m_peak + second.m_peak);
> +		      m_peak + second.m_peak,
> +		      m_instances + second.m_instances);
>    }
>  
>    /* Comparison operator.  */
> @@ -163,7 +170,7 @@ struct mem_usage
>    /* Print line made of dashes.  */
>    static inline void print_dash_line ()
>    {
> -    fprintf (stderr, "%s\n", std::string (128, '-').c_str ());
> +    fprintf (stderr, "%s\n", std::string (140, '-').c_str ());
>    }
>  
>    /* Dump header with NAME.  */
> @@ -180,6 +187,8 @@ struct mem_usage
>    size_t m_times;
>    /* Peak allocation in bytes.  */
>    size_t m_peak;
> +  /* Number of container instances.  */
> +  size_t m_instances;
>  };
>  
>  /* Memory usage pair that connectes memory usage and number
> @@ -241,9 +250,13 @@ public:
>    /* Return descriptor for instance PTR.  */
>    T *get_descriptor_for_instance (const void *ptr);
>  
> -  /* Register memory allocation descriptor for container PTR. ORIGIN identifies
> +  /* Register memory allocation descriptor for container PTR which is
> +     described by a memory LOCATION.  */
> +  T *register_descriptor (const void *ptr, mem_location *location);
> +
> +  /* Register memory allocation descriptor for container PTR.  ORIGIN identifies
>       type of container and GGC identifes if the allocation is handled in GGC
> -     memory. Each location is identified by file NAME, LINE in source code and
> +     memory.  Each location is identified by file NAME, LINE in source code and
>       FUNCTION name.  */
>    T *register_descriptor (const void *ptr, mem_alloc_origin origin,
>  			  bool ggc, const char *name, int line,
> @@ -321,33 +334,27 @@ mem_alloc_description<T>::get_descriptor_for_instance (const void *ptr)
>    return m_reverse_map->get (ptr) ? (*m_reverse_map->get (ptr)).usage : NULL;
>  }
>  
> -/* Register memory allocation descriptor for container PTR. ORIGIN identifies
> -   type of container and GGC identifes if the allocation is handled in GGC
> -   memory. Each location is identified by file NAME, LINE in source code and
> -   FUNCTION name.  */
>  
> +  /* Register memory allocation descriptor for container PTR which is
> +     described by a memory LOCATION.  */
>  template <class T>
>  inline T*
>  mem_alloc_description<T>::register_descriptor (const void *ptr,
> -					       mem_alloc_origin origin,
> -					       bool ggc,
> -					       const char *filename,
> -					       int line,
> -					       const char *function)
> +					       mem_location *location)
>  {
> -  mem_location *l = new mem_location (filename, function, line, origin, ggc);
>    T *usage = NULL;
>  
> -  T **slot = m_map->get (l);
> +  T **slot = m_map->get (location);
>    if (slot)
>      {
> -      delete l;
> +      delete location;
>        usage = *slot;
> +      usage->m_instances++;
>      }
>    else
>      {
>        usage = new T ();
> -      m_map->put (l, usage);
> +      m_map->put (location, usage);
>      }
>  
>    if (!m_reverse_map->get (ptr))
> @@ -356,6 +363,24 @@ mem_alloc_description<T>::register_descriptor (const void *ptr,
>    return usage;
>  }
>  
> +/* Register memory allocation descriptor for container PTR.  ORIGIN identifies
> +   type of container and GGC identifes if the allocation is handled in GGC
> +   memory.  Each location is identified by file NAME, LINE in source code and
> +   FUNCTION name.  */
> +
> +template <class T>
> +inline T*
> +mem_alloc_description<T>::register_descriptor (const void *ptr,
> +					       mem_alloc_origin origin,
> +					       bool ggc,
> +					       const char *filename,
> +					       int line,
> +					       const char *function)
> +{
> +  mem_location *l = new mem_location (origin, ggc, filename, line, function);
> +  return register_descriptor (ptr, l);
> +}
> +
>  /* Register instance overhead identified by PTR pointer. Allocation takes
>     SIZE bytes.  */
>  
> 

Hello.

This fix changes all function definitions (related to memory statistics infrastructure)
with respect to GNU coding style.

Ready for trunk?
Thanks,
Martin

[-- Attachment #2: 0001-Fix-GNU-coding-style-in-memory-statistics.patch --]
[-- Type: text/x-patch, Size: 17742 bytes --]

From a52eecf7c78f2eee0ccac662459e89ddbedd0244 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Wed, 3 Jun 2015 11:37:01 +0200
Subject: [PATCH] Fix GNU coding style in memory statistics.

gcc/ChangeLog:

2015-06-03  Martin Liska  <mliska@suse.cz>

	* alloc-pool.h (struct pool_usage): Correct GNU coding style.
	* bitmap.h (struct bitmap_usage): Likewise.
	* ggc-common.c (struct ggc_usage): Likewise.
	* mem-stats.h (struct mem_location): Likewise.
	(struct mem_usage): Likewise.
	* vec.c (struct vec_usage): Likewise.
---
 gcc/alloc-pool.h | 21 +++++++++-----
 gcc/bitmap.h     |  9 ++++--
 gcc/ggc-common.c | 36 +++++++++++++++--------
 gcc/mem-stats.h  | 87 +++++++++++++++++++++++++++++++++++++-------------------
 gcc/vec.c        | 18 ++++++++----
 5 files changed, 113 insertions(+), 58 deletions(-)

diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
index b10cfa4..ed0cb6d 100644
--- a/gcc/alloc-pool.h
+++ b/gcc/alloc-pool.h
@@ -40,7 +40,8 @@ struct pool_usage: public mem_usage
       m_pool_name (pool_name) {}
 
   /* Sum the usage with SECOND usage.  */
-  pool_usage operator+ (const pool_usage &second)
+  pool_usage
+  operator+ (const pool_usage &second)
   {
     return pool_usage (m_allocated + second.m_allocated,
 			     m_times + second.m_times,
@@ -50,7 +51,8 @@ struct pool_usage: public mem_usage
   }
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
-  inline void dump (mem_location *loc, mem_usage &total) const
+  inline void
+  dump (mem_location *loc, mem_usage &total) const
   {
     char *location_string = loc->to_string ();
 
@@ -65,7 +67,8 @@ struct pool_usage: public mem_usage
   }
 
   /* Dump header with NAME.  */
-  static inline void dump_header (const char *name)
+  static inline void
+  dump_header (const char *name)
   {
     fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
 	     "Pools", "Leak", "Peak", "Times", "Elt size");
@@ -73,7 +76,8 @@ struct pool_usage: public mem_usage
   }
 
   /* Dump footer.  */
-  inline void dump_footer ()
+  inline void
+  dump_footer ()
   {
     print_dash_line ();
     fprintf (stderr, "%s%82li%10li\n", "Total", (long)m_instances,
@@ -133,21 +137,24 @@ private:
 	int64_t align_i;
       } u;
 
-    static inline allocation_object<U> *get_instance (void *data_ptr)
+    static inline allocation_object<U> *
+    get_instance (void *data_ptr)
     {
       return (allocation_object<U> *)(((char *)(data_ptr))
 				      - offsetof (allocation_object<U>,
 						  u.data));
     }
 
-    static inline U *get_data (void *instance_ptr)
+    static inline U *
+    get_data (void *instance_ptr)
     {
       return (U*)(((allocation_object<U> *) instance_ptr)->u.data);
     }
   };
 
   /* Align X to 8.  */
-  size_t align_eight (size_t x)
+  size_t
+  align_eight (size_t x)
   {
     return (((x+7) >> 3) << 3);
   }
diff --git a/gcc/bitmap.h b/gcc/bitmap.h
index 4309f6d..1d37bca 100644
--- a/gcc/bitmap.h
+++ b/gcc/bitmap.h
@@ -144,7 +144,8 @@ struct bitmap_usage: public mem_usage
     m_nsearches (nsearches), m_search_iter (search_iter) {}
 
   /* Sum the usage with SECOND usage.  */
-  bitmap_usage operator+ (const bitmap_usage &second)
+  bitmap_usage
+  operator+ (const bitmap_usage &second)
   {
     return bitmap_usage (m_allocated + second.m_allocated,
 			     m_times + second.m_times,
@@ -154,7 +155,8 @@ struct bitmap_usage: public mem_usage
   }
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
-  inline void dump (mem_location *loc, mem_usage &total) const
+  inline void
+  dump (mem_location *loc, mem_usage &total) const
   {
     char *location_string = loc->to_string ();
 
@@ -170,7 +172,8 @@ struct bitmap_usage: public mem_usage
   }
 
   /* Dump header with NAME.  */
-  static inline void dump_header (const char *name)
+  static inline void
+  dump_header (const char *name)
   {
     fprintf (stderr, "%-48s %11s%16s%17s%12s%12s%10s\n", name, "Leak", "Peak",
 	     "Times", "N searches", "Search iter", "Type");
diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index 43ccc0b..cd81770 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -843,7 +843,8 @@ struct ggc_usage: public mem_usage
     m_freed (freed), m_collected (collected), m_overhead (overhead) {}
 
   /* Comparison operator.  */
-  inline bool operator< (const ggc_usage &second) const
+  inline bool
+  operator< (const ggc_usage &second) const
   {
     return (get_balance () == second.get_balance () ?
 	    (m_peak == second.m_peak ? m_times < second.m_times
@@ -852,7 +853,8 @@ struct ggc_usage: public mem_usage
   }
 
   /* Register overhead of ALLOCATED and OVERHEAD bytes.  */
-  inline void register_overhead (size_t allocated, size_t overhead)
+  inline void
+  register_overhead (size_t allocated, size_t overhead)
   {
     m_allocated += allocated;
     m_overhead += overhead;
@@ -860,13 +862,15 @@ struct ggc_usage: public mem_usage
   }
 
   /* Release overhead of SIZE bytes.  */
-  inline void release_overhead (size_t size)
+  inline void
+  release_overhead (size_t size)
   {
     m_freed += size;
   }
 
   /* Sum the usage with SECOND usage.  */
-  ggc_usage operator+ (const ggc_usage &second)
+  ggc_usage
+  operator+ (const ggc_usage &second)
   {
     return ggc_usage (m_allocated + second.m_allocated,
 		      m_times + second.m_times,
@@ -877,7 +881,8 @@ struct ggc_usage: public mem_usage
   }
 
   /* Dump usage with PREFIX, where TOTAL is sum of all rows.  */
-  inline void dump (const char *prefix, ggc_usage &total) const
+  inline void
+  dump (const char *prefix, ggc_usage &total) const
   {
     long balance = get_balance ();
     fprintf (stderr,
@@ -892,7 +897,8 @@ struct ggc_usage: public mem_usage
   }
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
-  inline void dump (mem_location *loc, ggc_usage &total) const
+  inline void
+  dump (mem_location *loc, ggc_usage &total) const
   {
     char *location_string = loc->to_string ();
 
@@ -902,7 +908,8 @@ struct ggc_usage: public mem_usage
   }
 
   /* Dump footer.  */
-  inline void dump_footer ()
+  inline void
+  dump_footer ()
   {
     print_dash_line ();
     dump ("Total", *this);
@@ -910,7 +917,8 @@ struct ggc_usage: public mem_usage
   }
 
   /* Get balance which is GGC allocation leak.  */
-  inline long get_balance () const
+  inline long
+  get_balance () const
   {
     return m_allocated + m_overhead - m_collected - m_freed;
   }
@@ -918,7 +926,8 @@ struct ggc_usage: public mem_usage
   typedef std::pair<mem_location *, ggc_usage *> mem_pair_t;
 
   /* Compare wrapper used by qsort method.  */
-  static int compare (const void *first, const void *second)
+  static int
+  compare (const void *first, const void *second)
   {
     const mem_pair_t f = *(const mem_pair_t *)first;
     const mem_pair_t s = *(const mem_pair_t *)second;
@@ -927,8 +936,10 @@ struct ggc_usage: public mem_usage
   }
 
   /* Compare rows in final GGC summary dump.  */
-  static int compare_final (const void *first, const void *second)
-  {  typedef std::pair<mem_location *, ggc_usage *> mem_pair_t;
+  static int
+  compare_final (const void *first, const void *second)
+  {
+    typedef std::pair<mem_location *, ggc_usage *> mem_pair_t;
 
     const ggc_usage *f = ((const mem_pair_t *)first)->second;
     const ggc_usage *s = ((const mem_pair_t *)second)->second;
@@ -940,7 +951,8 @@ struct ggc_usage: public mem_usage
   }
 
   /* Dump header with NAME.  */
-  static inline void dump_header (const char *name)
+  static inline void
+  dump_header (const char *name)
   {
     fprintf (stderr, "%-48s %11s%17s%17s%16s%17s\n", name, "Garbage", "Freed",
 	     "Leak", "Overhead", "Times");
diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index 641f7c1..7524ff7 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -18,17 +18,20 @@ class hash_map;
 struct mem_location
 {
   /* Default constructor.  */
-  inline mem_location () {}
+  inline
+  mem_location () {}
 
   /* Constructor.  */
-  inline mem_location (mem_alloc_origin origin, bool ggc,
+  inline
+  mem_location (mem_alloc_origin origin, bool ggc,
 		const char *filename = NULL, int line = 0,
 		const char *function = NULL):
     m_filename (filename), m_function (function), m_line (line), m_origin
     (origin), m_ggc (ggc) {}
 
   /* Copy constructor.  */
-  inline mem_location (mem_location &other): m_filename (other.m_filename),
+  inline
+  mem_location (mem_location &other): m_filename (other.m_filename),
     m_function (other.m_function), m_line (other.m_line),
     m_origin (other.m_origin), m_ggc (other.m_ggc) {}
 
@@ -36,7 +39,8 @@ struct mem_location
      source code. As there is just a single pointer registered for every
      constant that points to e.g. the same file name, we can use hash
      of the pointer.  */
-  hashval_t hash ()
+  hashval_t
+  hash ()
   {
     inchash::hash hash;
 
@@ -48,14 +52,16 @@ struct mem_location
   }
 
   /* Return true if the memory location is equal to OTHER.  */
-  int equal (mem_location &other)
+  int
+  equal (mem_location &other)
   {
     return m_filename == other.m_filename && m_function == other.m_function
       && m_line == other.m_line;
   }
 
   /* Return trimmed filename for the location.  */
-  inline const char *get_trimmed_filename ()
+  inline const char *
+  get_trimmed_filename ()
   {
     const char *s1 = m_filename;
     const char *s2;
@@ -66,7 +72,8 @@ struct mem_location
     return s1;
   }
 
-  inline char *to_string ()
+  inline char *
+  to_string ()
   {
     unsigned l = strlen (get_trimmed_filename ()) + strlen (m_function)
       + LOCATION_LINE_EXTRA_SPACE;
@@ -81,7 +88,8 @@ struct mem_location
   }
 
   /* Return display name associated to ORIGIN type.  */
-  static const char *get_origin_name (mem_alloc_origin origin)
+  static const char *
+  get_origin_name (mem_alloc_origin origin)
   {
     return mem_alloc_origin_names[(unsigned) origin];
   }
@@ -110,7 +118,8 @@ struct mem_usage
     m_instances (instances) {}
 
   /* Register overhead of SIZE bytes.  */
-  inline void register_overhead (size_t size)
+  inline void
+  register_overhead (size_t size)
   {
     m_allocated += size;
     m_times++;
@@ -120,7 +129,8 @@ struct mem_usage
   }
 
   /* Release overhead of SIZE bytes.  */
-  inline void release_overhead (size_t size)
+  inline void
+  release_overhead (size_t size)
   {
     gcc_assert (size <= m_allocated);
 
@@ -128,7 +138,8 @@ struct mem_usage
   }
 
   /* Sum the usage with SECOND usage.  */
-  mem_usage operator+ (const mem_usage &second)
+  mem_usage
+  operator+ (const mem_usage &second)
   {
     return mem_usage (m_allocated + second.m_allocated,
 		      m_times + second.m_times,
@@ -137,7 +148,8 @@ struct mem_usage
   }
 
   /* Comparison operator.  */
-  inline bool operator< (const mem_usage &second) const
+  inline bool
+  operator< (const mem_usage &second) const
   {
     return (m_allocated == second.m_allocated ?
 	    (m_peak == second.m_peak ? m_times < second.m_times
@@ -145,7 +157,8 @@ struct mem_usage
   }
 
   /* Compare wrapper used by qsort method.  */
-  static int compare (const void *first, const void *second)
+  static int
+  compare (const void *first, const void *second)
   {
     typedef std::pair<mem_location *, mem_usage *> mem_pair_t;
 
@@ -156,7 +169,8 @@ struct mem_usage
   }
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
-  inline void dump (mem_location *loc, mem_usage &total) const
+  inline void
+  dump (mem_location *loc, mem_usage &total) const
   {
     char *location_string = loc->to_string ();
 
@@ -170,7 +184,8 @@ struct mem_usage
   }
 
   /* Dump footer.  */
-  inline void dump_footer () const
+  inline void
+  dump_footer () const
   {
     print_dash_line ();
     fprintf (stderr, "%s%54li%27li\n", "Total", (long)m_allocated,
@@ -179,19 +194,22 @@ struct mem_usage
   }
 
   /* Return fraction of NOMINATOR and DENOMINATOR in percent.  */
-  static inline float get_percent (size_t nominator, size_t denominator)
+  static inline float
+  get_percent (size_t nominator, size_t denominator)
   {
     return denominator == 0 ? 0.0f : nominator * 100.0 / denominator;
   }
 
   /* Print line made of dashes.  */
-  static inline void print_dash_line (size_t count = 140)
+  static inline void
+  print_dash_line (size_t count = 140)
   {
     fprintf (stderr, "%s\n", std::string (count, '-').c_str ());
   }
 
   /* Dump header with NAME.  */
-  static inline void dump_header (const char *name)
+  static inline void
+  dump_header (const char *name)
   {
     fprintf (stderr, "%-48s %11s%16s%10s%17s\n", name, "Leak", "Peak",
 	     "Times", "Type");
@@ -262,50 +280,59 @@ public:
   ~mem_alloc_description ();
 
   /* Returns true if instance PTR is registered by the memory description.  */
-  bool contains_descriptor_for_instance (const void *ptr);
+  bool
+  contains_descriptor_for_instance (const void *ptr);
 
   /* Return descriptor for instance PTR.  */
-  T *get_descriptor_for_instance (const void *ptr);
+  T *
+  get_descriptor_for_instance (const void *ptr);
 
   /* Register memory allocation descriptor for container PTR which is
      described by a memory LOCATION.  */
-  T *register_descriptor (const void *ptr, mem_location *location);
+  T *
+  register_descriptor (const void *ptr, mem_location *location);
 
   /* Register memory allocation descriptor for container PTR.  ORIGIN identifies
      type of container and GGC identifes if the allocation is handled in GGC
      memory.  Each location is identified by file NAME, LINE in source code and
      FUNCTION name.  */
-  T *register_descriptor (const void *ptr, mem_alloc_origin origin,
+  T *
+  register_descriptor (const void *ptr, mem_alloc_origin origin,
 			  bool ggc, const char *name, int line,
 			  const char *function);
 
   /* Register instance overhead identified by PTR pointer. Allocation takes
      SIZE bytes.  */
-  T *register_instance_overhead (size_t size, const void *ptr);
+  T *
+  register_instance_overhead (size_t size, const void *ptr);
 
   /* For containers (and GGC) where we want to track every instance object,
      we register allocation of SIZE bytes, identified by PTR pointer, belonging
      to USAGE descriptor.  */
-  void register_object_overhead (T *usage, size_t size, const void *ptr);
+  void
+  register_object_overhead (T *usage, size_t size, const void *ptr);
 
   /* Release PTR pointer of SIZE bytes. If REMOVE_FROM_MAP is set to true,
      remove the instance from reverse map.  */
-  void release_instance_overhead (void *ptr, size_t size,
+  void
+  release_instance_overhead (void *ptr, size_t size,
 				  bool remove_from_map = false);
 
   /* Release intance object identified by PTR pointer.  */
-  void release_object_overhead (void *ptr);
+  void
+  release_object_overhead (void *ptr);
 
   /* Get sum value for ORIGIN type of allocation for the descriptor.  */
-  T get_sum (mem_alloc_origin origin);
+  T
+  get_sum (mem_alloc_origin origin);
 
   /* Get all tracked instances registered by the description. Items
      are filtered by ORIGIN type, LENGTH is return value where we register
      the number of elements in the list. If we want to process custom order,
      CMP comparator can be provided.  */
-  mem_list_t *get_list (mem_alloc_origin origin, unsigned *length,
-			int (*cmp) (const void *first, const void *second)
-			  = NULL);
+  mem_list_t *
+  get_list (mem_alloc_origin origin, unsigned *length,
+	    int (*cmp) (const void *first, const void *second) = NULL);
 
   /* Dump all tracked instances of type ORIGIN. If we want to process custom
      order, CMP comparator can be provided.  */
diff --git a/gcc/vec.c b/gcc/vec.c
index d4a7a02..f1a4d65 100644
--- a/gcc/vec.c
+++ b/gcc/vec.c
@@ -60,7 +60,8 @@ struct vec_usage: public mem_usage
     m_items (items), m_items_peak (items_peak) {}
 
   /* Comparison operator.  */
-  inline bool operator< (const vec_usage &second) const
+  inline bool
+  operator< (const vec_usage &second) const
   {
     return (m_allocated == second.m_allocated ?
 	    (m_peak == second.m_peak ? m_times < second.m_times
@@ -68,7 +69,8 @@ struct vec_usage: public mem_usage
   }
 
   /* Sum the usage with SECOND usage.  */
-  vec_usage operator+ (const vec_usage &second)
+  vec_usage
+  operator+ (const vec_usage &second)
   {
     return vec_usage (m_allocated + second.m_allocated,
 		      m_times + second.m_times,
@@ -78,7 +80,8 @@ struct vec_usage: public mem_usage
   }
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
-  inline void dump (mem_location *loc, mem_usage &total) const
+  inline void
+  dump (mem_location *loc, mem_usage &total) const
   {
     char s[4096];
     sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
@@ -93,7 +96,8 @@ struct vec_usage: public mem_usage
   }
 
   /* Dump footer.  */
-  inline void dump_footer ()
+  inline void
+  dump_footer ()
   {
     print_dash_line ();
     fprintf (stderr, "%s%55li%25li%17li\n", "Total", (long)m_allocated,
@@ -102,7 +106,8 @@ struct vec_usage: public mem_usage
   }
 
   /* Dump header with NAME.  */
-  static inline void dump_header (const char *name)
+  static inline void
+  dump_header (const char *name)
   {
     fprintf (stderr, "%-48s %11s%15s%10s%17s%11s\n", name, "Leak", "Peak",
 	     "Times", "Leak items", "Peak items");
@@ -110,7 +115,8 @@ struct vec_usage: public mem_usage
   }
 
   /* Compare wrapper used by qsort method.  */
-  static int compare (const void *first, const void *second)
+  static int
+  compare (const void *first, const void *second)
   {
     typedef std::pair<mem_location *, vec_usage *> mem_pair_t;
 
-- 
2.1.4


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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-03  9:16       ` Martin Liška
@ 2015-06-03 13:19         ` Jeff Law
  2015-06-03 14:27           ` Martin Liška
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff Law @ 2015-06-03 13:19 UTC (permalink / raw)
  To: Martin Liška, gcc-patches

On 06/03/2015 03:14 AM, Martin Liška wrote:
> On 06/02/2015 07:17 PM, Jeff Law wrote:
>> On 06/02/2015 09:05 AM, Martin Liška wrote:
>>> On 06/02/2015 03:58 PM, Jeff Law wrote:
>>>> On 06/01/2015 10:16 AM, mliska wrote:
>>>>> Hi.
>>>>>
>>>>> Following 2 patches improve memory statistics infrastructure. First one
>>>>> ports pool allocator to the new infrastructure. And the second one makes
>>>>> column alignment properly.
>>>>>
>>>>> Both can bootstrap on x86_64-linux-pc and survive regression tests.
>>>>>
>>>>> Ready for trunk?
>>>>> Thank you,
>>>>> Martin
>>>>>
>>>>> Port pool-allocator memory stats to a new infrastructure.
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2015-06-02  Martin Liska  <mliska@suse.cz>
>>>>>
>>>>>       * alloc-pool.c (allocate_pool_descriptor): Remove.
>>>>>       (struct pool_output_info): Likewise.
>>>>>       (print_alloc_pool_statistics): Likewise.
>>>>>       (dump_alloc_pool_statistics): Likewise.
>>>>>       * alloc-pool.h (struct pool_usage): New struct.
>>>>>       (pool_allocator::initialize): Change usage of memory statistics
>>>>>       to a new interface.
>>>>>       (pool_allocator::release): Likewise.
>>>>>       (pool_allocator::allocate): Likewise.
>>>>>       (pool_allocator::remove): Likewise.
>>>>>       * mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
>>>>>       for a pool allocator.
>>>>>       * mem-stats.h (struct mem_location): Add new ctor.
>>>>>       (struct mem_usage): Add counter for number of
>>>>>       instances.
>>>>>       (mem_alloc_description::register_descriptor): New overload of
>>>>>       the function.
>>>>    -
>>>>
>>>>> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
>>>>> index 96a1342..a1727ce 100644
>>>>> --- a/gcc/alloc-pool.h
>>>>> +++ b/gcc/alloc-pool.h
>>>>
>>>>> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
>>>>> +  inline void dump (mem_location *loc, mem_usage &total) const
>>>>> +  {
>>>>> +    char s[4096];
>>>>> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
>>>>> +         loc->m_line, loc->m_function);
>>>> Static sized buffer used in a sprintf where the strings are potentially user controlled.   Not good, even in dumping code, still not good.
>>>>
>>>>> +
>>>>> +    s[48] = '\0';
>>>> ?!?  Presumably you're just truncating the output line here for the subsequent fprintf call.  Consider using a const with a symbolic name rather than the magic "48".  I say "consider" because there's magic constants all over the place in the dumping code. So it may not be worth the effort.  Your call.
>>>>
>>>>    +
>>>>> +  /* Dump header with NAME.  */
>>>>> +  static inline void dump_header (const char *name)
>>>>> +  {
>>>>> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
>>>>> +         "Pools", "Leak", "Peak", "Times", "Elt size");
>>>>> +    print_dash_line ();
>>>>> +  }
>>>>> +
>>>>> +  /* Dump footer.  */
>>>>> +  inline void dump_footer ()
>>>>> +  {
>>>>> +    print_dash_line ();
>>>>> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
>>>>> +         (long)m_allocated);
>>>>> +    print_dash_line ();
>>>>> +  }
>>>> Note the header is static inline, footer is just inline.  Please try to make them consistent.
>> It doesn't look like you did anything with this.  Is there a reason that the dump_header and dump_footer have different linkage?  Also the linkage/return type for dump_header should be on its own line.
>>
>> With that fixed, this is OK for the trunk.
>>
>> jeff
>
> Hi Jeff.
>
> Different linkage is because of dump_header is just a generic header unrelated to any real numbers.
> On the other hand, dump_footer is called on a total instance.
Right, but why does that affect linkage?   Sorry to keep harping on this 
minor issue, but something just doesn't make any sense to me.

>
> I've just identified that the whole memory statistics infrastructure lacks correct GNU formatting
> in case of C++ member functions. I'm going to fix it in a different patch.
Excellent.  Thanks,

jeff

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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-03  9:47 ` Martin Liška
@ 2015-06-03 13:27   ` Jeff Law
  0 siblings, 0 replies; 14+ messages in thread
From: Jeff Law @ 2015-06-03 13:27 UTC (permalink / raw)
  To: Martin Liška, gcc-patches

On 06/03/2015 03:41 AM, Martin Liška wrote:
>
>  From a52eecf7c78f2eee0ccac662459e89ddbedd0244 Mon Sep 17 00:00:00 2001
> From: mliska<mliska@suse.cz>
> Date: Wed, 3 Jun 2015 11:37:01 +0200
> Subject: [PATCH] Fix GNU coding style in memory statistics.
>
> gcc/ChangeLog:
>
> 2015-06-03  Martin Liska<mliska@suse.cz>
>
> 	* alloc-pool.h (struct pool_usage): Correct GNU coding style.
> 	* bitmap.h (struct bitmap_usage): Likewise.
> 	* ggc-common.c (struct ggc_usage): Likewise.
> 	* mem-stats.h (struct mem_location): Likewise.
> 	(struct mem_usage): Likewise.
> 	* vec.c (struct vec_usage): Likewise.
OK.
jeff


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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-03 13:19         ` Jeff Law
@ 2015-06-03 14:27           ` Martin Liška
  2015-06-03 17:32             ` Jeff Law
  0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2015-06-03 14:27 UTC (permalink / raw)
  To: gcc-patches

On 06/03/2015 03:18 PM, Jeff Law wrote:
> On 06/03/2015 03:14 AM, Martin Liška wrote:
>> On 06/02/2015 07:17 PM, Jeff Law wrote:
>>> On 06/02/2015 09:05 AM, Martin Liška wrote:
>>>> On 06/02/2015 03:58 PM, Jeff Law wrote:
>>>>> On 06/01/2015 10:16 AM, mliska wrote:
>>>>>> Hi.
>>>>>>
>>>>>> Following 2 patches improve memory statistics infrastructure. First one
>>>>>> ports pool allocator to the new infrastructure. And the second one makes
>>>>>> column alignment properly.
>>>>>>
>>>>>> Both can bootstrap on x86_64-linux-pc and survive regression tests.
>>>>>>
>>>>>> Ready for trunk?
>>>>>> Thank you,
>>>>>> Martin
>>>>>>
>>>>>> Port pool-allocator memory stats to a new infrastructure.
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2015-06-02  Martin Liska  <mliska@suse.cz>
>>>>>>
>>>>>>       * alloc-pool.c (allocate_pool_descriptor): Remove.
>>>>>>       (struct pool_output_info): Likewise.
>>>>>>       (print_alloc_pool_statistics): Likewise.
>>>>>>       (dump_alloc_pool_statistics): Likewise.
>>>>>>       * alloc-pool.h (struct pool_usage): New struct.
>>>>>>       (pool_allocator::initialize): Change usage of memory statistics
>>>>>>       to a new interface.
>>>>>>       (pool_allocator::release): Likewise.
>>>>>>       (pool_allocator::allocate): Likewise.
>>>>>>       (pool_allocator::remove): Likewise.
>>>>>>       * mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
>>>>>>       for a pool allocator.
>>>>>>       * mem-stats.h (struct mem_location): Add new ctor.
>>>>>>       (struct mem_usage): Add counter for number of
>>>>>>       instances.
>>>>>>       (mem_alloc_description::register_descriptor): New overload of
>>>>>>       the function.
>>>>>    -
>>>>>
>>>>>> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
>>>>>> index 96a1342..a1727ce 100644
>>>>>> --- a/gcc/alloc-pool.h
>>>>>> +++ b/gcc/alloc-pool.h
>>>>>
>>>>>> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
>>>>>> +  inline void dump (mem_location *loc, mem_usage &total) const
>>>>>> +  {
>>>>>> +    char s[4096];
>>>>>> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
>>>>>> +         loc->m_line, loc->m_function);
>>>>> Static sized buffer used in a sprintf where the strings are potentially user controlled.   Not good, even in dumping code, still not good.
>>>>>
>>>>>> +
>>>>>> +    s[48] = '\0';
>>>>> ?!?  Presumably you're just truncating the output line here for the subsequent fprintf call.  Consider using a const with a symbolic name rather than the magic "48".  I say "consider" because there's magic constants all over the place in the dumping code. So it may not be worth the effort.  Your call.
>>>>>
>>>>>    +
>>>>>> +  /* Dump header with NAME.  */
>>>>>> +  static inline void dump_header (const char *name)
>>>>>> +  {
>>>>>> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
>>>>>> +         "Pools", "Leak", "Peak", "Times", "Elt size");
>>>>>> +    print_dash_line ();
>>>>>> +  }
>>>>>> +
>>>>>> +  /* Dump footer.  */
>>>>>> +  inline void dump_footer ()
>>>>>> +  {
>>>>>> +    print_dash_line ();
>>>>>> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
>>>>>> +         (long)m_allocated);
>>>>>> +    print_dash_line ();
>>>>>> +  }
>>>>> Note the header is static inline, footer is just inline.  Please try to make them consistent.
>>> It doesn't look like you did anything with this.  Is there a reason that the dump_header and dump_footer have different linkage?  Also the linkage/return type for dump_header should be on its own line.
>>>
>>> With that fixed, this is OK for the trunk.
>>>
>>> jeff
>>
>> Hi Jeff.
>>
>> Different linkage is because of dump_header is just a generic header unrelated to any real numbers.
>> On the other hand, dump_footer is called on a total instance.
> Right, but why does that affect linkage?   Sorry to keep harping on this minor issue, but something just doesn't make any sense to me.

Hi.

That's fine, I would like to explain that difference. Each of these two functions are called in a bit different way:
T::dump_header (mem_location::get_origin_name (origin)); // that's why the function is static
total.dump_footer (); // that's why the function is method

Martin

> 
>>
>> I've just identified that the whole memory statistics infrastructure lacks correct GNU formatting
>> in case of C++ member functions. I'm going to fix it in a different patch.
> Excellent.  Thanks,
> 
> jeff

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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-03 14:27           ` Martin Liška
@ 2015-06-03 17:32             ` Jeff Law
  0 siblings, 0 replies; 14+ messages in thread
From: Jeff Law @ 2015-06-03 17:32 UTC (permalink / raw)
  To: Martin Liška, gcc-patches

On 06/03/2015 08:11 AM, Martin Liška wrote:

>
> That's fine, I would like to explain that difference. Each of these two functions are called in a bit different way:
> T::dump_header (mem_location::get_origin_name (origin)); // that's why the function is static
> total.dump_footer (); // that's why the function is method
OK.  Patch is good to go.  Thanks for being patient with my questions :-)

jeff

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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-02 11:14 [PATCH 1/2] Memory statistics enhancement mliska
                   ` (2 preceding siblings ...)
  2015-06-03  9:47 ` Martin Liška
@ 2015-06-08 15:02 ` Martin Liška
  2015-06-16 13:18   ` Martin Liška
  3 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2015-06-08 15:02 UTC (permalink / raw)
  To: gcc-patches

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

On 06/01/2015 06:16 PM, mliska wrote:
> Hi.
> 
> Following 2 patches improve memory statistics infrastructure. First one
> ports pool allocator to the new infrastructure. And the second one makes
> column alignment properly.
> 
> Both can bootstrap on x86_64-linux-pc and survive regression tests.
> 
> Ready for trunk?
> Thank you,
> Martin
> 
> Port pool-allocator memory stats to a new infrastructure.
> 
> gcc/ChangeLog:
> 
> 2015-06-02  Martin Liska  <mliska@suse.cz>
> 
> 	* alloc-pool.c (allocate_pool_descriptor): Remove.
> 	(struct pool_output_info): Likewise.
> 	(print_alloc_pool_statistics): Likewise.
> 	(dump_alloc_pool_statistics): Likewise.
> 	* alloc-pool.h (struct pool_usage): New struct.
> 	(pool_allocator::initialize): Change usage of memory statistics
> 	to a new interface.
> 	(pool_allocator::release): Likewise.
> 	(pool_allocator::allocate): Likewise.
> 	(pool_allocator::remove): Likewise.
> 	* mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
> 	for a pool allocator.
> 	* mem-stats.h (struct mem_location): Add new ctor.
> 	(struct mem_usage): Add counter for number of
> 	instances.
> 	(mem_alloc_description::register_descriptor): New overload of
> 	the function.
> ---
>  gcc/alloc-pool.c       |  60 +----------------------------
>  gcc/alloc-pool.h       | 102 +++++++++++++++++++++++++++++++++++++++----------
>  gcc/mem-stats-traits.h |   3 +-
>  gcc/mem-stats.h        |  69 ++++++++++++++++++++++-----------
>  4 files changed, 132 insertions(+), 102 deletions(-)
> 
> diff --git a/gcc/alloc-pool.c b/gcc/alloc-pool.c
> index e9fdc86..601c2b7 100644
> --- a/gcc/alloc-pool.c
> +++ b/gcc/alloc-pool.c
> @@ -26,70 +26,14 @@ along with GCC; see the file COPYING3.  If not see
>  #include "hash-map.h"
>  
>  ALLOC_POOL_ID_TYPE last_id;
> -
> -/* Hashtable mapping alloc_pool names to descriptors.  */
> -hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
> -
> -struct alloc_pool_descriptor *
> -allocate_pool_descriptor (const char *name)
> -{
> -  if (!alloc_pool_hash)
> -    alloc_pool_hash = new hash_map<const char *, alloc_pool_descriptor> (10,
> -									 false,
> -									 false);
> -
> -  return &alloc_pool_hash->get_or_insert (name);
> -}
> -
> -/* Output per-alloc_pool statistics.  */
> -
> -/* Used to accumulate statistics about alloc_pool sizes.  */
> -struct pool_output_info
> -{
> -  unsigned long total_created;
> -  unsigned long total_allocated;
> -};
> -
> -/* Called via hash_map.traverse.  Output alloc_pool descriptor pointed out by
> -   SLOT and update statistics.  */
> -bool
> -print_alloc_pool_statistics (const char *const &name,
> -			     const alloc_pool_descriptor &d,
> -			     struct pool_output_info *i)
> -{
> -  if (d.allocated)
> -    {
> -      fprintf (stderr,
> -	       "%-22s %6d %10lu %10lu(%10lu) %10lu(%10lu) %10lu(%10lu)\n",
> -	       name, d.elt_size, d.created, d.allocated,
> -	       d.allocated / d.elt_size, d.peak, d.peak / d.elt_size,
> -	       d.current, d.current / d.elt_size);
> -      i->total_allocated += d.allocated;
> -      i->total_created += d.created;
> -    }
> -  return 1;
> -}
> +mem_alloc_description<pool_usage> pool_allocator_usage;
>  
>  /* Output per-alloc_pool memory usage statistics.  */
>  void
>  dump_alloc_pool_statistics (void)
>  {
> -  struct pool_output_info info;
> -
>    if (! GATHER_STATISTICS)
>      return;
>  
> -  if (!alloc_pool_hash)
> -    return;
> -
> -  fprintf (stderr, "\nAlloc-pool Kind         Elt size  Pools  Allocated (elts)            Peak (elts)            Leak (elts)\n");
> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
> -  info.total_created = 0;
> -  info.total_allocated = 0;
> -  alloc_pool_hash->traverse <struct pool_output_info *,
> -			     print_alloc_pool_statistics> (&info);
> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
> -  fprintf (stderr, "%-22s           %7lu %10lu\n",
> -	   "Total", info.total_created, info.total_allocated);
> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
> +  pool_allocator_usage.dump (ALLOC_POOL);
>  }
> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
> index 96a1342..a1727ce 100644
> --- a/gcc/alloc-pool.h
> +++ b/gcc/alloc-pool.h
> @@ -26,6 +26,71 @@ extern void dump_alloc_pool_statistics (void);
>  
>  typedef unsigned long ALLOC_POOL_ID_TYPE;
>  
> +/* Pool allocator memory usage.  */
> +struct pool_usage: public mem_usage
> +{
> +  /* Default contructor.  */
> +  pool_usage (): m_element_size (0), m_pool_name ("") {}
> +  /* Constructor.  */
> +  pool_usage (size_t allocated, size_t times, size_t peak,
> +	      size_t instances, size_t element_size,
> +	      const char *pool_name)
> +    : mem_usage (allocated, times, peak, instances),
> +      m_element_size (element_size),
> +      m_pool_name (pool_name) {}
> +
> +  /* Sum the usage with SECOND usage.  */
> +  pool_usage operator+ (const pool_usage &second)
> +  {
> +    return pool_usage (m_allocated + second.m_allocated,
> +			     m_times + second.m_times,
> +			     m_peak + second.m_peak,
> +			     m_instances + second.m_instances,
> +			     m_element_size, m_pool_name);
> +  }
> +
> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
> +  inline void dump (mem_location *loc, mem_usage &total) const
> +  {
> +    char s[4096];
> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
> +	     loc->m_line, loc->m_function);
> +
> +    s[48] = '\0';
> +
> +    fprintf (stderr, "%-32s%-48s %6li%10li:%5.1f%%%10li%10li:%5.1f%%%12li\n",
> +	     m_pool_name, s, (long)m_instances,
> +	     (long)m_allocated, get_percent (m_allocated, total.m_allocated),
> +	     (long)m_peak, (long)m_times,
> +	     get_percent (m_times, total.m_times),
> +	     (long)m_element_size);
> +  }
> +
> +  /* Dump header with NAME.  */
> +  static inline void dump_header (const char *name)
> +  {
> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
> +	     "Pools", "Leak", "Peak", "Times", "Elt size");
> +    print_dash_line ();
> +  }
> +
> +  /* Dump footer.  */
> +  inline void dump_footer ()
> +  {
> +    print_dash_line ();
> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
> +	     (long)m_allocated);
> +    print_dash_line ();
> +  }
> +
> +  /* Element size.  */
> +  size_t m_element_size;
> +  /* Pool name.  */
> +  const char *m_pool_name;
> +};
> +
> +extern mem_alloc_description<pool_usage> pool_allocator_usage;
> +
>  /* Type based memory pool allocator.  */
>  template <typename T>
>  class pool_allocator
> @@ -35,7 +100,7 @@ public:
>       has NUM elements.  The allocator support EXTRA_SIZE and can
>       potentially IGNORE_TYPE_SIZE.  */
>    pool_allocator (const char *name, size_t num, size_t extra_size = 0,
> -		  bool ignore_type_size = false);
> +		  bool ignore_type_size = false CXX_MEM_STAT_INFO);
>    ~pool_allocator ();
>    void release ();
>    void release_if_empty ();
> @@ -122,6 +187,8 @@ private:
>    size_t m_extra_size;
>    /* Flag if a pool allocator is initialized.  */
>    bool m_initialized;
> +  /* Memory allocation location.  */
> +  mem_location m_location;
>  };
>  
>  /* Last used ID.  */
> @@ -151,19 +218,16 @@ struct alloc_pool_descriptor
>  /* Hashtable mapping alloc_pool names to descriptors.  */
>  extern hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
>  
> -/* For given name, return descriptor, create new if needed.  */
> -alloc_pool_descriptor *
> -allocate_pool_descriptor (const char *name);
> -
>  template <typename T>
>  inline
>  pool_allocator<T>::pool_allocator (const char *name, size_t num,
> -				   size_t extra_size, bool ignore_type_size):
> +				   size_t extra_size, bool ignore_type_size
> +				   MEM_STAT_DECL):
>    m_name (name), m_elts_per_block (num), m_returned_free_list (NULL),
>    m_virgin_free_list (NULL), m_virgin_elts_remaining (0), m_elts_allocated (0),
>    m_elts_free (0), m_blocks_allocated (0), m_block_list (NULL),
>    m_ignore_type_size (ignore_type_size), m_extra_size (extra_size),
> -  m_initialized (false) {}
> +  m_initialized (false), m_location (ALLOC_POOL, false PASS_MEM_STAT) {}
>  
>  /* Initialize a pool allocator.  */
>  
> @@ -196,9 +260,11 @@ pool_allocator<T>::initialize ()
>  
>    if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -      desc->elt_size = size;
> -      desc->created++;
> +      pool_usage *u = pool_allocator_usage.register_descriptor
> +	(this, new mem_location (m_location));
> +
> +      u->m_element_size = m_elt_size;
> +      u->m_pool_name = m_name;
>      }
>  
>    /* List header size should be a multiple of 8.  */
> @@ -235,10 +301,10 @@ pool_allocator<T>::release ()
>        free (block);
>      }
>  
> -  if (GATHER_STATISTICS && false)
> +  if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -      desc->current -= (m_elts_allocated - m_elts_free) * m_elt_size;
> +      pool_allocator_usage.release_instance_overhead (this,
> +	(m_elts_allocated - m_elts_free) * m_elt_size);
>      }
>  
>    m_returned_free_list = NULL;
> @@ -279,12 +345,7 @@ pool_allocator<T>::allocate ()
>  
>    if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -
> -      desc->allocated += m_elt_size;
> -      desc->current += m_elt_size;
> -      if (desc->peak < desc->current)
> -	desc->peak = desc->current;
> +      pool_allocator_usage.register_instance_overhead (m_elt_size, this);
>      }
>  
>  #ifdef ENABLE_VALGRIND_ANNOTATIONS
> @@ -383,8 +444,7 @@ pool_allocator<T>::remove (T *object)
>  
>    if (GATHER_STATISTICS)
>      {
> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
> -      desc->current -= m_elt_size;
> +      pool_allocator_usage.release_instance_overhead (this, m_elt_size);
>      }
>  }
>  
> diff --git a/gcc/mem-stats-traits.h b/gcc/mem-stats-traits.h
> index de1614e..f7843f2 100644
> --- a/gcc/mem-stats-traits.h
> +++ b/gcc/mem-stats-traits.h
> @@ -10,11 +10,12 @@ enum mem_alloc_origin
>    VEC,
>    BITMAP,
>    GGC,
> +  ALLOC_POOL,
>    MEM_ALLOC_ORIGIN_LENGTH
>  };
>  
>  /* Verbose names of the memory allocation origin.  */
>  static const char * mem_alloc_origin_names[] = { "Hash tables", "Hash maps",
> -  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory" };
> +  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory", "Allocation pool" };
>  
>  #endif // GCC_MEM_STATS_TRAITS_H
> diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
> index ac47231..fea68fa 100644
> --- a/gcc/mem-stats.h
> +++ b/gcc/mem-stats.h
> @@ -18,11 +18,16 @@ struct mem_location
>    inline mem_location () {}
>  
>    /* Constructor.  */
> -  inline mem_location (const char *filename, const char *function, int line,
> -		mem_alloc_origin origin, bool ggc):
> +  inline mem_location (mem_alloc_origin origin, bool ggc,
> +		const char *filename = NULL, int line = 0, const char *function = NULL):
>      m_filename (filename), m_function (function), m_line (line), m_origin
>      (origin), m_ggc (ggc) {}
>  
> +  /* Copy constructor.  */
> +  inline mem_location (mem_location &other): m_filename (other.m_filename),
> +    m_function (other.m_function), m_line (other.m_line),
> +    m_origin (other.m_origin), m_ggc (other.m_ggc) {}
> +
>    /* Compute hash value based on file name, function name and line in
>       source code. As there is just a single pointer registered for every
>       constant that points to e.g. the same file name, we can use hash
> @@ -79,11 +84,12 @@ struct mem_location
>  struct mem_usage
>  {
>    /* Default constructor.  */
> -  mem_usage (): m_allocated (0), m_times (0), m_peak (0) {}
> +  mem_usage (): m_allocated (0), m_times (0), m_peak (0), m_instances (1) {}
>  
>    /* Constructor.  */
> -  mem_usage (size_t allocated, size_t times, size_t peak):
> -    m_allocated (allocated), m_times (times), m_peak (peak) {}
> +  mem_usage (size_t allocated, size_t times, size_t peak, size_t instances = 0):
> +    m_allocated (allocated), m_times (times), m_peak (peak),
> +    m_instances (instances) {}
>  
>    /* Register overhead of SIZE bytes.  */
>    inline void register_overhead (size_t size)
> @@ -108,7 +114,8 @@ struct mem_usage
>    {
>      return mem_usage (m_allocated + second.m_allocated,
>  		      m_times + second.m_times,
> -		      m_peak + second.m_peak);
> +		      m_peak + second.m_peak,
> +		      m_instances + second.m_instances);
>    }
>  
>    /* Comparison operator.  */
> @@ -163,7 +170,7 @@ struct mem_usage
>    /* Print line made of dashes.  */
>    static inline void print_dash_line ()
>    {
> -    fprintf (stderr, "%s\n", std::string (128, '-').c_str ());
> +    fprintf (stderr, "%s\n", std::string (140, '-').c_str ());
>    }
>  
>    /* Dump header with NAME.  */
> @@ -180,6 +187,8 @@ struct mem_usage
>    size_t m_times;
>    /* Peak allocation in bytes.  */
>    size_t m_peak;
> +  /* Number of container instances.  */
> +  size_t m_instances;
>  };
>  
>  /* Memory usage pair that connectes memory usage and number
> @@ -241,9 +250,13 @@ public:
>    /* Return descriptor for instance PTR.  */
>    T *get_descriptor_for_instance (const void *ptr);
>  
> -  /* Register memory allocation descriptor for container PTR. ORIGIN identifies
> +  /* Register memory allocation descriptor for container PTR which is
> +     described by a memory LOCATION.  */
> +  T *register_descriptor (const void *ptr, mem_location *location);
> +
> +  /* Register memory allocation descriptor for container PTR.  ORIGIN identifies
>       type of container and GGC identifes if the allocation is handled in GGC
> -     memory. Each location is identified by file NAME, LINE in source code and
> +     memory.  Each location is identified by file NAME, LINE in source code and
>       FUNCTION name.  */
>    T *register_descriptor (const void *ptr, mem_alloc_origin origin,
>  			  bool ggc, const char *name, int line,
> @@ -321,33 +334,27 @@ mem_alloc_description<T>::get_descriptor_for_instance (const void *ptr)
>    return m_reverse_map->get (ptr) ? (*m_reverse_map->get (ptr)).usage : NULL;
>  }
>  
> -/* Register memory allocation descriptor for container PTR. ORIGIN identifies
> -   type of container and GGC identifes if the allocation is handled in GGC
> -   memory. Each location is identified by file NAME, LINE in source code and
> -   FUNCTION name.  */
>  
> +  /* Register memory allocation descriptor for container PTR which is
> +     described by a memory LOCATION.  */
>  template <class T>
>  inline T*
>  mem_alloc_description<T>::register_descriptor (const void *ptr,
> -					       mem_alloc_origin origin,
> -					       bool ggc,
> -					       const char *filename,
> -					       int line,
> -					       const char *function)
> +					       mem_location *location)
>  {
> -  mem_location *l = new mem_location (filename, function, line, origin, ggc);
>    T *usage = NULL;
>  
> -  T **slot = m_map->get (l);
> +  T **slot = m_map->get (location);
>    if (slot)
>      {
> -      delete l;
> +      delete location;
>        usage = *slot;
> +      usage->m_instances++;
>      }
>    else
>      {
>        usage = new T ();
> -      m_map->put (l, usage);
> +      m_map->put (location, usage);
>      }
>  
>    if (!m_reverse_map->get (ptr))
> @@ -356,6 +363,24 @@ mem_alloc_description<T>::register_descriptor (const void *ptr,
>    return usage;
>  }
>  
> +/* Register memory allocation descriptor for container PTR.  ORIGIN identifies
> +   type of container and GGC identifes if the allocation is handled in GGC
> +   memory.  Each location is identified by file NAME, LINE in source code and
> +   FUNCTION name.  */
> +
> +template <class T>
> +inline T*
> +mem_alloc_description<T>::register_descriptor (const void *ptr,
> +					       mem_alloc_origin origin,
> +					       bool ggc,
> +					       const char *filename,
> +					       int line,
> +					       const char *function)
> +{
> +  mem_location *l = new mem_location (origin, ggc, filename, line, function);
> +  return register_descriptor (ptr, l);
> +}
> +
>  /* Register instance overhead identified by PTR pointer. Allocation takes
>     SIZE bytes.  */
>  
> 

Hello.

There's a small patch that fixes ICE if a compiler is not built with --enable-gather-mem-stats
and we append -fmem-report to a command line options.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready for trunk?
Thanks,
Martin

[-- Attachment #2: 0001-Fallout-for-new-memory-statistics-infrastructure.patch --]
[-- Type: text/x-patch, Size: 1298 bytes --]

From 0d0e2cabf7f6946b319788ebb884c0e9e35790bd Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Mon, 8 Jun 2015 10:39:12 +0200
Subject: [PATCH] Fallout for new memory statistics infrastructure.

gcc/ChangeLog:

2015-06-08  Martin Liska  <mliska@suse.cz>

	* bitmap.c (dump_bitmap_statistics): Fix GNU coding style.
	* hash-table.c (void dump_hash_table_loc_statistics): Add missing
	guard.
---
 gcc/bitmap.c     | 2 +-
 gcc/hash-table.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/gcc/bitmap.c b/gcc/bitmap.c
index 5fc4654..20a093c 100644
--- a/gcc/bitmap.c
+++ b/gcc/bitmap.c
@@ -2079,7 +2079,7 @@ bitmap_print (FILE *file, const_bitmap head, const char *prefix,
 void
 dump_bitmap_statistics (void)
 {
-  if (! GATHER_STATISTICS)
+  if (!GATHER_STATISTICS)
     return;
 
   bitmap_mem_desc.dump (BITMAP);
diff --git a/gcc/hash-table.c b/gcc/hash-table.c
index 012b241..ee972e7 100644
--- a/gcc/hash-table.c
+++ b/gcc/hash-table.c
@@ -103,6 +103,9 @@ mem_alloc_description<mem_usage> hash_table_usage;
 /* Support function for statistics.  */
 void dump_hash_table_loc_statistics (void)
 {
+  if (!GATHER_STATISTICS)
+    return;
+
   for (unsigned i = HASH_TABLE; i <= HASH_SET; i++)
     {
       mem_alloc_origin origin = (mem_alloc_origin) i;
-- 
2.1.4


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

* Re: [PATCH 1/2] Memory statistics enhancement.
  2015-06-08 15:02 ` Martin Liška
@ 2015-06-16 13:18   ` Martin Liška
  0 siblings, 0 replies; 14+ messages in thread
From: Martin Liška @ 2015-06-16 13:18 UTC (permalink / raw)
  To: gcc-patches

On 06/08/2015 05:01 PM, Martin Liška wrote:
> On 06/01/2015 06:16 PM, mliska wrote:
>> Hi.
>>
>> Following 2 patches improve memory statistics infrastructure. First one
>> ports pool allocator to the new infrastructure. And the second one makes
>> column alignment properly.
>>
>> Both can bootstrap on x86_64-linux-pc and survive regression tests.
>>
>> Ready for trunk?
>> Thank you,
>> Martin
>>
>> Port pool-allocator memory stats to a new infrastructure.
>>
>> gcc/ChangeLog:
>>
>> 2015-06-02  Martin Liska  <mliska@suse.cz>
>>
>> 	* alloc-pool.c (allocate_pool_descriptor): Remove.
>> 	(struct pool_output_info): Likewise.
>> 	(print_alloc_pool_statistics): Likewise.
>> 	(dump_alloc_pool_statistics): Likewise.
>> 	* alloc-pool.h (struct pool_usage): New struct.
>> 	(pool_allocator::initialize): Change usage of memory statistics
>> 	to a new interface.
>> 	(pool_allocator::release): Likewise.
>> 	(pool_allocator::allocate): Likewise.
>> 	(pool_allocator::remove): Likewise.
>> 	* mem-stats-traits.h (enum mem_alloc_origin): Add new enum value
>> 	for a pool allocator.
>> 	* mem-stats.h (struct mem_location): Add new ctor.
>> 	(struct mem_usage): Add counter for number of
>> 	instances.
>> 	(mem_alloc_description::register_descriptor): New overload of
>> 	the function.
>> ---
>>  gcc/alloc-pool.c       |  60 +----------------------------
>>  gcc/alloc-pool.h       | 102 +++++++++++++++++++++++++++++++++++++++----------
>>  gcc/mem-stats-traits.h |   3 +-
>>  gcc/mem-stats.h        |  69 ++++++++++++++++++++++-----------
>>  4 files changed, 132 insertions(+), 102 deletions(-)
>>
>> diff --git a/gcc/alloc-pool.c b/gcc/alloc-pool.c
>> index e9fdc86..601c2b7 100644
>> --- a/gcc/alloc-pool.c
>> +++ b/gcc/alloc-pool.c
>> @@ -26,70 +26,14 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "hash-map.h"
>>  
>>  ALLOC_POOL_ID_TYPE last_id;
>> -
>> -/* Hashtable mapping alloc_pool names to descriptors.  */
>> -hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
>> -
>> -struct alloc_pool_descriptor *
>> -allocate_pool_descriptor (const char *name)
>> -{
>> -  if (!alloc_pool_hash)
>> -    alloc_pool_hash = new hash_map<const char *, alloc_pool_descriptor> (10,
>> -									 false,
>> -									 false);
>> -
>> -  return &alloc_pool_hash->get_or_insert (name);
>> -}
>> -
>> -/* Output per-alloc_pool statistics.  */
>> -
>> -/* Used to accumulate statistics about alloc_pool sizes.  */
>> -struct pool_output_info
>> -{
>> -  unsigned long total_created;
>> -  unsigned long total_allocated;
>> -};
>> -
>> -/* Called via hash_map.traverse.  Output alloc_pool descriptor pointed out by
>> -   SLOT and update statistics.  */
>> -bool
>> -print_alloc_pool_statistics (const char *const &name,
>> -			     const alloc_pool_descriptor &d,
>> -			     struct pool_output_info *i)
>> -{
>> -  if (d.allocated)
>> -    {
>> -      fprintf (stderr,
>> -	       "%-22s %6d %10lu %10lu(%10lu) %10lu(%10lu) %10lu(%10lu)\n",
>> -	       name, d.elt_size, d.created, d.allocated,
>> -	       d.allocated / d.elt_size, d.peak, d.peak / d.elt_size,
>> -	       d.current, d.current / d.elt_size);
>> -      i->total_allocated += d.allocated;
>> -      i->total_created += d.created;
>> -    }
>> -  return 1;
>> -}
>> +mem_alloc_description<pool_usage> pool_allocator_usage;
>>  
>>  /* Output per-alloc_pool memory usage statistics.  */
>>  void
>>  dump_alloc_pool_statistics (void)
>>  {
>> -  struct pool_output_info info;
>> -
>>    if (! GATHER_STATISTICS)
>>      return;
>>  
>> -  if (!alloc_pool_hash)
>> -    return;
>> -
>> -  fprintf (stderr, "\nAlloc-pool Kind         Elt size  Pools  Allocated (elts)            Peak (elts)            Leak (elts)\n");
>> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
>> -  info.total_created = 0;
>> -  info.total_allocated = 0;
>> -  alloc_pool_hash->traverse <struct pool_output_info *,
>> -			     print_alloc_pool_statistics> (&info);
>> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
>> -  fprintf (stderr, "%-22s           %7lu %10lu\n",
>> -	   "Total", info.total_created, info.total_allocated);
>> -  fprintf (stderr, "--------------------------------------------------------------------------------------------------------------\n");
>> +  pool_allocator_usage.dump (ALLOC_POOL);
>>  }
>> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
>> index 96a1342..a1727ce 100644
>> --- a/gcc/alloc-pool.h
>> +++ b/gcc/alloc-pool.h
>> @@ -26,6 +26,71 @@ extern void dump_alloc_pool_statistics (void);
>>  
>>  typedef unsigned long ALLOC_POOL_ID_TYPE;
>>  
>> +/* Pool allocator memory usage.  */
>> +struct pool_usage: public mem_usage
>> +{
>> +  /* Default contructor.  */
>> +  pool_usage (): m_element_size (0), m_pool_name ("") {}
>> +  /* Constructor.  */
>> +  pool_usage (size_t allocated, size_t times, size_t peak,
>> +	      size_t instances, size_t element_size,
>> +	      const char *pool_name)
>> +    : mem_usage (allocated, times, peak, instances),
>> +      m_element_size (element_size),
>> +      m_pool_name (pool_name) {}
>> +
>> +  /* Sum the usage with SECOND usage.  */
>> +  pool_usage operator+ (const pool_usage &second)
>> +  {
>> +    return pool_usage (m_allocated + second.m_allocated,
>> +			     m_times + second.m_times,
>> +			     m_peak + second.m_peak,
>> +			     m_instances + second.m_instances,
>> +			     m_element_size, m_pool_name);
>> +  }
>> +
>> +  /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
>> +  inline void dump (mem_location *loc, mem_usage &total) const
>> +  {
>> +    char s[4096];
>> +    sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (),
>> +	     loc->m_line, loc->m_function);
>> +
>> +    s[48] = '\0';
>> +
>> +    fprintf (stderr, "%-32s%-48s %6li%10li:%5.1f%%%10li%10li:%5.1f%%%12li\n",
>> +	     m_pool_name, s, (long)m_instances,
>> +	     (long)m_allocated, get_percent (m_allocated, total.m_allocated),
>> +	     (long)m_peak, (long)m_times,
>> +	     get_percent (m_times, total.m_times),
>> +	     (long)m_element_size);
>> +  }
>> +
>> +  /* Dump header with NAME.  */
>> +  static inline void dump_header (const char *name)
>> +  {
>> +    fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name,
>> +	     "Pools", "Leak", "Peak", "Times", "Elt size");
>> +    print_dash_line ();
>> +  }
>> +
>> +  /* Dump footer.  */
>> +  inline void dump_footer ()
>> +  {
>> +    print_dash_line ();
>> +    fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances,
>> +	     (long)m_allocated);
>> +    print_dash_line ();
>> +  }
>> +
>> +  /* Element size.  */
>> +  size_t m_element_size;
>> +  /* Pool name.  */
>> +  const char *m_pool_name;
>> +};
>> +
>> +extern mem_alloc_description<pool_usage> pool_allocator_usage;
>> +
>>  /* Type based memory pool allocator.  */
>>  template <typename T>
>>  class pool_allocator
>> @@ -35,7 +100,7 @@ public:
>>       has NUM elements.  The allocator support EXTRA_SIZE and can
>>       potentially IGNORE_TYPE_SIZE.  */
>>    pool_allocator (const char *name, size_t num, size_t extra_size = 0,
>> -		  bool ignore_type_size = false);
>> +		  bool ignore_type_size = false CXX_MEM_STAT_INFO);
>>    ~pool_allocator ();
>>    void release ();
>>    void release_if_empty ();
>> @@ -122,6 +187,8 @@ private:
>>    size_t m_extra_size;
>>    /* Flag if a pool allocator is initialized.  */
>>    bool m_initialized;
>> +  /* Memory allocation location.  */
>> +  mem_location m_location;
>>  };
>>  
>>  /* Last used ID.  */
>> @@ -151,19 +218,16 @@ struct alloc_pool_descriptor
>>  /* Hashtable mapping alloc_pool names to descriptors.  */
>>  extern hash_map<const char *, alloc_pool_descriptor> *alloc_pool_hash;
>>  
>> -/* For given name, return descriptor, create new if needed.  */
>> -alloc_pool_descriptor *
>> -allocate_pool_descriptor (const char *name);
>> -
>>  template <typename T>
>>  inline
>>  pool_allocator<T>::pool_allocator (const char *name, size_t num,
>> -				   size_t extra_size, bool ignore_type_size):
>> +				   size_t extra_size, bool ignore_type_size
>> +				   MEM_STAT_DECL):
>>    m_name (name), m_elts_per_block (num), m_returned_free_list (NULL),
>>    m_virgin_free_list (NULL), m_virgin_elts_remaining (0), m_elts_allocated (0),
>>    m_elts_free (0), m_blocks_allocated (0), m_block_list (NULL),
>>    m_ignore_type_size (ignore_type_size), m_extra_size (extra_size),
>> -  m_initialized (false) {}
>> +  m_initialized (false), m_location (ALLOC_POOL, false PASS_MEM_STAT) {}
>>  
>>  /* Initialize a pool allocator.  */
>>  
>> @@ -196,9 +260,11 @@ pool_allocator<T>::initialize ()
>>  
>>    if (GATHER_STATISTICS)
>>      {
>> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
>> -      desc->elt_size = size;
>> -      desc->created++;
>> +      pool_usage *u = pool_allocator_usage.register_descriptor
>> +	(this, new mem_location (m_location));
>> +
>> +      u->m_element_size = m_elt_size;
>> +      u->m_pool_name = m_name;
>>      }
>>  
>>    /* List header size should be a multiple of 8.  */
>> @@ -235,10 +301,10 @@ pool_allocator<T>::release ()
>>        free (block);
>>      }
>>  
>> -  if (GATHER_STATISTICS && false)
>> +  if (GATHER_STATISTICS)
>>      {
>> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
>> -      desc->current -= (m_elts_allocated - m_elts_free) * m_elt_size;
>> +      pool_allocator_usage.release_instance_overhead (this,
>> +	(m_elts_allocated - m_elts_free) * m_elt_size);
>>      }
>>  
>>    m_returned_free_list = NULL;
>> @@ -279,12 +345,7 @@ pool_allocator<T>::allocate ()
>>  
>>    if (GATHER_STATISTICS)
>>      {
>> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
>> -
>> -      desc->allocated += m_elt_size;
>> -      desc->current += m_elt_size;
>> -      if (desc->peak < desc->current)
>> -	desc->peak = desc->current;
>> +      pool_allocator_usage.register_instance_overhead (m_elt_size, this);
>>      }
>>  
>>  #ifdef ENABLE_VALGRIND_ANNOTATIONS
>> @@ -383,8 +444,7 @@ pool_allocator<T>::remove (T *object)
>>  
>>    if (GATHER_STATISTICS)
>>      {
>> -      alloc_pool_descriptor *desc = allocate_pool_descriptor (m_name);
>> -      desc->current -= m_elt_size;
>> +      pool_allocator_usage.release_instance_overhead (this, m_elt_size);
>>      }
>>  }
>>  
>> diff --git a/gcc/mem-stats-traits.h b/gcc/mem-stats-traits.h
>> index de1614e..f7843f2 100644
>> --- a/gcc/mem-stats-traits.h
>> +++ b/gcc/mem-stats-traits.h
>> @@ -10,11 +10,12 @@ enum mem_alloc_origin
>>    VEC,
>>    BITMAP,
>>    GGC,
>> +  ALLOC_POOL,
>>    MEM_ALLOC_ORIGIN_LENGTH
>>  };
>>  
>>  /* Verbose names of the memory allocation origin.  */
>>  static const char * mem_alloc_origin_names[] = { "Hash tables", "Hash maps",
>> -  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory" };
>> +  "Hash sets", "Heap vectors", "Bitmaps", "GGC memory", "Allocation pool" };
>>  
>>  #endif // GCC_MEM_STATS_TRAITS_H
>> diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
>> index ac47231..fea68fa 100644
>> --- a/gcc/mem-stats.h
>> +++ b/gcc/mem-stats.h
>> @@ -18,11 +18,16 @@ struct mem_location
>>    inline mem_location () {}
>>  
>>    /* Constructor.  */
>> -  inline mem_location (const char *filename, const char *function, int line,
>> -		mem_alloc_origin origin, bool ggc):
>> +  inline mem_location (mem_alloc_origin origin, bool ggc,
>> +		const char *filename = NULL, int line = 0, const char *function = NULL):
>>      m_filename (filename), m_function (function), m_line (line), m_origin
>>      (origin), m_ggc (ggc) {}
>>  
>> +  /* Copy constructor.  */
>> +  inline mem_location (mem_location &other): m_filename (other.m_filename),
>> +    m_function (other.m_function), m_line (other.m_line),
>> +    m_origin (other.m_origin), m_ggc (other.m_ggc) {}
>> +
>>    /* Compute hash value based on file name, function name and line in
>>       source code. As there is just a single pointer registered for every
>>       constant that points to e.g. the same file name, we can use hash
>> @@ -79,11 +84,12 @@ struct mem_location
>>  struct mem_usage
>>  {
>>    /* Default constructor.  */
>> -  mem_usage (): m_allocated (0), m_times (0), m_peak (0) {}
>> +  mem_usage (): m_allocated (0), m_times (0), m_peak (0), m_instances (1) {}
>>  
>>    /* Constructor.  */
>> -  mem_usage (size_t allocated, size_t times, size_t peak):
>> -    m_allocated (allocated), m_times (times), m_peak (peak) {}
>> +  mem_usage (size_t allocated, size_t times, size_t peak, size_t instances = 0):
>> +    m_allocated (allocated), m_times (times), m_peak (peak),
>> +    m_instances (instances) {}
>>  
>>    /* Register overhead of SIZE bytes.  */
>>    inline void register_overhead (size_t size)
>> @@ -108,7 +114,8 @@ struct mem_usage
>>    {
>>      return mem_usage (m_allocated + second.m_allocated,
>>  		      m_times + second.m_times,
>> -		      m_peak + second.m_peak);
>> +		      m_peak + second.m_peak,
>> +		      m_instances + second.m_instances);
>>    }
>>  
>>    /* Comparison operator.  */
>> @@ -163,7 +170,7 @@ struct mem_usage
>>    /* Print line made of dashes.  */
>>    static inline void print_dash_line ()
>>    {
>> -    fprintf (stderr, "%s\n", std::string (128, '-').c_str ());
>> +    fprintf (stderr, "%s\n", std::string (140, '-').c_str ());
>>    }
>>  
>>    /* Dump header with NAME.  */
>> @@ -180,6 +187,8 @@ struct mem_usage
>>    size_t m_times;
>>    /* Peak allocation in bytes.  */
>>    size_t m_peak;
>> +  /* Number of container instances.  */
>> +  size_t m_instances;
>>  };
>>  
>>  /* Memory usage pair that connectes memory usage and number
>> @@ -241,9 +250,13 @@ public:
>>    /* Return descriptor for instance PTR.  */
>>    T *get_descriptor_for_instance (const void *ptr);
>>  
>> -  /* Register memory allocation descriptor for container PTR. ORIGIN identifies
>> +  /* Register memory allocation descriptor for container PTR which is
>> +     described by a memory LOCATION.  */
>> +  T *register_descriptor (const void *ptr, mem_location *location);
>> +
>> +  /* Register memory allocation descriptor for container PTR.  ORIGIN identifies
>>       type of container and GGC identifes if the allocation is handled in GGC
>> -     memory. Each location is identified by file NAME, LINE in source code and
>> +     memory.  Each location is identified by file NAME, LINE in source code and
>>       FUNCTION name.  */
>>    T *register_descriptor (const void *ptr, mem_alloc_origin origin,
>>  			  bool ggc, const char *name, int line,
>> @@ -321,33 +334,27 @@ mem_alloc_description<T>::get_descriptor_for_instance (const void *ptr)
>>    return m_reverse_map->get (ptr) ? (*m_reverse_map->get (ptr)).usage : NULL;
>>  }
>>  
>> -/* Register memory allocation descriptor for container PTR. ORIGIN identifies
>> -   type of container and GGC identifes if the allocation is handled in GGC
>> -   memory. Each location is identified by file NAME, LINE in source code and
>> -   FUNCTION name.  */
>>  
>> +  /* Register memory allocation descriptor for container PTR which is
>> +     described by a memory LOCATION.  */
>>  template <class T>
>>  inline T*
>>  mem_alloc_description<T>::register_descriptor (const void *ptr,
>> -					       mem_alloc_origin origin,
>> -					       bool ggc,
>> -					       const char *filename,
>> -					       int line,
>> -					       const char *function)
>> +					       mem_location *location)
>>  {
>> -  mem_location *l = new mem_location (filename, function, line, origin, ggc);
>>    T *usage = NULL;
>>  
>> -  T **slot = m_map->get (l);
>> +  T **slot = m_map->get (location);
>>    if (slot)
>>      {
>> -      delete l;
>> +      delete location;
>>        usage = *slot;
>> +      usage->m_instances++;
>>      }
>>    else
>>      {
>>        usage = new T ();
>> -      m_map->put (l, usage);
>> +      m_map->put (location, usage);
>>      }
>>  
>>    if (!m_reverse_map->get (ptr))
>> @@ -356,6 +363,24 @@ mem_alloc_description<T>::register_descriptor (const void *ptr,
>>    return usage;
>>  }
>>  
>> +/* Register memory allocation descriptor for container PTR.  ORIGIN identifies
>> +   type of container and GGC identifes if the allocation is handled in GGC
>> +   memory.  Each location is identified by file NAME, LINE in source code and
>> +   FUNCTION name.  */
>> +
>> +template <class T>
>> +inline T*
>> +mem_alloc_description<T>::register_descriptor (const void *ptr,
>> +					       mem_alloc_origin origin,
>> +					       bool ggc,
>> +					       const char *filename,
>> +					       int line,
>> +					       const char *function)
>> +{
>> +  mem_location *l = new mem_location (origin, ggc, filename, line, function);
>> +  return register_descriptor (ptr, l);
>> +}
>> +
>>  /* Register instance overhead identified by PTR pointer. Allocation takes
>>     SIZE bytes.  */
>>  
>>
> 
> Hello.
> 
> There's a small patch that fixes ICE if a compiler is not built with --enable-gather-mem-stats
> and we append -fmem-report to a command line options.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready for trunk?
> Thanks,
> Martin
> 

Honza approved the patch. I'm going to install the patch.

Martin

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

end of thread, other threads:[~2015-06-16 13:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-02 11:14 [PATCH 1/2] Memory statistics enhancement mliska
2015-06-02 11:18 ` [PATCH 2/2] Fix memory report layout at various places mliska
2015-06-02 14:11   ` Jeff Law
2015-06-02 14:00 ` [PATCH 1/2] Memory statistics enhancement Jeff Law
2015-06-02 15:38   ` Martin Liška
2015-06-02 17:33     ` Jeff Law
2015-06-03  9:16       ` Martin Liška
2015-06-03 13:19         ` Jeff Law
2015-06-03 14:27           ` Martin Liška
2015-06-03 17:32             ` Jeff Law
2015-06-03  9:47 ` Martin Liška
2015-06-03 13:27   ` Jeff Law
2015-06-08 15:02 ` Martin Liška
2015-06-16 13:18   ` Martin Liška

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).