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