Fix summary generation with fork

Hi,
this patch fixes problem we noticed with Martin Liska where gcov_dump is called
several times per execution of firefox (on each fork and exec).  This causes
runs to be large and makes functions executed once per program to be considered
cold.

This patch makes us to update runs just once per execution and not on each
streaming of profile data.  While testing it I also noticed that program
summary is now broken, since crc32 is accumulated per each dumping instead just
once.

I believe the newly introduced static vars should go - there is nothing really
preventing us from doing two concurent updates and also it just unnecesary
increases to footprint of libgcov.  I converted thus all_prg and crc32 back to
local vars.

Bootstrapped/regtested x86_64-linux, comitted.

	* libgcov-driver.c (get_gcov_dump_complete): Update comments.
	(all_prg, crc32): Remove static vars.
	(gcov_exit_compute_summary): Rewrite to return crc32; do not clear
	all_prg.
	(gcov_exit_merge_gcda): Add crc32 parameter.
	(gcov_exit_merge_summary): Add crc32 and all_prg parameter;
	do not account run if it was already accounted.
	(gcov_exit_dump_gcov): Add crc32 and all_prg parameters.
	(gcov_exit): Initialize all_prg; update.

On Mon, Nov 18, 2013 at 2:15 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this patch fixes problem we noticed with Martin Liska where gcov_dump is called
> several times per execution of firefox (on each fork and exec).  This causes
> runs to be large and makes functions executed once per program to be considered
> cold.
>
> This patch makes us to update runs just once per execution and not on each
> streaming of profile data.  While testing it I also noticed that program
> summary is now broken, since crc32 is accumulated per each dumping instead just
> once.

You are right. I forgot that gcov_exit() can be called multiple times.
Should have
initialized crc32 per gcov_exit() call.

