Re: [PATCH] profiling: fix streaming of TOPN counters

[Martin Liška]

On 3/4/21 1:50 PM, Jan Hubicka wrote:

  .../gcc.dg/tree-prof/indir-call-prof-malloc.c |  2 +-
  gcc/testsuite/gcc.dg/tree-prof/pr97461.c      |  2 +-
  libgcc/libgcov-driver.c                       | 56 ++++++++++++++++---
  3 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c 
b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
index 454e224c95f..7bda4aedfc8 100644
--- a/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
+++ b/gcc/testsuite/gcc.dg/tree-prof/indir-call-prof-malloc.c
@@ -1,4 +1,4 @@
-/* { dg-options "-O2 -ldl" } */
+/* { dg-options "-O2 -ldl -fprofile-correction" } */

#define _GNU_SOURCE

  #include <stdio.h>
diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr97461.c 
b/gcc/testsuite/gcc.dg/tree-prof/pr97461.c
index 213fac9af04..f684be4d80f 100644
--- a/gcc/testsuite/gcc.dg/tree-prof/pr97461.c
+++ b/gcc/testsuite/gcc.dg/tree-prof/pr97461.c
@@ -1,5 +1,5 @@
  /* PR gcov-profile/97461 */
-/* { dg-options "-O2 -ldl" } */
+/* { dg-options "-O2 -ldl -fprofile-correction" } */

#define _GNU_SOURCE diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c

index 91462350132..d2e60a5a6df 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -42,6 +42,10 @@ void __gcov_init (struct gcov_info *p __attribute__ 
((unused))) {}
  #include <sys/stat.h>
  #endif

+#if HAVE_SYS_MMAN_H

+#include <sys/mman.h>
+#endif
+
  #ifdef L_gcov

/* A utility function for outputting errors. */

@@ -334,30 +338,66 @@ read_error:
    return -1;
  }

+#define MAX(X,Y) ((X) > (Y) ? (X) : (Y))

+
  /* Store all TOP N counters where each has a dynamic length.  */

static void

-write_top_counters (const struct gcov_ctr_info *ci_ptr,
-                   unsigned t_ix,
-                   gcov_unsigned_t n_counts)
+write_topn_counters (const struct gcov_ctr_info *ci_ptr,
+                    unsigned t_ix,
+                    gcov_unsigned_t n_counts)
  {
    unsigned counters = n_counts / GCOV_TOPN_MEM_COUNTERS;
    gcc_assert (n_counts % GCOV_TOPN_MEM_COUNTERS == 0);
+
+  /* It can happen in a multi-threaded environment that number of counters is
+     different from the size of the corresponding linked lists.  */
+#define LIST_SIZE_MIN_LENGTH 4 * 1024
+
+  static unsigned *list_sizes = NULL;
+  static unsigned list_size_length = 0;
+
+  if (list_sizes == NULL || counters > list_size_length)
+    {
+      list_size_length = MAX (LIST_SIZE_MIN_LENGTH, counters);
+#if HAVE_SYS_MMAN_H
+      list_sizes = (unsigned *)mmap (NULL, list_size_length * sizeof 
(unsigned),
+                                    PROT_READ | PROT_WRITE,
+                                    MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+#endif
+
+      /* Malloc fallback.  */
+      if (list_sizes == NULL)
+       list_sizes = (unsigned *)xmalloc (list_size_length * sizeof (unsigned));

I see, you switched to allocating buffer permanently. This also works.
Given that you do not deallocate the memory, I think you want to
allocate list_size_length*2 so we do not end up taking O(mn) memory
where n is the largest number of counters in a file and m is number of
sections with counters.

Yeah, I used there a minimal amount of memory that should fit all normal 
functions.
I updated that to 2 * counters.

Can you please unify the mmap code in list allocation and here, so there
is only one place in libgcov Windows folks will need to update?

Otherwise the patch looks OK.

Thanks, pushed.
Martin

Honza