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 > > * 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 *alloc_pool_hash; > - > -struct alloc_pool_descriptor * > -allocate_pool_descriptor (const char *name) > -{ > - if (!alloc_pool_hash) > - alloc_pool_hash = new hash_map (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_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 - 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_allocator_usage; > + > /* Type based memory pool allocator. */ > template > 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 *alloc_pool_hash; > > -/* For given name, return descriptor, create new if needed. */ > -alloc_pool_descriptor * > -allocate_pool_descriptor (const char *name); > - > template > inline > pool_allocator::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::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::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::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::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::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 > inline T* > mem_alloc_description::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::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 > +inline T* > +mem_alloc_description::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