Thanks for the fix.
>
> I believe the newly introduced static vars should go - there is nothing really
> preventing us from doing two concurent updates and also it just unnecesary
> increases to footprint of libgcov.  I converted thus all_prg and crc32 back to
> local vars.
>
> Bootstrapped/regtested x86_64-linux, comitted.
>
>         * libgcov-driver.c (get_gcov_dump_complete): Update comments.
>         (all_prg, crc32): Remove static vars.
>         (gcov_exit_compute_summary): Rewrite to return crc32; do not clear
>         all_prg.
>         (gcov_exit_merge_gcda): Add crc32 parameter.
>         (gcov_exit_merge_summary): Add crc32 and all_prg parameter;
>         do not account run if it was already accounted.
>         (gcov_exit_dump_gcov): Add crc32 and all_prg parameters.
>         (gcov_exit): Initialize all_prg; update.
> Index: libgcov-driver.c
> ===================================================================
> --- libgcov-driver.c    (revision 204945)
> +++ libgcov-driver.c    (working copy)
> @@ -96,7 +96,7 @@ static size_t gcov_max_filename = 0;
>  /* Flag when the profile has already been dumped via __gcov_dump().  */
>  static int gcov_dump_complete;
>
> -/* A global functino that get the vaule of gcov_dump_complete.  */
> +/* A global function that get the vaule of gcov_dump_complete.  */
>
>  int
>  get_gcov_dump_complete (void)
> @@ -319,12 +319,6 @@ gcov_compute_histogram (struct gcov_summ
>
>  /* summary for program.  */
>  static struct gcov_summary this_prg;
> -#if !GCOV_LOCKED
> -/* summary for all instances of program.  */
> -static struct gcov_summary all_prg;
> -#endif
> -/* crc32 for this program.  */
> -static gcov_unsigned_t crc32;
>  /* gcda filename.  */
>  static char *gi_filename;
>  /* buffer for the fn_data from another program.  */
> @@ -333,10 +327,9 @@ static struct gcov_fn_buffer *fn_buffer;
>  static struct gcov_summary_buffer *sum_buffer;
>
>  /* This funtions computes the program level summary and the histo-gram.
> -   It initializes ALL_PRG, computes CRC32, and stores the summary in
> -   THIS_PRG. All these three variables are file statics.  */
> +   It computes and returns CRC32  and stored summari in THIS_PRG.  */
>
> -static void
> +static gcov_unsigned_t
>  gcov_exit_compute_summary (void)
>  {
>    struct gcov_info *gi_ptr;
> @@ -346,10 +339,8 @@ gcov_exit_compute_summary (void)
>    int f_ix;
>    unsigned t_ix;
>    gcov_unsigned_t c_num;
> +  gcov_unsigned_t crc32 = 0;
>
> -#if !GCOV_LOCKED
> -  memset (&all_prg, 0, sizeof (all_prg));
> -#endif
>    /* Find the totals for this execution.  */
>    memset (&this_prg, 0, sizeof (this_prg));
>    for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
> @@ -391,6 +382,7 @@ gcov_exit_compute_summary (void)
>          }
>      }
>    gcov_compute_histogram (&this_prg);
> +  return crc32;
>  }
>
>  /* A struct that bundles all the related information about the
> @@ -412,7 +404,8 @@ static int
>  gcov_exit_merge_gcda (struct gcov_info *gi_ptr,
>                        struct gcov_summary *prg_p,
>                        gcov_position_t *summary_pos_p,
> -                      gcov_position_t *eof_pos_p)
> +                      gcov_position_t *eof_pos_p,
> +                     gcov_unsigned_t crc32)
>  {
>    gcov_unsigned_t tag, length;
>    unsigned t_ix;
> @@ -652,13 +645,19 @@ gcov_exit_write_gcda (const struct gcov_
>     Return -1 on error. Return 0 on success.  */
>
>  static int
> -gcov_exit_merge_summary (const struct gcov_info *gi_ptr, struct gcov_summary *prg)
> +gcov_exit_merge_summary (const struct gcov_info *gi_ptr, struct gcov_summary *prg,
> +                        gcov_unsigned_t crc32, struct gcov_summary *all_prg)
>  {
>    struct gcov_ctr_summary *cs_prg, *cs_tprg;
> -#if !GCOV_LOCKED
> -  struct gcov_ctr_summary *cs_all;
> -#endif
>    unsigned t_ix;
> +  /* If application calls fork or exec multiple times, we end up storing
> +     profile repeadely.  We should not account this as multiple runs or
> +     functions executed once may mistakely become cold.  */
> +  static int run_accounted = 0;
> +#if !GCOV_LOCKED
> +  /* summary for all instances of program.  */
> +  struct gcov_ctr_summary *cs_all;
> +#endif
>
>    /* Merge the summaries.  */
>    for (t_ix = 0; t_ix < GCOV_COUNTERS_SUMMABLE; t_ix++)
> @@ -668,13 +667,18 @@ gcov_exit_merge_summary (const struct gc
>
>        if (gi_ptr->merge[t_ix])
>          {
> -          if (!cs_prg->runs++)
> +         int first = !cs_prg->runs;
> +
> +         if (!run_accounted)
> +           cs_prg->runs++;
> +         run_accounted = 1;
> +          if (first)
>              cs_prg->num = cs_tprg->num;
>            cs_prg->sum_all += cs_tprg->sum_all;
>            if (cs_prg->run_max < cs_tprg->run_max)
>              cs_prg->run_max = cs_tprg->run_max;
>            cs_prg->sum_max += cs_tprg->run_max;
> -          if (cs_prg->runs == 1)
> +          if (first)
>              memcpy (cs_prg->histogram, cs_tprg->histogram,
>                     sizeof (gcov_bucket_type) * GCOV_HISTOGRAM_SIZE);
>            else
> @@ -686,9 +690,8 @@ gcov_exit_merge_summary (const struct gc
>                        gi_filename);
>            return -1;
>          }
> -
>  #if !GCOV_LOCKED
> -      cs_all = &all_prg.ctrs[t_ix];
> +      cs_all = &all_prg->ctrs[t_ix];
>        if (!cs_all->runs && cs_prg->runs)
>          {
>            cs_all->num = cs_prg->num;
> @@ -697,7 +700,7 @@ gcov_exit_merge_summary (const struct gc
>            cs_all->run_max = cs_prg->run_max;
>            cs_all->sum_max = cs_prg->sum_max;
>          }
> -      else if (!all_prg.checksum
> +      else if (!all_prg->checksum
>                 /* Don't compare the histograms, which may have slight
>                    variations depending on the order they were updated
>                    due to the truncating integer divides used in the
> @@ -711,7 +714,7 @@ gcov_exit_merge_summary (const struct gc
>                 gcov_error ("profiling:%s:Data file mismatch - some "
>                             "data files may have been concurrently "
>                             "updated without locking support\n", gi_filename);
> -               all_prg.checksum = ~0u;
> +               all_prg->checksum = ~0u;
>               }
>  #endif
>      }
> @@ -729,7 +732,8 @@ gcov_exit_merge_summary (const struct gc
>     summaries separate.  */
>
>  static void
> -gcov_exit_dump_gcov (struct gcov_info *gi_ptr, struct gcov_filename_aux *gf)
> +gcov_exit_dump_gcov (struct gcov_info *gi_ptr, struct gcov_filename_aux *gf,
> +                    gcov_unsigned_t crc32, struct gcov_summary *all_prg)
>  {
>    struct gcov_summary prg; /* summary for this object over all program.  */
>    int error;
> @@ -753,7 +757,8 @@ gcov_exit_dump_gcov (struct gcov_info *g
>            gcov_error ("profiling:%s:Not a gcov data file\n", gi_filename);
>            goto read_fatal;
>          }
> -      error = gcov_exit_merge_gcda (gi_ptr, &prg, &summary_pos, &eof_pos);
> +      error = gcov_exit_merge_gcda (gi_ptr, &prg, &summary_pos, &eof_pos,
> +                                   crc32);
>        if (error == -1)
>          goto read_fatal;
>      }
> @@ -766,7 +771,7 @@ gcov_exit_dump_gcov (struct gcov_info *g
>        summary_pos = eof_pos;
>      }
>
> -  error = gcov_exit_merge_summary (gi_ptr, &prg);
> +  error = gcov_exit_merge_summary (gi_ptr, &prg, crc32, all_prg);
>    if (error == -1)
>      goto read_fatal;
>
> @@ -794,19 +799,24 @@ gcov_exit (void)
>  {
>    struct gcov_info *gi_ptr;
>    struct gcov_filename_aux gf;
> +  gcov_unsigned_t crc32;
> +  struct gcov_summary all_prg;
>
>    /* Prevent the counters from being dumped a second time on exit when the
>       application already wrote out the profile using __gcov_dump().  */
>    if (gcov_dump_complete)
>      return;
>
> -  gcov_exit_compute_summary ();
> +  crc32 = gcov_exit_compute_summary ();
>
>    allocate_filename_struct (&gf);
> +#if !GCOV_LOCKED
> +  memset (&all_prg, 0, sizeof (all_prg));
> +#endif
>
>    /* Now merge each file.  */
>    for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next)
> -    gcov_exit_dump_gcov (gi_ptr, &gf);
> +    gcov_exit_dump_gcov (gi_ptr, &gf, crc32, &all_prg);
>
>    if (gi_filename)
>      free (gi_filename);