Re: [PATCH 1/3] gcov: Cache source files

[Jørgen Kvalsvik]

On 8/23/24 09:39, Jan Hubicka wrote:

Hi,

         1:    4:int notmain(const char *entity)
         -: == inlined from hello.h ==
         1:    6:  if (s)
branch  0 taken 0 (fallthrough)
branch  1 taken 1
     #####:    7:    printf ("hello, %s!\n", s);
     %%%%%:    7-block 3
call    0 never executed
         -:    8:  else
         1:    9:    printf ("hello, world!\n");
         1:    9-block 4
call    0 returned 1
         1:   10:  return 0;
         1:   10-block 5
         -: == inlined from hello.h (end) ==
         -:    5:{
         1:    6:  return hello (entity);
         1:    6-block 7
         -:    7:}

This indeed looks like a reasonable goal.

Thanks, pushed.

gcc/ChangeLog:

        * gcov.cc (release_structures): Release source_lines.
        (slurp): New function.
        (output_lines): Read sources with slurp.
---
  gcc/gcov.cc | 70 ++++++++++++++++++++++++++++++++++++++++-------------
  1 file changed, 53 insertions(+), 17 deletions(-)

diff --git a/gcc/gcov.cc b/gcc/gcov.cc
index e76a314041c..19019f404ee 100644
--- a/gcc/gcov.cc
+++ b/gcc/gcov.cc
@@ -550,6 +550,11 @@ static vector<name_map> names;
     a file being read multiple times.  */
  static vector<char *> processed_files;

+/* The contents of a source file. The nth SOURCE_LINES entry is the

+   contents of the nth SOURCES, or empty if it has not or could not be
+   read.  */
+static vector<vector<const char *>*> source_lines;
+
  /* This holds data summary information.  */

static unsigned object_runs;

@@ -762,6 +767,8 @@ static string make_gcov_file_name (const char *, const char 
*);
  static char *mangle_name (const char *);
  static void release_structures (void);
  extern int main (int, char **);
+static const vector<const char *>&
+slurp (const source_info &src, FILE *gcov_file, const char *line_start);

function_info::function_info (): m_name (NULL), m_demangled_name (NULL),

    ident (0), lineno_checksum (0), cfg_checksum (0), has_catch (0),
@@ -1804,6 +1811,15 @@ release_structures (void)
         it != functions.end (); it++)
      delete (*it);

+ for (vector<const char *> *lines : source_lines)

+    {
+      if (lines)
+       for (const char *line : *lines)
+         free (const_cast <char*> (line));
+      delete (lines);
+    }
+  source_lines.resize (0);
+
    for (fnfilter &filter : filters)
      regfree (&filter.regex);

@@ -3246,6 +3262,41 @@ read_line (FILE *file)

    return pos ? string : NULL;
  }

+/* Get the vector with the contents SRC, possibly from a cache. If

+   the reading fails, a message prefixed with LINE_START is written to
+   GCOV_FILE.  */
+static const vector<const char *>&
+slurp (const source_info &src, FILE *gcov_file,
+       const char *line_start)
+{
+  if (source_lines.size () <= src.index)
+    source_lines.resize (src.index + 1);
+
+  /* Store vector pointers so that the returned references remain
+     stable and won't be broken by successive calls to slurp.  */
+  if (!source_lines[src.index])
+    source_lines[src.index] = new vector<const char *> ();
+
+  if (!source_lines[src.index]->empty ())
+    return *source_lines[src.index];
+
+  FILE *source_file = fopen (src.name, "r");
+  if (!source_file)
+    fnotice (stderr, "Cannot open source file %s\n", src.name);
+  else if (src.file_time == 0)
+    fprintf (gcov_file, "%sSource is newer than graph\n", line_start);
+
+  const char *retval;
+  vector<const char *> &lines = *source_lines[src.index];
+  if (source_file)
+    while ((retval = read_line (source_file)))
+      lines.push_back (xstrdup (retval));
+
+  if (source_file)
+    fclose (source_file);
+  return lines;

So this is basically going to read all sources needed for single
compilation unit (.cc file) into memory at once.  I guess that is OK.
I wonder if we don't want to do something like mmap the source and then
set up source_lines array instead of reading every like into spearate
xstrduped chunk?
But that could be done incrementally, so th epatch is OK.

Honza

+}
+
  /* Pad string S with spaces from left to have total width equal to 9.  */

static void

@@ -3435,9 +3486,6 @@ output_lines (FILE *gcov_file, const source_info *src)
  #define  DEFAULT_LINE_START "        -:    0:"
  #define FN_SEPARATOR "------------------\n"

- FILE *source_file;

-  const char *retval;
-
    /* Print colorization legend.  */
    if (flag_use_colors)
      fprintf (gcov_file, "%s",
@@ -3464,17 +3512,8 @@ output_lines (FILE *gcov_file, const source_info *src)
        fprintf (gcov_file, DEFAULT_LINE_START "Runs:%u\n", object_runs);
      }

- source_file = fopen (src->name, "r");

-  if (!source_file)
-    fnotice (stderr, "Cannot open source file %s\n", src->name);
-  else if (src->file_time == 0)
-    fprintf (gcov_file, DEFAULT_LINE_START "Source is newer than graph\n");
-
-  vector<const char *> source_lines;
-  if (source_file)
-    while ((retval = read_line (source_file)) != NULL)
-      source_lines.push_back (xstrdup (retval));
-
+  const vector<const char *> &source_lines = slurp (*src, gcov_file,
+                                                   DEFAULT_LINE_START);
    unsigned line_start_group = 0;
    vector<function_info *> *fns;
    unsigned filtered_line_end = !filters.empty () ? 0 : source_lines.size ();
@@ -3596,7 +3635,4 @@ output_lines (FILE *gcov_file, const source_info *src)
          line_start_group = 0;
        }
      }
-
-  if (source_file)
-    fclose (source_file);
  }
--
2.39.2