Re: [PATCH 5/6] Add line map statistics to -fmem-report output

[Gabriel Dos Reis <gdr@integrable-solutions.net>]

On Fri, Dec 10, 2010 at 5:11 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> This patch adds statistics about the memory consumption of line maps
> to the output of -fmem-report. It has been useful in trying to reduce
> the memory consumption of the macro maps support.
>
> Tested on x86_64-unknown-linux-gnu against trunk.
>
> gcc/
>        * input.c (SCALE, STAT_LABEL, FORMAT_AMOUNT): New macros.
>        (dump_line_table_statistics): Define new function.
>        * input.h (dump_line_table_statistics): Declare new function.
>        * toplev.c (dump_memory_report): Call dump_line_table_statistics.

Can we give these `1024' some meaningfull symbolic names?
SCALE is a bit a vague -- one has to look into its body to
understand what it is doing, which defeats the purpose of abstraction.
Also, those macro should be documented.

Finally, this:

> +  linemap_get_statistics (line_table,
> +                         &num_ordinary_maps_allocated,
> +                         &num_ordinary_maps_used,
> +                         &ordinary_maps_allocated_size,
> +                         &ordinary_maps_used_size,
> +                         &num_macro_maps_used,
> +                         &macro_maps_used_size,
> +                         &macro_maps_locations_size,
> +                         &duplicated_maps_locations_size,
> +                         &total_allocated_map_size,
> +                         &total_used_map_size);

is a too impressive paramater list :-)
Could you use a structure to package all monster?

> +void linemap_get_statistics (struct line_maps *set,
> +                            size_t *, size_t *,
> +                            size_t *, size_t *,
> +                            size_t *, size_t *,
> +                            size_t *, size_t *,
> +                            size_t *, size_t *);

same here.