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

[Dodji Seketeli <dodji@redhat.com>]

Hello,

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

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

Agreed.  Done.

I have one additional question, though.  I actually copied these macros
from tree-flow.h; there are copied all over the place, by the way.  I
guess it would be good to have them defined at only on place.  But
where?  Ideally it would be header suitable to included both by input.c
and pretty much everything else.  Would input.h be good enough?
everywhere.  I guess this would be a separate patch independent from
this macro location business.

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

Sure.  Done in the patch below.

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

Fixed in the patch below as well.

Thank you for the review, and sorry for my late reply.

-- 
		Dodji

    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.
    
    libcpp/
    	* line-map.h (linemap_get_statistics): Declare ...
    	* line-map.c (linemap_get_statistics):  ... new function.

diff --git a/gcc/input.c b/gcc/input.c
index 29e4de1..8701e4d 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -61,3 +61,74 @@ expand_location (source_location loc)
     }
   return xloc;
 }
+
+#define ONE_K 1024
+#define ONE_M ONE_K * ONE_K
+
+/* Display a number as an integer multiple of either:
+   - 1024, if said integer is >= to 10 K (in base 2)
+   - 1024 * 1024, if said integer is >= 10 M in (base 2)
+ */
+#define SCALE(x) ((unsigned long) ((x) < 10 * ONE_K \
+		  ? (x) \
+		  : ((x) < 10 * ONE_M \
+		     ? (x) / ONE_K \
+		     : (x) / (ONE_M))))
+
+/* For a given integer, display either:
+   - the character 'k', if the number is higher than 10 K (in base 2)
+     but strictly lower than 10 M (in base 2)
+   - the character 'M' if the number is higher than 10 M (in base2)
+   - the charcter ' ' if the number is strictly lower  than 10 K  */
+#define STAT_LABEL(x) ((x) < 10 * ONE_K ? ' ' : ((x) < 10 * ONE_M ? 'k' : 'M'))
+
+/* Display an integer amount as multiple of 1K or 1M (in base 2à).
+   Display the correct unit (either k, M, or ' ') after the amout, as
+   well.  */
+#define FORMAT_AMOUNT(size) SCALE (size), STAT_LABEL (size)
+
+/* Dump statistics to stderr about the memory usage of the line_table
+   set of line maps.  */
+
+void
+dump_line_table_statistics (void)
+{
+  struct linemap_stats s;
+
+  memset (&s, 0, sizeof (s));
+
+  linemap_get_statistics (line_table, &s);
+
+  fprintf (stderr, "\nLine Table allocations during the compilation process\n");
+  fprintf (stderr, "Total allocated maps size:           %5lu%c\n",
+	   SCALE (s.total_allocated_map_size),
+	   STAT_LABEL (s.total_allocated_map_size));
+  fprintf (stderr, "Total used maps size:                %5lu%c\n",
+	   SCALE (s.total_used_map_size),
+	   STAT_LABEL (s.total_used_map_size));
+  fprintf (stderr, "Ordinary map used size:              %5lu%c\n",
+	   SCALE (s.ordinary_maps_used_size),
+	   STAT_LABEL (s.ordinary_maps_used_size));
+  fprintf (stderr, "Macro maps used size:                %5lu%c\n",
+	   SCALE (s.macro_maps_used_size),
+	   STAT_LABEL (s.macro_maps_used_size));
+  fprintf (stderr, "Number of ordinary maps allocated:   %5lu%c\n",
+	   SCALE (s.num_ordinary_maps_allocated),
+	   STAT_LABEL (s.num_ordinary_maps_allocated));
+  fprintf (stderr, "Number of ordinary maps used:        %5lu%c\n",
+	   SCALE (s.num_ordinary_maps_used),
+	   STAT_LABEL (s.num_ordinary_maps_used));
+  fprintf (stderr, "Number of macro maps used:           %5lu%c\n",
+	   SCALE (s.num_macro_maps_used),
+	   STAT_LABEL (s.num_macro_maps_used));
+  fprintf (stderr, "Ordinary maps allocated size:        %5lu%c\n",
+	   SCALE (s.ordinary_maps_allocated_size),
+	   STAT_LABEL (s.ordinary_maps_allocated_size));
+  fprintf (stderr, "Macro maps locations size:           %5lu%c\n",
+	   SCALE (s.macro_maps_locations_size),
+	   STAT_LABEL (s.macro_maps_locations_size));
+  fprintf (stderr, "Duplicated maps locations size:      %5lu%c\n",
+	   SCALE (s.duplicated_macro_maps_locations_size),
+	   STAT_LABEL (s.duplicated_macro_maps_locations_size));
+  fprintf (stderr, "\n");
+}
diff --git a/gcc/input.h b/gcc/input.h
index 835c95a..ca122b5 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -55,4 +55,6 @@ extern location_t input_location;
   ((linemap_location_in_system_header_p (line_table, LOC)))
 #define in_system_header  (in_system_header_at (input_location))
 
