>
> libgcc/ChangeLog:
>
> PR gcov-profile/99105
> * libgcov-driver.c (write_top_counters): Rename to ...
> (write_topn_counters): ... this.
> (write_one_data): Pre-allocate buffer for number of items
> in the corresponding linked lists.
> * libgcov-merge.c (__gcov_merge_topn): Use renamed function.
>
> gcc/testsuite/ChangeLog:
>
> PR gcov-profile/99105
> * gcc.dg/tree-prof/indir-call-prof-malloc.c: Use profile
> correction as the wrapped malloc is called one more time
> from libgcov.
> for (unsigned i = 0; i < counters; i++)
> {
> - gcov_type pair_count = ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i + 1];
> gcov_write_counter (ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i]);
> - gcov_write_counter (pair_count);
> + gcov_write_counter (list_sizes[i]);
> gcov_type start = ci_ptr->values[GCOV_TOPN_MEM_COUNTERS * i + 2];
> +
> + unsigned j = 0;
> for (struct gcov_kvp *node = (struct gcov_kvp *)(intptr_t)start;
> - node != NULL; node = node->next)
> + node != NULL; node = node->next, j++)
> {
> gcov_write_counter (node->value);
> gcov_write_counter (node->count);
> +
> + /* Stop when we reach expected number of items. */
> + if (j + 1 == list_sizes[i])
> + break;
Since you counted number of entries earlier, I would expect loop to
always terminate here and thus the node != NULL condition in for loop
above to be unnecesary.
> }
> }
> +
> + free (list_sizes);
We already have our own mmap allocator, so I wonder if we don't want to
avoid malloc/free pair here. The counters are per-function, right? I
wonder if they happen to be large on some bigger project, but it may
reduct risk of user messing this up with his own malloc/free
implementation if we used alloca for counts of reasonable size.
> }
>
> /* Write counters in GI_PTR and the summary in PRG to a gcda file. In
> @@ -425,7 +446,7 @@ write_one_data (const struct gcov_info *gi_ptr,
> n_counts = ci_ptr->num;
>
> if (t_ix == GCOV_COUNTER_V_TOPN || t_ix == GCOV_COUNTER_V_INDIR)
> - write_top_counters (ci_ptr, t_ix, n_counts);
> + write_topn_counters (ci_ptr, t_ix, n_counts);
> else
> {
> /* Do not stream when all counters are zero. */
> diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
> index 7db188a4f4c..3379b688128 100644
> --- a/libgcc/libgcov-merge.c
> +++ b/libgcc/libgcov-merge.c
> @@ -109,6 +109,7 @@ __gcov_merge_topn (gcov_type *counters, unsigned
> n_counters)
> /* First value is number of total executions of the profiler. */
> gcov_type all = gcov_get_counter_ignore_scaling (-1);
> gcov_type n = gcov_get_counter_ignore_scaling (-1);
> + gcc_assert (n <= GCOV_TOPN_MAXIMUM_TRACKED_VALUES);
I think in the runtime we do not want to have asserts checking
implementation correctness since it bloats it up. So I would leave it
out.
I wonder if we can have some testcase for parallel updating/streaming in
testsuite?
Otherwise the patch looks good to me.
Honza
>
> unsigned full = all < 0;
> gcov_type *total = &counters[GCOV_TOPN_MEM_COUNTERS * i];
> --
> 2.30.0
>
