diff mbox series

[3/4] Fix vector memory statistics.

Message ID ff52bdfe70b1ab3ea9e53a4fde33c34c158190f6.1541405092.git.mliska@suse.cz
State New
Headers show
Series Enhance and fix various issues in -fmem-report | expand

Commit Message

Martin Liška Nov. 2, 2018, 11:52 a.m. UTC 
gcc/ChangeLog:

2018-11-02  Martin Liska  <mliska@suse.cz>

	* mem-stats.h (mem_alloc_description::release_instance_overhead):
	Return T *.
	* vec.c (struct vec_usage): Register m_element_size.
	(vec_prefix::register_overhead): New arguments: elements and
	element_size.
	(vec_prefix::release_overhead): Subtract elements.
	* vec.h (struct vec_prefix): Change signature.
	(va_heap::reserve): Pass proper arguments.
	(va_heap::release): Likewise.
---
 gcc/mem-stats.h | 14 ++++++++------
 gcc/vec.c       | 34 +++++++++++++++++++++-------------
 gcc/vec.h       | 12 ++++++++----
 3 files changed, 37 insertions(+), 23 deletions(-)

Comments

Richard Biener Nov. 5, 2018, 9:56 a.m. UTC | #1 
On Mon, Nov 5, 2018 at 9:07 AM marxin <mliska@suse.cz> wrote:
>
>
> gcc/ChangeLog:

   /* 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,
-                                 bool remove_from_map = false);
+  T * release_instance_overhead (void *ptr, size_t size,
+                                bool remove_from_map = false);

can you document the return value?

Otherwise OK.

Richard.

> 2018-11-02  Martin Liska  <mliska@suse.cz>
>
>         * mem-stats.h (mem_alloc_description::release_instance_overhead):
>         Return T *.
>         * vec.c (struct vec_usage): Register m_element_size.
>         (vec_prefix::register_overhead): New arguments: elements and
>         element_size.
>         (vec_prefix::release_overhead): Subtract elements.
>         * vec.h (struct vec_prefix): Change signature.
>         (va_heap::reserve): Pass proper arguments.
>         (va_heap::release): Likewise.
> ---
>  gcc/mem-stats.h | 14 ++++++++------
>  gcc/vec.c       | 34 +++++++++++++++++++++-------------
>  gcc/vec.h       | 12 ++++++++----
>  3 files changed, 37 insertions(+), 23 deletions(-)
>
Martin Liška Nov. 5, 2018, 12:17 p.m. UTC | #2 
On 11/5/18 10:56 AM, Richard Biener wrote:
> On Mon, Nov 5, 2018 at 9:07 AM marxin <mliska@suse.cz> wrote:
>>
>>
>> gcc/ChangeLog:
> 
>    /* 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,
> -                                 bool remove_from_map = false);
> +  T * release_instance_overhead (void *ptr, size_t size,
> +                                bool remove_from_map = false);
> 
> can you document the return value?

Sure, fixed in attached patch.

Martin

> 
> Otherwise OK.
> 
> Richard.
> 
>> 2018-11-02  Martin Liska  <mliska@suse.cz>
>>
>>         * mem-stats.h (mem_alloc_description::release_instance_overhead):
>>         Return T *.
>>         * vec.c (struct vec_usage): Register m_element_size.
>>         (vec_prefix::register_overhead): New arguments: elements and
>>         element_size.
>>         (vec_prefix::release_overhead): Subtract elements.
>>         * vec.h (struct vec_prefix): Change signature.
>>         (va_heap::reserve): Pass proper arguments.
>>         (va_heap::release): Likewise.
>> ---
>>  gcc/mem-stats.h | 14 ++++++++------
>>  gcc/vec.c       | 34 +++++++++++++++++++++-------------
>>  gcc/vec.h       | 12 ++++++++----
>>  3 files changed, 37 insertions(+), 23 deletions(-)
>>
From ff75c10bc06eb9c172188cb49863cd78fd0540b7 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 2 Nov 2018 12:52:28 +0100
Subject: [PATCH 3/4] Fix vector memory statistics.

gcc/ChangeLog:

2018-11-02  Martin Liska  <mliska@suse.cz>

	* mem-stats.h (mem_alloc_description::release_instance_overhead):
	Return T *.
	* vec.c (struct vec_usage): Register m_element_size.
	(vec_prefix::register_overhead): New arguments: elements and
	element_size.
	(vec_prefix::release_overhead): Subtract elements.
	* vec.h (struct vec_prefix): Change signature.
	(va_heap::reserve): Pass proper arguments.
	(va_heap::release): Likewise.
---
 gcc/mem-stats.h | 17 ++++++++++-------
 gcc/vec.c       | 34 +++++++++++++++++++++-------------
 gcc/vec.h       | 12 ++++++++----
 3 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index 3ef6d53dfa6..b7f7e06a1c7 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -340,9 +340,10 @@ public:
   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,
-				  bool remove_from_map = false);
+     remove the instance from reverse map.  Return memory usage that belongs
+     to this memory description.  */
+  T * 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);
@@ -503,7 +504,7 @@ mem_alloc_description<T>::register_overhead (size_t size,
 /* Release PTR pointer of SIZE bytes.  */
 
 template <class T>
-inline void
+inline T *
 mem_alloc_description<T>::release_instance_overhead (void *ptr, size_t size,
 						     bool remove_from_map)
 {
@@ -512,14 +513,16 @@ mem_alloc_description<T>::release_instance_overhead (void *ptr, size_t size,
   if (!slot)
     {
       /* Due to PCH, it can really happen.  */
-      return;
+      return NULL;
     }
 
-  mem_usage_pair<T> usage_pair = *slot;
-  usage_pair.usage->release_overhead (size);
+  T *usage = (*slot).usage;
+  usage->release_overhead (size);
 
   if (remove_from_map)
     m_reverse_map->remove (ptr);
+
+  return usage;
 }
 
 /* Release intance object identified by PTR pointer.  */
diff --git a/gcc/vec.c b/gcc/vec.c
index ff2456aead9..bfd52856e46 100644
--- a/gcc/vec.c
+++ b/gcc/vec.c
@@ -52,13 +52,14 @@ vnull vNULL;
 struct vec_usage: public mem_usage
 {
   /* Default constructor.  */
-  vec_usage (): m_items (0), m_items_peak (0) {}
+  vec_usage (): m_items (0), m_items_peak (0), m_element_size (0) {}
 
   /* Constructor.  */
   vec_usage (size_t allocated, size_t times, size_t peak,
-	     size_t items, size_t items_peak)
+	     size_t items, size_t items_peak, size_t element_size)
     : mem_usage (allocated, times, peak),
-    m_items (items), m_items_peak (items_peak) {}
+    m_items (items), m_items_peak (items_peak),
+    m_element_size (element_size) {}
 
   /* Sum the usage with SECOND usage.  */
   vec_usage
@@ -68,7 +69,7 @@ struct vec_usage: public mem_usage
 		      m_times + second.m_times,
 		      m_peak + second.m_peak,
 		      m_items + second.m_items,
-		      m_items_peak + second.m_items_peak);
+		      m_items_peak + second.m_items_peak, 0);
   }
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
@@ -81,7 +82,8 @@ struct vec_usage: public mem_usage
 
     s[48] = '\0';
 