+void dump_line_table_statistics (void);
+
 #endif
diff --git a/gcc/toplev.c b/gcc/toplev.c
index c0f6ee3..6f5792b 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1818,6 +1818,7 @@ target_reinit (void)
 void
 dump_memory_report (bool final)
 {
+  dump_line_table_statistics ();
   ggc_print_statistics ();
   stringpool_statistics ();
   dump_tree_statistics ();
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index fdd0a71..3216049 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -641,4 +641,21 @@ expanded_location linemap_expand_location_full (struct line_maps *,
 						enum location_resolution_kind,
 						const struct line_map**);
 void linemap_dump_location (struct line_maps *, source_location, FILE *);
+
+struct linemap_stats
+{
+  size_t num_ordinary_maps_allocated;
+  size_t num_ordinary_maps_used;
+  size_t ordinary_maps_allocated_size;
+  size_t ordinary_maps_used_size;
+  size_t num_macro_maps_used;
+  size_t macro_maps_used_size;
+  size_t macro_maps_locations_size;
+  size_t duplicated_macro_maps_locations_size;
+  size_t total_allocated_map_size;
+  size_t total_used_map_size;
+};
+
+void linemap_get_statistics (struct line_maps *, struct linemap_stats *);
+
 #endif /* !LIBCPP_LINE_MAP_H  */
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index 4012f99..442dddc 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -1021,3 +1021,68 @@ linemap_dump_location (struct line_maps *set,
   fprintf (stream, "{P:%s;F:%s;L:%d;C:%d;S:%d;M:%p;E:%d,LOC:%d}",
 	   path, from, l, c, s, (void*)map, e, loc);
 }
+
+/* Compute and return statistics about the memory consumption of some
+   parts of the line table SET.  */
+
+void
+linemap_get_statistics (struct line_maps *set,
+			struct linemap_stats *s)
+{
+  size_t ordinary_maps_allocated_size, ordinary_maps_used_size,
+    macro_maps_allocated_size, macro_maps_used_size,
+    macro_maps_locations_size = 0, duplicated_macro_maps_locations_size = 0,
+    total_allocated_map_size, total_used_map_size;
+  struct line_map *cur_map;
+
+  ordinary_maps_allocated_size =
+    LINEMAPS_ORDINARY_ALLOCATED (set) * sizeof (struct line_map);
+
+  ordinary_maps_used_size =
+    LINEMAPS_ORDINARY_USED (set) * sizeof (struct line_map);
+
+  macro_maps_allocated_size =
+    LINEMAPS_MACRO_ALLOCATED (set) * sizeof (struct line_map);
+
+  for (cur_map = LINEMAPS_MACRO_MAPS (set);
+       cur_map && cur_map <= LINEMAPS_LAST_MACRO_MAP (set);
+       ++cur_map)
+    {
+      unsigned i;
+
+      linemap_assert (linemap_macro_expansion_map_p (cur_map));
+
+      macro_maps_locations_size +=
+	2 * MACRO_MAP_NUM_MACRO_TOKENS (cur_map) * sizeof (source_location);
+
+      for (i = 0; i < 2 * MACRO_MAP_NUM_MACRO_TOKENS (cur_map); i+=2)
+	{
+	  if (MACRO_MAP_LOCATIONS (cur_map)[i] ==
+	      MACRO_MAP_LOCATIONS (cur_map)[i + 1])
+	    duplicated_macro_maps_locations_size +=
+	      sizeof (source_location);
+	}
+    }
+
+  macro_maps_used_size =
+    LINEMAPS_MACRO_USED (set) * sizeof (struct line_map)
+    + macro_maps_locations_size;
+
+  total_used_map_size = ordinary_maps_used_size + macro_maps_used_size;
+
+  total_allocated_map_size =
+    ordinary_maps_allocated_size + macro_maps_allocated_size +
+    macro_maps_locations_size;
+
+  s->num_ordinary_maps_allocated = LINEMAPS_ORDINARY_ALLOCATED (set);
+  s->num_ordinary_maps_used = LINEMAPS_ORDINARY_USED (set);
+  s->ordinary_maps_allocated_size = ordinary_maps_allocated_size;
+  s->ordinary_maps_used_size = ordinary_maps_used_size;
+  s->num_macro_maps_used = LINEMAPS_MACRO_USED (set);
+  s->macro_maps_used_size = macro_maps_used_size;
+  s->macro_maps_locations_size = macro_maps_locations_size;
+  s->duplicated_macro_maps_locations_size =
+    duplicated_macro_maps_locations_size;
+  s->total_allocated_map_size = total_allocated_map_size;
+  s->total_used_map_size = total_used_map_size;
+}