public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Lewis Hyatt <lhyatt@gmail.com>
To: David Malcolm <dmalcolm@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [committed] input.c: move file caching globals to a new file_cache class
Date: Sun, 11 Jul 2021 12:58:39 -0400	[thread overview]
Message-ID: <CAA_5UQ6p58TMXDXpAseY3Ahk9iHYcf2Z1me5tzjab_FFxQOYag@mail.gmail.com> (raw)
In-Reply-To: <20210701215110.1409601-1-dmalcolm@redhat.com>

Hi David-

I thought this might be a good opportunity to ask about the patch that
supports -finput-charset in diagnostic.c please?
https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564527.html

The patch will require some work to adapt to the new changes below. I
am happy to do that, but thought I should check first whether you have
any interest in this approach? Thanks!

-Lewis

On Thu, Jul 1, 2021 at 5:52 PM David Malcolm via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This moves some global state from input.c to a new file_cache class,
> of which an instance is owned by global_dc.  Various state is also
> made private.
>
> No functional change intended.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> Pushed to trunk as b544c348e13ad33d55f0d954370ab1fb0f2bf683.
>
> gcc/ChangeLog:
>         * diagnostic.h (diagnostic_context::m_file_cache): New field.
>         * input.c (class fcache): Rename to...
>         (class file_cache_slot): ...this, making most members private and
>         prefixing fields with "m_".
>         (file_cache_slot::get_file_path): New accessor.
>         (file_cache_slot::get_use_count): New accessor.
>         (file_cache_slot::missing_trailing_newline_p): New accessor.
>         (file_cache_slot::inc_use_count): New.
>         (fcache_buffer_size): Move to...
>         (file_cache_slot::buffer_size): ...here.
>         (fcache_line_record_size): Move to...
>         (file_cache_slot::line_record_size): ...here.
>         (fcache_tab): Delete, in favor of global_dc->m_file_cache.
>         (fcache_tab_size): Move to file_cache::num_file_slots.
>         (diagnostic_file_cache_init): Update for move of fcache_tab
>         to global_dc->m_file_cache.
>         (diagnostic_file_cache_fini): Likewise.
>         (lookup_file_in_cache_tab): Convert to...
>         (file_cache::lookup_file): ...this.
>         (diagnostics_file_cache_forcibly_evict_file): Update for move of
>         fcache_tab to global_dc->m_file_cache, moving most of
>         implementation to...
>         (file_cache::forcibly_evict_file): ...this new function and...
>         (file_cache_slot::evict): ...this new function.
>         (evicted_cache_tab_entry): Convert to...
>         (file_cache::evicted_cache_tab_entry): ...this.
>         (add_file_to_cache_tab): Convert to...
>         (file_cache::add_file): ...this, moving bulk of implementation
>         to...
>         (file_cache_slot::create): ..this new function.
>         (file_cache::file_cache): New.
>         (file_cache::~file_cache): New.
>         (lookup_or_add_file_to_cache_tab): Convert to...
>         (file_cache::lookup_or_add_file): ..this new function.
>         (fcache::fcache): Rename to...
>         (file_cache_slot::file_cache_slot): ...this, adding "m_" prefixes
>         to fields.
>         (fcache::~fcache): Rename to...
>         (file_cache_slot::~file_cache_slot): ...this, adding "m_" prefixes
>         to fields.
>         (needs_read): Convert to...
>         (file_cache_slot::needs_read_p): ...this.
>         (needs_grow): Convert to...
>         (file_cache_slot::needs_grow_p): ...this.
>         (maybe_grow): Convert to...
>         (file_cache_slot::maybe_grow): ...this.
>         (read_data): Convert to...
>         (file_cache_slot::read_data): ...this.
>         (maybe_read_data): Convert to...
>         (file_cache_slot::maybe_read_data): ...this.
>         (get_next_line): Convert to...
>         (file_cache_slot::get_next_line): ...this.
>         (goto_next_line): Convert to...
>         (file_cache_slot::goto_next_line): ...this.
>         (read_line_num): Convert to...
>         (file_cache_slot::read_line_num): ...this.
>         (location_get_source_line): Update for moving of globals to
>         global_dc->m_file_cache.
>         (location_missing_trailing_newline): Likewise.
>         * input.h (class file_cache_slot): New forward decl.
>         (class file_cache): New.
>
> Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> ---
>  gcc/diagnostic.h |   3 +
>  gcc/input.c      | 459 +++++++++++++++++++++++++++--------------------
>  gcc/input.h      |  33 ++++
>  3 files changed, 301 insertions(+), 194 deletions(-)
>
> diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
> index 1b9d6b1f64d..086bc4f903f 100644
> --- a/gcc/diagnostic.h
> +++ b/gcc/diagnostic.h
> @@ -136,6 +136,9 @@ struct diagnostic_context
>    /* Where most of the diagnostic formatting work is done.  */
>    pretty_printer *printer;
>
> +  /* Cache of source code.  */
> +  file_cache *m_file_cache;
> +
>    /* The number of times we have issued diagnostics.  */
>    int diagnostic_count[DK_LAST_DIAGNOSTIC_KIND];
>
> diff --git a/gcc/input.c b/gcc/input.c
> index 9e39e7df83c..de20d983d2c 100644
> --- a/gcc/input.c
> +++ b/gcc/input.c
> @@ -32,9 +32,29 @@ along with GCC; see the file COPYING3.  If not see
>
>  /* This is a cache used by get_next_line to store the content of a
>     file to be searched for file lines.  */
> -class fcache
> +class file_cache_slot
>  {
>  public:
> +  file_cache_slot ();
> +  ~file_cache_slot ();
> +
> +  bool read_line_num (size_t line_num,
> +                     char ** line, ssize_t *line_len);
> +
> +  /* Accessors.  */
> +  const char *get_file_path () const { return m_file_path; }
> +  unsigned get_use_count () const { return m_use_count; }
> +  bool missing_trailing_newline_p () const
> +  {
> +    return m_missing_trailing_newline;
> +  }
> +
> +  void inc_use_count () { m_use_count++; }
> +
> +  void create (const char *file_path, FILE *fp, unsigned highest_use_count);
> +  void evict ();
> +
> + private:
>    /* These are information used to store a line boundary.  */
>    class line_info
>    {
> @@ -61,36 +81,48 @@ public:
>      {}
>    };
>
> +  bool needs_read_p () const;
> +  bool needs_grow_p () const;
> +  void maybe_grow ();
> +  bool read_data ();
> +  bool maybe_read_data ();
> +  bool get_next_line (char **line, ssize_t *line_len);
> +  bool read_next_line (char ** line, ssize_t *line_len);
> +  bool goto_next_line ();
> +
> +  static const size_t buffer_size = 4 * 1024;
> +  static const size_t line_record_size = 100;
> +
>    /* The number of time this file has been accessed.  This is used
>       to designate which file cache to evict from the cache
>       array.  */
> -  unsigned use_count;
> +  unsigned m_use_count;
>
>    /* The file_path is the key for identifying a particular file in
>       the cache.
>       For libcpp-using code, the underlying buffer for this field is
>       owned by the corresponding _cpp_file within the cpp_reader.  */
> -  const char *file_path;
> +  const char *m_file_path;
>
> -  FILE *fp;
> +  FILE *m_fp;
>
>    /* This points to the content of the file that we've read so
>       far.  */
> -  char *data;
> +  char *m_data;
>
>    /*  The size of the DATA array above.*/
> -  size_t size;
> +  size_t m_size;
>
>    /* The number of bytes read from the underlying file so far.  This
>       must be less (or equal) than SIZE above.  */
> -  size_t nb_read;
> +  size_t m_nb_read;
>
>    /* The index of the beginning of the current line.  */
> -  size_t line_start_idx;
> +  size_t m_line_start_idx;
>
>    /* The number of the previous line read.  This starts at 1.  Zero
>       means we've read no line so far.  */
> -  size_t line_num;
> +  size_t m_line_num;
>
>    /* This is the total number of lines of the current file.  At the
>       moment, we try to get this information from the line map
> @@ -100,24 +132,21 @@ public:
>       the number of lines before compilation really starts.  For e.g,
>       the C front-end, it can happen that we start emitting diagnostics
>       before the line map has seen the end of the file.  */
> -  size_t total_lines;
> +  size_t m_total_lines;
>
>    /* Could this file be missing a trailing newline on its final line?
>       Initially true (to cope with empty files), set to true/false
>       as each line is read.  */
> -  bool missing_trailing_newline;
> +  bool m_missing_trailing_newline;
>
>    /* This is a record of the beginning and end of the lines we've seen
>       while reading the file.  This is useful to avoid walking the data
>       from the beginning when we are asked to read a line that is
>       before LINE_START_IDX above.  Note that the maximum size of this
> -     record is fcache_line_record_size, so that the memory consumption
> +     record is line_record_size, so that the memory consumption
>       doesn't explode.  We thus scale total_lines down to
> -     fcache_line_record_size.  */
> -  vec<line_info, va_heap> line_record;
> -
> -  fcache ();
> -  ~fcache ();
> +     line_record_size.  */
> +  vec<line_info, va_heap> m_line_record;
>  };
>
>  /* Current position in real source file.  */
> @@ -133,11 +162,6 @@ class line_maps *line_table;
>
>  class line_maps *saved_line_table;
>
> -static fcache *fcache_tab;
> -static const size_t fcache_tab_size = 16;
> -static const size_t fcache_buffer_size = 4 * 1024;
> -static const size_t fcache_line_record_size = 100;
> -
>  /* Expand the source location LOC into a human readable location.  If
>     LOC resolves to a builtin location, the file name of the readable
>     location is set to the string "<built-in>". If EXPANSION_POINT_P is
> @@ -233,8 +257,9 @@ expand_location_1 (location_t loc,
>  static void
>  diagnostic_file_cache_init (void)
>  {
> -  if (fcache_tab == NULL)
> -    fcache_tab = new fcache[fcache_tab_size];
> +  gcc_assert (global_dc);
> +  if (global_dc->m_file_cache == NULL)
> +    global_dc->m_file_cache = new file_cache ();
>  }
>
>  /* Free the resources used by the set of cache used for files accessed
> @@ -243,10 +268,10 @@ diagnostic_file_cache_init (void)
>  void
>  diagnostic_file_cache_fini (void)
>  {
> -  if (fcache_tab)
> +  if (global_dc->m_file_cache)
>      {
> -      delete [] (fcache_tab);
> -      fcache_tab = NULL;
> +      delete global_dc->m_file_cache;
> +      global_dc->m_file_cache = NULL;
>      }
>  }
>
> @@ -273,28 +298,25 @@ total_lines_num (const char *file_path)
>     caret diagnostic.  Return the found cached file, or NULL if no
>     cached file was found.  */
>
> -static fcache*
> -lookup_file_in_cache_tab (const char *file_path)
> +file_cache_slot *
> +file_cache::lookup_file (const char *file_path)
>  {
> -  if (file_path == NULL)
> -    return NULL;
> -
> -  diagnostic_file_cache_init ();
> +  gcc_assert (file_path);
>
>    /* This will contain the found cached file.  */
> -  fcache *r = NULL;
> -  for (unsigned i = 0; i < fcache_tab_size; ++i)
> +  file_cache_slot *r = NULL;
> +  for (unsigned i = 0; i < num_file_slots; ++i)
>      {
> -      fcache *c = &fcache_tab[i];
> -      if (c->file_path && !strcmp (c->file_path, file_path))
> +      file_cache_slot *c = &m_file_slots[i];
> +      if (c->get_file_path () && !strcmp (c->get_file_path (), file_path))
>         {
> -         ++c->use_count;
> +         c->inc_use_count ();
>           r = c;
>         }
>      }
>
>    if (r)
> -    ++r->use_count;
> +    r->inc_use_count ();
>
>    return r;
>  }
> @@ -308,22 +330,39 @@ diagnostics_file_cache_forcibly_evict_file (const char *file_path)
>  {
>    gcc_assert (file_path);
>
> -  fcache *r = lookup_file_in_cache_tab (file_path);
> +  if (!global_dc->m_file_cache)
> +    return;
> +
> +  global_dc->m_file_cache->forcibly_evict_file (file_path);
> +}
> +
> +void
> +file_cache::forcibly_evict_file (const char *file_path)
> +{
> +  gcc_assert (file_path);
> +
> +  file_cache_slot *r = lookup_file (file_path);
>    if (!r)
>      /* Not found.  */
>      return;
>
> -  r->file_path = NULL;
> -  if (r->fp)
> -    fclose (r->fp);
> -  r->fp = NULL;
> -  r->nb_read = 0;
> -  r->line_start_idx = 0;
> -  r->line_num = 0;
> -  r->line_record.truncate (0);
> -  r->use_count = 0;
> -  r->total_lines = 0;
> -  r->missing_trailing_newline = true;
> +  r->evict ();
> +}
> +
> +void
> +file_cache_slot::evict ()
> +{
> +  m_file_path = NULL;
> +  if (m_fp)
> +    fclose (m_fp);
> +  m_fp = NULL;
> +  m_nb_read = 0;
> +  m_line_start_idx = 0;
> +  m_line_num = 0;
> +  m_line_record.truncate (0);
> +  m_use_count = 0;
> +  m_total_lines = 0;
> +  m_missing_trailing_newline = true;
>  }
>
>  /* Return the file cache that has been less used, recently, or the
> @@ -331,26 +370,26 @@ diagnostics_file_cache_forcibly_evict_file (const char *file_path)
>     *HIGHEST_USE_COUNT is set to the highest use count of the entries
>     in the cache table.  */
>
> -static fcache*
> -evicted_cache_tab_entry (unsigned *highest_use_count)
> +file_cache_slot*
> +file_cache::evicted_cache_tab_entry (unsigned *highest_use_count)
>  {
>    diagnostic_file_cache_init ();
>
> -  fcache *to_evict = &fcache_tab[0];
> -  unsigned huc = to_evict->use_count;
> -  for (unsigned i = 1; i < fcache_tab_size; ++i)
> +  file_cache_slot *to_evict = &m_file_slots[0];
> +  unsigned huc = to_evict->get_use_count ();
> +  for (unsigned i = 1; i < num_file_slots; ++i)
>      {
> -      fcache *c = &fcache_tab[i];
> -      bool c_is_empty = (c->file_path == NULL);
> +      file_cache_slot *c = &m_file_slots[i];
> +      bool c_is_empty = (c->get_file_path () == NULL);
>
> -      if (c->use_count < to_evict->use_count
> -         || (to_evict->file_path && c_is_empty))
> +      if (c->get_use_count () < to_evict->get_use_count ()
> +         || (to_evict->get_file_path () && c_is_empty))
>         /* We evict C because it's either an entry with a lower use
>            count or one that is empty.  */
>         to_evict = c;
>
> -      if (huc < c->use_count)
> -       huc = c->use_count;
> +      if (huc < c->get_use_count ())
> +       huc = c->get_use_count ();
>
>        if (c_is_empty)
>         /* We've reached the end of the cache; subsequent elements are
> @@ -368,10 +407,10 @@ evicted_cache_tab_entry (unsigned *highest_use_count)
>     accessed by caret diagnostic.  This cache is added to an array of
>     cache and can be retrieved by lookup_file_in_cache_tab.  This
>     function returns the created cache.  Note that only the last
> -   fcache_tab_size files are cached.  */
> +   num_file_slots files are cached.  */
>
> -static fcache*
> -add_file_to_cache_tab (const char *file_path)
> +file_cache_slot*
> +file_cache::add_file (const char *file_path)
>  {
>
>    FILE *fp = fopen (file_path, "r");
> @@ -379,22 +418,45 @@ add_file_to_cache_tab (const char *file_path)
>      return NULL;
>
>    unsigned highest_use_count = 0;
> -  fcache *r = evicted_cache_tab_entry (&highest_use_count);
> -  r->file_path = file_path;
> -  if (r->fp)
> -    fclose (r->fp);
> -  r->fp = fp;
> -  r->nb_read = 0;
> -  r->line_start_idx = 0;
> -  r->line_num = 0;
> -  r->line_record.truncate (0);
> +  file_cache_slot *r = evicted_cache_tab_entry (&highest_use_count);
> +  r->create (file_path, fp, highest_use_count);
> +  return r;
> +}
> +
> +/* Populate this slot for use on FILE_PATH and FP, dropping any
> +   existing cached content within it.  */
> +
> +void
> +file_cache_slot::create (const char *file_path, FILE *fp,
> +                        unsigned highest_use_count)
> +{
> +  m_file_path = file_path;
> +  if (m_fp)
> +    fclose (m_fp);
> +  m_fp = fp;
> +  m_nb_read = 0;
> +  m_line_start_idx = 0;
> +  m_line_num = 0;
> +  m_line_record.truncate (0);
>    /* Ensure that this cache entry doesn't get evicted next time
>       add_file_to_cache_tab is called.  */
> -  r->use_count = ++highest_use_count;
> -  r->total_lines = total_lines_num (file_path);
> -  r->missing_trailing_newline = true;
> +  m_use_count = ++highest_use_count;
> +  m_total_lines = total_lines_num (file_path);
> +  m_missing_trailing_newline = true;
> +}
>
> -  return r;
> +/* file_cache's ctor.  */
> +
> +file_cache::file_cache ()
> +: m_file_slots (new file_cache_slot[num_file_slots])
> +{
> +}
> +
> +/* file_cache's dtor.  */
> +
> +file_cache::~file_cache ()
> +{
> +  delete[] m_file_slots;
>  }
>
>  /* Lookup the cache used for the content of a given file accessed by
> @@ -402,41 +464,41 @@ add_file_to_cache_tab (const char *file_path)
>     for this file, add it to the array of cached file and return
>     it.  */
>
> -static fcache*
> -lookup_or_add_file_to_cache_tab (const char *file_path)
> +file_cache_slot*
> +file_cache::lookup_or_add_file (const char *file_path)
>  {
> -  fcache *r = lookup_file_in_cache_tab (file_path);
> +  file_cache_slot *r = lookup_file (file_path);
>    if (r == NULL)
> -    r = add_file_to_cache_tab (file_path);
> +    r = add_file (file_path);
>    return r;
>  }
>
>  /* Default constructor for a cache of file used by caret
>     diagnostic.  */
>
> -fcache::fcache ()
> -: use_count (0), file_path (NULL), fp (NULL), data (0),
> -  size (0), nb_read (0), line_start_idx (0), line_num (0),
> -  total_lines (0), missing_trailing_newline (true)
> +file_cache_slot::file_cache_slot ()
> +: m_use_count (0), m_file_path (NULL), m_fp (NULL), m_data (0),
> +  m_size (0), m_nb_read (0), m_line_start_idx (0), m_line_num (0),
> +  m_total_lines (0), m_missing_trailing_newline (true)
>  {
> -  line_record.create (0);
> +  m_line_record.create (0);
>  }
>
>  /* Destructor for a cache of file used by caret diagnostic.  */
>
> -fcache::~fcache ()
> +file_cache_slot::~file_cache_slot ()
>  {
> -  if (fp)
> +  if (m_fp)
>      {
> -      fclose (fp);
> -      fp = NULL;
> +      fclose (m_fp);
> +      m_fp = NULL;
>      }
> -  if (data)
> +  if (m_data)
>      {
> -      XDELETEVEC (data);
> -      data = 0;
> +      XDELETEVEC (m_data);
> +      m_data = 0;
>      }
> -  line_record.release ();
> +  m_line_record.release ();
>  }
>
>  /* Returns TRUE iff the cache would need to be filled with data coming
> @@ -444,55 +506,55 @@ fcache::~fcache ()
>     current line is empty.  Note that if the cache is full, it would
>     need to be extended and filled again.  */
>
> -static bool
> -needs_read (fcache *c)
> +bool
> +file_cache_slot::needs_read_p () const
>  {
> -  return (c->nb_read == 0
> -         || c->nb_read == c->size
> -         || (c->line_start_idx >= c->nb_read - 1));
> +  return (m_nb_read == 0
> +         || m_nb_read == m_size
> +         || (m_line_start_idx >= m_nb_read - 1));
>  }
>
>  /*  Return TRUE iff the cache is full and thus needs to be
>      extended.  */
>
> -static bool
> -needs_grow (fcache *c)
> +bool
> +file_cache_slot::needs_grow_p () const
>  {
> -  return c->nb_read == c->size;
> +  return m_nb_read == m_size;
>  }
>
>  /* Grow the cache if it needs to be extended.  */
>
> -static void
> -maybe_grow (fcache *c)
> +void
> +file_cache_slot::maybe_grow ()
>  {
> -  if (!needs_grow (c))
> +  if (!needs_grow_p ())
>      return;
>
> -  size_t size = c->size == 0 ? fcache_buffer_size : c->size * 2;
> -  c->data = XRESIZEVEC (char, c->data, size);
> -  c->size = size;
> +  size_t size = m_size == 0 ? buffer_size : m_size * 2;
> +  m_data = XRESIZEVEC (char, m_data, size);
> +  m_size = size;
>  }
>
>  /*  Read more data into the cache.  Extends the cache if need be.
>      Returns TRUE iff new data could be read.  */
>
> -static bool
> -read_data (fcache *c)
> +bool
> +file_cache_slot::read_data ()
>  {
> -  if (feof (c->fp) || ferror (c->fp))
> +  if (feof (m_fp) || ferror (m_fp))
>      return false;
>
> -  maybe_grow (c);
> +  maybe_grow ();
>
> -  char * from = c->data + c->nb_read;
> -  size_t to_read = c->size - c->nb_read;
> -  size_t nb_read = fread (from, 1, to_read, c->fp);
> +  char * from = m_data + m_nb_read;
> +  size_t to_read = m_size - m_nb_read;
> +  size_t nb_read = fread (from, 1, to_read, m_fp);
>
> -  if (ferror (c->fp))
> +  if (ferror (m_fp))
>      return false;
>
> -  c->nb_read += nb_read;
> +  m_nb_read += nb_read;
>    return !!nb_read;
>  }
>
> @@ -500,12 +562,12 @@ read_data (fcache *c)
>     coming from the file FP.  Return TRUE iff the cache was filled with
>     mode data.  */
>
> -static bool
> -maybe_read_data (fcache *c)
> +bool
> +file_cache_slot::maybe_read_data ()
>  {
> -  if (!needs_read (c))
> +  if (!needs_read_p ())
>      return false;
> -  return read_data (c);
> +  return read_data ();
>  }
>
>  /* Read a new line from file FP, using C as a cache for the data
> @@ -518,18 +580,18 @@ maybe_read_data (fcache *c)
>     otherwise.  Note that subsequent calls to get_next_line might
>     make the content of *LINE invalid.  */
>
> -static bool
> -get_next_line (fcache *c, char **line, ssize_t *line_len)
> +bool
> +file_cache_slot::get_next_line (char **line, ssize_t *line_len)
>  {
>    /* Fill the cache with data to process.  */
> -  maybe_read_data (c);
> +  maybe_read_data ();
>
> -  size_t remaining_size = c->nb_read - c->line_start_idx;
> +  size_t remaining_size = m_nb_read - m_line_start_idx;
>    if (remaining_size == 0)
>      /* There is no more data to process.  */
>      return false;
>
> -  char *line_start = c->data + c->line_start_idx;
> +  char *line_start = m_data + m_line_start_idx;
>
>    char *next_line_start = NULL;
>    size_t len = 0;
> @@ -539,10 +601,10 @@ get_next_line (fcache *c, char **line, ssize_t *line_len)
>        /* We haven't found the end-of-line delimiter in the cache.
>          Fill the cache with more data from the file and look for the
>          '\n'.  */
> -      while (maybe_read_data (c))
> +      while (maybe_read_data ())
>         {
> -         line_start = c->data + c->line_start_idx;
> -         remaining_size = c->nb_read - c->line_start_idx;
> +         line_start = m_data + m_line_start_idx;
> +         remaining_size = m_nb_read - m_line_start_idx;
>           line_end = (char *) memchr (line_start, '\n', remaining_size);
>           if (line_end != NULL)
>             {
> @@ -558,19 +620,19 @@ get_next_line (fcache *c, char **line, ssize_t *line_len)
>              of when the line ends up with a '\n' and line_end points to
>              that terminal '\n'.  That consistency is useful below in
>              the len calculation.  */
> -         line_end = c->data + c->nb_read ;
> -         c->missing_trailing_newline = true;
> +         line_end = m_data + m_nb_read ;
> +         m_missing_trailing_newline = true;
>         }
>        else
> -       c->missing_trailing_newline = false;
> +       m_missing_trailing_newline = false;
>      }
>    else
>      {
>        next_line_start = line_end + 1;
> -      c->missing_trailing_newline = false;
> +      m_missing_trailing_newline = false;
>      }
>
> -  if (ferror (c->fp))
> +  if (ferror (m_fp))
>      return false;
>
>    /* At this point, we've found the end of the of line.  It either
> @@ -580,54 +642,56 @@ get_next_line (fcache *c, char **line, ssize_t *line_len)
>
>    len = line_end - line_start;
>
> -  if (c->line_start_idx < c->nb_read)
> +  if (m_line_start_idx < m_nb_read)
>      *line = line_start;
>
> -  ++c->line_num;
> +  ++m_line_num;
>
>    /* Before we update our line record, make sure the hint about the
>       total number of lines of the file is correct.  If it's not, then
>       we give up recording line boundaries from now on.  */
>    bool update_line_record = true;
> -  if (c->line_num > c->total_lines)
> +  if (m_line_num > m_total_lines)
>      update_line_record = false;
>
>      /* Now update our line record so that re-reading lines from the
> -     before c->line_start_idx is faster.  */
> +     before m_line_start_idx is faster.  */
>    if (update_line_record
> -      && c->line_record.length () < fcache_line_record_size)
> +      && m_line_record.length () < line_record_size)
>      {
>        /* If the file lines fits in the line record, we just record all
>          its lines ...*/
> -      if (c->total_lines <= fcache_line_record_size
> -         && c->line_num > c->line_record.length ())
> -       c->line_record.safe_push (fcache::line_info (c->line_num,
> -                                                c->line_start_idx,
> -                                                line_end - c->data));
> -      else if (c->total_lines > fcache_line_record_size)
> +      if (m_total_lines <= line_record_size
> +         && m_line_num > m_line_record.length ())
> +       m_line_record.safe_push
> +         (file_cache_slot::line_info (m_line_num,
> +                                      m_line_start_idx,
> +                                      line_end - m_data));
> +      else if (m_total_lines > line_record_size)
>         {
>           /* ... otherwise, we just scale total_lines down to
> -            (fcache_line_record_size lines.  */
> -         size_t n = (c->line_num * fcache_line_record_size) / c->total_lines;
> -         if (c->line_record.length () == 0
> -             || n >= c->line_record.length ())
> -           c->line_record.safe_push (fcache::line_info (c->line_num,
> -                                                    c->line_start_idx,
> -                                                    line_end - c->data));
> +            (line_record_size lines.  */
> +         size_t n = (m_line_num * line_record_size) / m_total_lines;
> +         if (m_line_record.length () == 0
> +             || n >= m_line_record.length ())
> +           m_line_record.safe_push
> +             (file_cache_slot::line_info (m_line_num,
> +                                          m_line_start_idx,
> +                                          line_end - m_data));
>         }
>      }
>
> -  /* Update c->line_start_idx so that it points to the next line to be
> +  /* Update m_line_start_idx so that it points to the next line to be
>       read.  */
>    if (next_line_start)
> -    c->line_start_idx = next_line_start - c->data;
> +    m_line_start_idx = next_line_start - m_data;
>    else
>      /* We didn't find any terminal '\n'.  Let's consider that the end
>         of line is the end of the data in the cache.  The next
>         invocation of get_next_line will either read more data from the
>         underlying file or return false early because we've reached the
>         end of the file.  */
> -    c->line_start_idx = c->nb_read;
> +    m_line_start_idx = m_nb_read;
>
>    *line_len = len;
>
> @@ -640,13 +704,13 @@ get_next_line (fcache *c, char **line, ssize_t *line_len)
>     copying from the cache involved.  Return TRUE upon successful
>     completion.  */
>
> -static bool
> -goto_next_line (fcache *cache)
> +bool
> +file_cache_slot::goto_next_line ()
>  {
>    char *l;
>    ssize_t len;
>
> -  return get_next_line (cache, &l, &len);
> +  return get_next_line (&l, &len);
>  }
>
>  /* Read an arbitrary line number LINE_NUM from the file cached in C.
> @@ -656,54 +720,54 @@ goto_next_line (fcache *cache)
>     *LINE is only valid until the next call of read_line_num.
>     This function returns bool if a line was read.  */
>
> -static bool
> -read_line_num (fcache *c, size_t line_num,
> -              char **line, ssize_t *line_len)
> +bool
> +file_cache_slot::read_line_num (size_t line_num,
> +                      char ** line, ssize_t *line_len)
>  {
>    gcc_assert (line_num > 0);
>
> -  if (line_num <= c->line_num)
> +  if (line_num <= m_line_num)
>      {
> -      /* We've been asked to read lines that are before c->line_num.
> +      /* We've been asked to read lines that are before m_line_num.
>          So lets use our line record (if it's not empty) to try to
>          avoid re-reading the file from the beginning again.  */
>
> -      if (c->line_record.is_empty ())
> +      if (m_line_record.is_empty ())
>         {
> -         c->line_start_idx = 0;
> -         c->line_num = 0;
> +         m_line_start_idx = 0;
> +         m_line_num = 0;
>         }
>        else
>         {
> -         fcache::line_info *i = NULL;
> -         if (c->total_lines <= fcache_line_record_size)
> +         file_cache_slot::line_info *i = NULL;
> +         if (m_total_lines <= line_record_size)
>             {
>               /* In languages where the input file is not totally
> -                preprocessed up front, the c->total_lines hint
> +                preprocessed up front, the m_total_lines hint
>                  can be smaller than the number of lines of the
>                  file.  In that case, only the first
> -                c->total_lines have been recorded.
> +                m_total_lines have been recorded.
>
> -                Otherwise, the first c->total_lines we've read have
> +                Otherwise, the first m_total_lines we've read have
>                  their start/end recorded here.  */
> -             i = (line_num <= c->total_lines)
> -               ? &c->line_record[line_num - 1]
> -               : &c->line_record[c->total_lines - 1];
> +             i = (line_num <= m_total_lines)
> +               ? &m_line_record[line_num - 1]
> +               : &m_line_record[m_total_lines - 1];
>               gcc_assert (i->line_num <= line_num);
>             }
>           else
>             {
>               /*  So the file had more lines than our line record
>                   size.  Thus the number of lines we've recorded has
> -                 been scaled down to fcache_line_reacord_size.  Let's
> +                 been scaled down to line_record_size.  Let's
>                   pick the start/end of the recorded line that is
>                   closest to line_num.  */
> -             size_t n = (line_num <= c->total_lines)
> -               ? line_num * fcache_line_record_size / c->total_lines
> -               : c ->line_record.length () - 1;
> -             if (n < c->line_record.length ())
> +             size_t n = (line_num <= m_total_lines)
> +               ? line_num * line_record_size / m_total_lines
> +               : m_line_record.length () - 1;
> +             if (n < m_line_record.length ())
>                 {
> -                 i = &c->line_record[n];
> +                 i = &m_line_record[n];
>                   gcc_assert (i->line_num <= line_num);
>                 }
>             }
> @@ -711,33 +775,33 @@ read_line_num (fcache *c, size_t line_num,
>           if (i && i->line_num == line_num)
>             {
>               /* We have the start/end of the line.  */
> -             *line = c->data + i->start_pos;
> +             *line = m_data + i->start_pos;
>               *line_len = i->end_pos - i->start_pos;
>               return true;
>             }
>
>           if (i)
>             {
> -             c->line_start_idx = i->start_pos;
> -             c->line_num = i->line_num - 1;
> +             m_line_start_idx = i->start_pos;
> +             m_line_num = i->line_num - 1;
>             }
>           else
>             {
> -             c->line_start_idx = 0;
> -             c->line_num = 0;
> +             m_line_start_idx = 0;
> +             m_line_num = 0;
>             }
>         }
>      }
>
> -  /*  Let's walk from line c->line_num up to line_num - 1, without
> +  /*  Let's walk from line m_line_num up to line_num - 1, without
>        copying any line.  */
> -  while (c->line_num < line_num - 1)
> -    if (!goto_next_line (c))
> +  while (m_line_num < line_num - 1)
> +    if (!goto_next_line ())
>        return false;
>
>    /* The line we want is the next one.  Let's read and copy it back to
>       the caller.  */
> -  return get_next_line (c, line, line_len);
> +  return get_next_line (line, line_len);
>  }
>
>  /* Return the physical source line that corresponds to FILE_PATH/LINE.
> @@ -756,11 +820,16 @@ location_get_source_line (const char *file_path, int line)
>    if (line == 0)
>      return char_span (NULL, 0);
>
> -  fcache *c = lookup_or_add_file_to_cache_tab (file_path);
> +  if (file_path == NULL)
> +    return char_span (NULL, 0);
> +
> +  diagnostic_file_cache_init ();
> +
> +  file_cache_slot *c = global_dc->m_file_cache->lookup_or_add_file (file_path);
>    if (c == NULL)
>      return char_span (NULL, 0);
>
> -  bool read = read_line_num (c, line, &buffer, &len);
> +  bool read = c->read_line_num (line, &buffer, &len);
>    if (!read)
>      return char_span (NULL, 0);
>
> @@ -774,11 +843,13 @@ location_get_source_line (const char *file_path, int line)
>  bool
>  location_missing_trailing_newline (const char *file_path)
>  {
> -  fcache *c = lookup_or_add_file_to_cache_tab (file_path);
> +  diagnostic_file_cache_init ();
> +
> +  file_cache_slot *c = global_dc->m_file_cache->lookup_or_add_file (file_path);
>    if (c == NULL)
>      return false;
>
> -  return c->missing_trailing_newline;
> +  return c->missing_trailing_newline_p ();
>  }
>
>  /* Test if the location originates from the spelling location of a
> diff --git a/gcc/input.h b/gcc/input.h
> index f1ef5d76cfd..bbcec84c521 100644
> --- a/gcc/input.h
> +++ b/gcc/input.h
> @@ -88,6 +88,39 @@ class char_span
>  extern char_span location_get_source_line (const char *file_path, int line);
>
>  extern bool location_missing_trailing_newline (const char *file_path);
> +
> +/* Forward decl of slot within file_cache, so that the definition doesn't
> +   need to be in this header.  */
> +class file_cache_slot;
> +
> +/* A cache of source files for use when emitting diagnostics
> +   (and in a few places in the C/C++ frontends).
> +
> +   Results are only valid until the next call to the cache, as
> +   slots can be evicted.
> +
> +   Filenames are stored by pointer, and so must outlive the cache
> +   instance.  */
> +
> +class file_cache
> +{
> + public:
> +  file_cache ();
> +  ~file_cache ();
> +
> +  file_cache_slot *lookup_or_add_file (const char *file_path);
> +  void forcibly_evict_file (const char *file_path);
> +
> + private:
> +  file_cache_slot *evicted_cache_tab_entry (unsigned *highest_use_count);
> +  file_cache_slot *add_file (const char *file_path);
> +  file_cache_slot *lookup_file (const char *file_path);
> +
> + private:
> +  static const size_t num_file_slots = 16;
> +  file_cache_slot *m_file_slots;
> +};
> +
>  extern expanded_location
>  expand_location_to_spelling_point (location_t,
>                                    enum location_aspect aspect
> --
> 2.26.3
>

  reply	other threads:[~2021-07-11 16:58 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-30  5:35 [PATCH 1/4] add utility to poison globals that should not be used Trevor Saunders
2021-06-30  5:35 ` [PATCH 2/4] allow poisoning input_location in ranges it " Trevor Saunders
2021-06-30  9:00   ` Richard Biener
2021-06-30 12:33     ` Trevor Saunders
2021-06-30 19:09       ` Richard Biener
2021-07-01 10:23         ` Trevor Saunders
2021-07-01 12:48           ` Richard Biener
2021-06-30 15:13   ` David Malcolm
2021-06-30 19:34     ` Jason Merrill
2021-07-01 10:16     ` Trevor Saunders
2021-07-01 12:53       ` Richard Biener
2021-07-01 15:40         ` David Malcolm
2021-07-01 16:04           ` David Malcolm
2021-07-01 21:51             ` [committed] input.c: move file caching globals to a new file_cache class David Malcolm
2021-07-11 16:58               ` Lewis Hyatt [this message]
2021-07-14 22:53                 ` David Malcolm
2021-07-02  0:44           ` [PATCH 2/4] allow poisoning input_location in ranges it should not be used Trevor Saunders
2021-07-02 15:46       ` Jason Merrill
2021-07-02 23:23         ` Trevor Saunders
2021-07-02 19:20   ` Martin Sebor
2021-07-02 23:47     ` Trevor Saunders
2021-07-06 20:53       ` Martin Sebor
2021-06-30  5:35 ` [PATCH 3/4] allow poisoning cfun Trevor Saunders
2021-06-30  5:35 ` [PATCH 4/4] poison input_location and cfun in one spot Trevor Saunders
2021-06-30  9:02   ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAA_5UQ6p58TMXDXpAseY3Ahk9iHYcf2Z1me5tzjab_FFxQOYag@mail.gmail.com \
    --to=lhyatt@gmail.com \
    --cc=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).