-    fprintf (stderr, "%-48s %10li:%4.1f%%%10li%10li:%4.1f%%%11li%11li\n", s,
+    fprintf (stderr, "%-48s %10li%11li:%4.1f%%%10li%10li:%4.1f%%%11li%11li\n", s,
+	     (long)m_element_size,
 	     (long)m_allocated, m_allocated * 100.0 / total.m_allocated,
 	     (long)m_peak, (long)m_times, m_times * 100.0 / total.m_times,
 	     (long)m_items, (long)m_items_peak);
@@ -101,8 +103,8 @@ struct vec_usage: public mem_usage
   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");
+    fprintf (stderr, "%-48s %10s%11s%16s%10s%17s%11s\n", name, "sizeof(T)",
+	     "Leak", "Peak", "Times", "Leak items", "Peak items");
     print_dash_line ();
   }
 
@@ -110,6 +112,8 @@ struct vec_usage: public mem_usage
   size_t m_items;
   /* Peak value of number of allocated items.  */
   size_t m_items_peak;
+  /* Size of element of the vector.  */
+  size_t m_element_size;
 };
 
 /* Vector memory description.  */
@@ -118,12 +122,14 @@ static mem_alloc_description <vec_usage> vec_mem_desc;
 /* Account the overhead.  */
 
 void
