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 <mli...@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
