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