-vec_prefix::register_overhead (void *ptr, size_t size, size_t elements
-			       MEM_STAT_DECL)
+vec_prefix::register_overhead (void *ptr, size_t elements,
+			       size_t element_size MEM_STAT_DECL)
 {
   vec_mem_desc.register_descriptor (ptr, VEC_ORIGIN, false
 				    FINAL_PASS_MEM_STAT);
-  vec_usage *usage = vec_mem_desc.register_instance_overhead (size, ptr);
+  vec_usage *usage
+    = vec_mem_desc.register_instance_overhead (elements * element_size, ptr);
+  usage->m_element_size = element_size;
   usage->m_items += elements;
   if (usage->m_items_peak < usage->m_items)
     usage->m_items_peak = usage->m_items;
@@ -132,13 +138,15 @@ vec_prefix::register_overhead (void *ptr, size_t size, size_t elements
 /* Notice that the memory allocated for the vector has been freed.  */
 
 void
-vec_prefix::release_overhead (void *ptr, size_t size, bool in_dtor
-			      MEM_STAT_DECL)
+vec_prefix::release_overhead (void *ptr, size_t size, size_t elements,
+			      bool in_dtor MEM_STAT_DECL)
 {
   if (!vec_mem_desc.contains_descriptor_for_instance (ptr))
     vec_mem_desc.register_descriptor (ptr, VEC_ORIGIN,
 				      false FINAL_PASS_MEM_STAT);
-  vec_mem_desc.release_instance_overhead (ptr, size, in_dtor);
+  vec_usage *usage = vec_mem_desc.release_instance_overhead (ptr, size,
+							     in_dtor);
+  usage->m_items -= elements;
 }
 
 /* Calculate the number of slots to reserve a vector, making sure that
diff --git a/gcc/vec.h b/gcc/vec.h
index 0af5187782e..f8c039754d2 100644
--- a/gcc/vec.h
+++ b/gcc/vec.h
@@ -195,7 +195,7 @@ struct vec_prefix
 
   /* Memory allocation support routines in vec.c.  */
   void register_overhead (void *, size_t, size_t CXX_MEM_STAT_INFO);
-  void release_overhead (void *, size_t, bool CXX_MEM_STAT_INFO);
+  void release_overhead (void *, size_t, size_t, bool CXX_MEM_STAT_INFO);
   static unsigned calculate_allocation (vec_prefix *, unsigned, bool);
   static unsigned calculate_allocation_1 (unsigned, unsigned);
 
@@ -276,12 +276,14 @@ inline void
 va_heap::reserve (vec<T, va_heap, vl_embed> *&v, unsigned reserve, bool exact
 		  MEM_STAT_DECL)
 {
+  size_t elt_size = sizeof (T);
   unsigned alloc
     = vec_prefix::calculate_allocation (v ? &v->m_vecpfx : 0, reserve, exact);
   gcc_checking_assert (alloc);
 
   if (GATHER_STATISTICS && v)
-    v->m_vecpfx.release_overhead (v, v->allocated (), false);
+    v->m_vecpfx.release_overhead (v, elt_size * v->allocated (),
+				  v->allocated (), false);
 
   size_t size = vec<T, va_heap, vl_embed>::embedded_size (alloc);
   unsigned nelem = v ? v->length () : 0;
@@ -289,7 +291,7 @@ va_heap::reserve (vec<T, va_heap, vl_embed> *&v, unsigned reserve, bool exact
   v->embedded_init (alloc, nelem);
 
   if (GATHER_STATISTICS)
-    v->m_vecpfx.register_overhead (v, alloc, nelem PASS_MEM_STAT);
+    v->m_vecpfx.register_overhead (v, alloc, elt_size PASS_MEM_STAT);
 }
 
 
@@ -299,11 +301,13 @@ template<typename T>
 void
 va_heap::release (vec<T, va_heap, vl_embed> *&v)
 {
+  size_t elt_size = sizeof (T);
   if (v == NULL)
     return;
 
   if (GATHER_STATISTICS)
-    v->m_vecpfx.release_overhead (v, v->allocated (), true);
+    v->m_vecpfx.release_overhead (v, elt_size * v->allocated (),
+				  v->allocated (), true);
   ::free (v);
   v = NULL;
 }
Richard Biener Nov. 5, 2018, 12:45 p.m. UTC | #3 
On Mon, Nov 5, 2018 at 1:17 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/5/18 10:56 AM, Richard Biener wrote:
> > On Mon, Nov 5, 2018 at 9:07 AM marxin <mliska@suse.cz> wrote:
> >>
> >>
> >> gcc/ChangeLog:
> >
> >    /* 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,
> > -                                 bool remove_from_map = false);
> > +  T * release_instance_overhead (void *ptr, size_t size,
> > +                                bool remove_from_map = false);
> >
> > can you document the return value?
>
> Sure, fixed in attached patch.

OK.

> Martin
>
> >
> > Otherwise OK.
> >
> > Richard.
> >
> >> 2018-11-02  Martin Liska  <mliska@suse.cz>
> >>
> >>         * mem-stats.h (mem_alloc_description::release_instance_overhead):
> >>         Return T *.
> >>         * vec.c (struct vec_usage): Register m_element_size.
> >>         (vec_prefix::register_overhead): New arguments: elements and
> >>         element_size.
> >>         (vec_prefix::release_overhead): Subtract elements.
> >>         * vec.h (struct vec_prefix): Change signature.
> >>         (va_heap::reserve): Pass proper arguments.
> >>         (va_heap::release): Likewise.
> >> ---
> >>  gcc/mem-stats.h | 14 ++++++++------
> >>  gcc/vec.c       | 34 +++++++++++++++++++++-------------
> >>  gcc/vec.h       | 12 ++++++++----
> >>  3 files changed, 37 insertions(+), 23 deletions(-)
> >>
>
diff mbox series

Patch

diff --git a/gcc/mem-stats.h b/gcc/mem-stats.h
index 3ef6d53dfa6..860908cf585 100644
--- a/gcc/mem-stats.h
+++ b/gcc/mem-stats.h
@@ -341,8 +341,8 @@  public:
 
   /* 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,
-				  bool remove_from_map = false);
+  T * 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);
@@ -503,7 +503,7 @@  mem_alloc_description<T>::register_overhead (size_t size,
 /* Release PTR pointer of SIZE bytes.  */
 
 template <class T>
-inline void
+inline T *
 mem_alloc_description<T>::release_instance_overhead (void *ptr, size_t size,
 						     bool remove_from_map)
 {
@@ -512,14 +512,16 @@  mem_alloc_description<T>::release_instance_overhead (void *ptr, size_t size,
   if (!slot)
     {
       /* Due to PCH, it can really happen.  */
-      return;
+      return NULL;
     }
 
-  mem_usage_pair<T> usage_pair = *slot;
-  usage_pair.usage->release_overhead (size);
+  T *usage = (*slot).usage;
+  usage->release_overhead (size);
 
   if (remove_from_map)
     m_reverse_map->remove (ptr);
+
+  return usage;
 }
 
 /* Release intance object identified by PTR pointer.  */
diff --git a/gcc/vec.c b/gcc/vec.c
index ff2456aead9..bfd52856e46 100644
--- a/gcc/vec.c
+++ b/gcc/vec.c
@@ -52,13 +52,14 @@  vnull vNULL;
 struct vec_usage: public mem_usage
 {
   /* Default constructor.  */
-  vec_usage (): m_items (0), m_items_peak (0) {}
+  vec_usage (): m_items (0), m_items_peak (0), m_element_size (0) {}
 
   /* Constructor.  */
   vec_usage (size_t allocated, size_t times, size_t peak,
-	     size_t items, size_t items_peak)
+	     size_t items, size_t items_peak, size_t element_size)
     : mem_usage (allocated, times, peak),
-    m_items (items), m_items_peak (items_peak) {}
+    m_items (items), m_items_peak (items_peak),
+    m_element_size (element_size) {}
 
   /* Sum the usage with SECOND usage.  */
   vec_usage
@@ -68,7 +69,7 @@  struct vec_usage: public mem_usage
 		      m_times + second.m_times,
 		      m_peak + second.m_peak,
 		      m_items + second.m_items,
-		      m_items_peak + second.m_items_peak);
+		      m_items_peak + second.m_items_peak, 0);
   }
 
   /* Dump usage coupled to LOC location, where TOTAL is sum of all rows.  */
@@ -81,7 +82,8 @@  struct vec_usage: public mem_usage
 
     s[48] = '\0';
 
-    fprintf (stderr, "%-48s %10li:%4.1f%%%10li%10li:%4.1f%%%11li%11li\n", s,
+    fprintf (stderr, "%-48s %10li%11li:%4.1f%%%10li%10li:%4.1f%%%11li%11li\n", s,
+	     (long)m_element_size,
 	     (long)m_allocated, m_allocated * 100.0 / total.m_allocated,
 	     (long)m_peak, (long)m_times, m_times * 100.0 / total.m_times,
 	     (long)m_items, (long)m_items_peak);
@@ -101,8 +103,8 @@  struct vec_usage: public mem_usage
   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");
+    fprintf (stderr, "%-48s %10s%11s%16s%10s%17s%11s\n", name, "sizeof(T)",
+	     "Leak", "Peak", "Times", "Leak items", "Peak items");
     print_dash_line ();
   }
 
@@ -110,6 +112,8 @@  struct vec_usage: public mem_usage
   size_t m_items;
   /* Peak value of number of allocated items.  */
   size_t m_items_peak;
+  /* Size of element of the vector.  */
+  size_t m_element_size;
 };
 
 /* Vector memory description.  */
@@ -118,12 +122,14 @@  static mem_alloc_description <vec_usage> vec_mem_desc;
 /* Account the overhead.  */
 
 void
-vec_prefix::register_overhead (void *ptr, size_t size, size_t elements
-			       MEM_STAT_DECL)
+vec_prefix::register_overhead (void *ptr, size_t elements,
+			       size_t element_size MEM_STAT_DECL)
 {
   vec_mem_desc.register_descriptor (ptr, VEC_ORIGIN, false
 				    FINAL_PASS_MEM_STAT);
-  vec_usage *usage = vec_mem_desc.register_instance_overhead (size, ptr);
+  vec_usage *usage
+    = vec_mem_desc.register_instance_overhead (elements * element_size, ptr);
+  usage->m_element_size = element_size;
   usage->m_items += elements;
   if (usage->m_items_peak < usage->m_items)
     usage->m_items_peak = usage->m_items;
@@ -132,13 +138,15 @@  vec_prefix::register_overhead (void *ptr, size_t size, size_t elements
 /* Notice that the memory allocated for the vector has been freed.  */
 
 void
-vec_prefix::release_overhead (void *ptr, size_t size, bool in_dtor
-			      MEM_STAT_DECL)
+vec_prefix::release_overhead (void *ptr, size_t size, size_t elements,
+			      bool in_dtor MEM_STAT_DECL)
 {
   if (!vec_mem_desc.contains_descriptor_for_instance (ptr))
     vec_mem_desc.register_descriptor (ptr, VEC_ORIGIN,
 				      false FINAL_PASS_MEM_STAT);
-  vec_mem_desc.release_instance_overhead (ptr, size, in_dtor);
+  vec_usage *usage = vec_mem_desc.release_instance_overhead (ptr, size,
+							     in_dtor);
+  usage->m_items -= elements;
 }
 
 /* Calculate the number of slots to reserve a vector, making sure that
diff --git a/gcc/vec.h b/gcc/vec.h
index 0af5187782e..f8c039754d2 100644
--- a/gcc/vec.h
+++ b/gcc/vec.h
@@ -195,7 +195,7 @@  struct vec_prefix
 
   /* Memory allocation support routines in vec.c.  */
   void register_overhead (void *, size_t, size_t CXX_MEM_STAT_INFO);
-  void release_overhead (void *, size_t, bool CXX_MEM_STAT_INFO);
+  void release_overhead (void *, size_t, size_t, bool CXX_MEM_STAT_INFO);
   static unsigned calculate_allocation (vec_prefix *, unsigned, bool);
   static unsigned calculate_allocation_1 (unsigned, unsigned);
 
@@ -276,12 +276,14 @@  inline void
 va_heap::reserve (vec<T, va_heap, vl_embed> *&v, unsigned reserve, bool exact
 		  MEM_STAT_DECL)
 {
+  size_t elt_size = sizeof (T);
   unsigned alloc
     = vec_prefix::calculate_allocation (v ? &v->m_vecpfx : 0, reserve, exact);
   gcc_checking_assert (alloc);
 
   if (GATHER_STATISTICS && v)
-    v->m_vecpfx.release_overhead (v, v->allocated (), false);
+    v->m_vecpfx.release_overhead (v, elt_size * v->allocated (),
+				  v->allocated (), false);
 
   size_t size = vec<T, va_heap, vl_embed>::embedded_size (alloc);
   unsigned nelem = v ? v->length () : 0;
@@ -289,7 +291,7 @@  va_heap::reserve (vec<T, va_heap, vl_embed> *&v, unsigned reserve, bool exact
   v->embedded_init (alloc, nelem);
 
   if (GATHER_STATISTICS)
-    v->m_vecpfx.register_overhead (v, alloc, nelem PASS_MEM_STAT);
+    v->m_vecpfx.register_overhead (v, alloc, elt_size PASS_MEM_STAT);
 }
 
 
@@ -299,11 +301,13 @@  template<typename T>
 void
 va_heap::release (vec<T, va_heap, vl_embed> *&v)
 {
+  size_t elt_size = sizeof (T);
   if (v == NULL)
     return;
 
   if (GATHER_STATISTICS)
-    v->m_vecpfx.release_overhead (v, v->allocated (), true);
+    v->m_vecpfx.release_overhead (v, elt_size * v->allocated (),
+				  v->allocated (), true);
   ::free (v);
   v = NULL;
 }