Re: [PATCH] Use mapped file name for symbols

[Joshua Watt]

On Fri, Feb 26, 2021, 12:19 AM Akim Demaille <akim@lrde.epita.fr> wrote:

> Hi Joshua,
>
> > Le 25 févr. 2021 à 08:44, Akim Demaille <akim@lrde.epita.fr> a écrit :
> >
> > I don't have time to finish this now, but this message is to make sure
> we aren't both working on it.  Here's what I have currently.  It appears to
> work.
>
> I'm going to install this.  Cheers!


Excellent! Thanks


> commit f6eb1ac87bd08f2231fc28b14129c1b458da3bdf
> Author: Akim Demaille <akim.demaille@gmail.com>
> Date:   Thu Feb 25 08:31:50 2021 +0100
>
>     output: cache the mapped file names
>
>     Don't repeatedly call malloc/free for each call to map_file_name.
>
>     * bootstrap.conf: We need hash-map.
>     * src/files.h, src/files.c (map_file_name): The caller must not free
>     the result.
>     Adjust callers.
>     (mapped_dir_prefix, spec_mapped_header_file): Remove.
>     * src/files.c
>     (map_file_name): Rename as...
>     (map_file_name_alloc): this.
>     (mapped_files, map_file_name, string_equals, string_hash, string_free):
>     New.
>
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 51777601b..6f1a3fc61 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -27,6 +27,7 @@ gnulib_modules='
>    getopt-gnu
>    gettext-h git-version-gen gitlog-to-changelog
>    gpl-3.0 intprops inttypes isnan javacomp-script
> +  hash-map
>    javaexec-script
>    ldexpl
>    libtextstyle-optional
> @@ -58,7 +59,7 @@ gnulib_modules='
>    xconcat-filename
>    xhash
>    xlist
> -  xmemdup0
> +  xmap xmemdup0
>    xstrndup
>
>    fprintf-posix printf-posix snprintf-posix sprintf-posix
> diff --git a/lib/.gitignore b/lib/.gitignore
> index 5d7f60e5b..00843d68f 100644
> --- a/lib/.gitignore
> +++ b/lib/.gitignore
> @@ -162,10 +162,14 @@
>  /gl_anytreehash_list2.h
>  /gl_array_list.c
>  /gl_array_list.h
> +/gl_hash_map.c
> +/gl_hash_map.h
>  /gl_linked_list.c
>  /gl_linked_list.h
>  /gl_list.c
>  /gl_list.h
> +/gl_map.c
> +/gl_map.h
>  /gl_oset.c
>  /gl_oset.h
>  /gl_rbtree_ordered.h
> @@ -175,6 +179,8 @@
>  /gl_rbtreehash_list.h
>  /gl_xlist.c
>  /gl_xlist.h
> +/gl_xmap.c
> +/gl_xmap.h
>  /gnulib.mk
>  /hard-locale.c
>  /hard-locale.h
> diff --git a/src/files.c b/src/files.c
> index 72652f426..04f711971 100644
> --- a/src/files.c
> +++ b/src/files.c
> @@ -26,7 +26,9 @@
>  #include <error.h>
>  #include <get-errno.h>
>  #include <gl_array_list.h>
> +#include <gl_hash_map.h>
>  #include <gl_xlist.h>
> +#include <gl_xmap.h>
>  #include <quote.h>
>  #include <quotearg.h>
>  #include <relocatable.h> /* relocate2 */
> @@ -58,7 +60,6 @@ char *spec_graph_file = NULL;    /* for -g. */
>  char *spec_html_file = NULL;     /* for --html. */
>  char *spec_xml_file = NULL;      /* for -x. */
>  char *spec_header_file = NULL;   /* for --header. */
> -char *spec_mapped_header_file = NULL;
>  char *parser_file_name;
>
>  /* All computed output file names.  */
> @@ -95,7 +96,6 @@ uniqstr grammar_file = NULL;
>  char *all_but_ext;
>  static char *all_but_tab_ext;
>  char *dir_prefix;
> -char *mapped_dir_prefix;
>
>  /* C source file extension (the parser source).  */
>  static char *src_extension = NULL;
> @@ -109,6 +109,10 @@ struct prefix_map
>  };
>
>  static gl_list_t prefix_maps = NULL;
> +
> +/* Map file names to prefix-mapped file names. */
> +static gl_map_t mapped_files = NULL;
> +
>
>  /*-----------------------------------------------------------------.
>  | Return a newly allocated string composed of the concatenation of |
> @@ -172,29 +176,25 @@ xfdopen (int fd, char const *mode)
>    return res;
>  }
>
> -/*  Given an input file path, returns a dynamically allocated string that
> -    contains the path with the file prefix mapping rules applied, or NULL
> if
> -    the input was NULL. */
> -char *
> -map_file_name (char const *filename)
> +/* The mapped name of FILENAME, allocated, if there are prefix maps.
> +   Otherwise NULL.  */
> +static char *
> +map_file_name_alloc (char const *filename)
>  {
> -  if (!filename)
> -    return NULL;
> -
>    struct prefix_map const *p = NULL;
> -  if (prefix_maps)
> -    {
> -      void const *ptr;
> -      gl_list_iterator_t iter = gl_list_iterator (prefix_maps);
> -      while (gl_list_iterator_next (&iter, &ptr, NULL))
> -        {
> -          p = ptr;
> -          if (strncmp (p->oldprefix, filename, strlen (p->oldprefix)) ==
> 0)
> -            break;
> -          p = NULL;
> -        }
> -      gl_list_iterator_free (&iter);
> -    }
> +  assert (prefix_maps);
> +  {
> +    void const *ptr;
> +    gl_list_iterator_t iter = gl_list_iterator (prefix_maps);
> +    while (gl_list_iterator_next (&iter, &ptr, NULL))
> +      {
> +        p = ptr;
> +        if (strncmp (p->oldprefix, filename, strlen (p->oldprefix)) == 0)
> +          break;
> +        p = NULL;
> +      }
> +    gl_list_iterator_free (&iter);
> +  }
>
>    if (!p)
>      return xstrdup (filename);
> @@ -209,6 +209,56 @@ map_file_name (char const *filename)
>    return res;
>  }
>
> +static bool
> +string_equals (const void *x1, const void *x2)
> +{
> +  const char *s1 = x1;
> +  const char *s2 = x2;
> +  return STREQ (s1, s2);
> +}
> +
> +/* A hash function for NUL-terminated char* strings using
> +   the method described by Bruno Haible.
> +   See https://www.haible.de/bruno/hashfunc.html.  */
> +static size_t
> +string_hash (const void *x)
> +{
> +#define SIZE_BITS (sizeof (size_t) * CHAR_BIT)
> +
> +  const char *s = x;
> +  size_t h = 0;
> +
> +  for (; *s; s++)
> +    h = *s + ((h << 9) | (h >> (SIZE_BITS - 9)));
> +
> +  return h;
> +}
> +
> +static void
> +string_free (const void *p)
> +{
> +  free ((void*) p);
> +}
> +
> +const char *
> +map_file_name (char const *filename)
> +{
> +  if (!filename || !prefix_maps)
> +    return filename;
> +  if (!mapped_files)
> +    mapped_files
> +      = gl_map_nx_create_empty (GL_HASH_MAP,
> +                                string_equals, string_hash,
> +                                string_free, string_free);
> +  const void *res = gl_map_get (mapped_files, filename);
> +  if (!res)
> +    {
> +      res = map_file_name_alloc (filename);
> +      gl_map_put (mapped_files, xstrdup (filename), res);
> +    }
> +  return res;
> +}
> +
>  static void
>  prefix_map_free (struct prefix_map *p)
>  {
> @@ -445,9 +495,6 @@ compute_output_file_names (void)
>        output_file_name_check (&spec_verbose_file, false);
>      }
>
> -  spec_mapped_header_file = map_file_name (spec_header_file);
> -  mapped_dir_prefix = map_file_name (dir_prefix);
> -
>    free (all_but_tab_ext);
>    free (src_extension);
>    free (header_extension);
> @@ -535,10 +582,8 @@ output_file_names_free (void)
>    free (spec_html_file);
>    free (spec_xml_file);
>    free (spec_header_file);
> -  free (spec_mapped_header_file);
>    free (parser_file_name);
>    free (dir_prefix);
> -  free (mapped_dir_prefix);
>    for (int i = 0; i < generated_files_size; i++)
>      free (generated_files[i].name);
>    free (generated_files);
> @@ -546,4 +591,6 @@ output_file_names_free (void)
>
>    if (prefix_maps)
>      gl_list_free (prefix_maps);
> +  if (mapped_files)
> +    gl_map_free (mapped_files);
>  }
> diff --git a/src/files.h b/src/files.h
> index d49847abc..385847739 100644
> --- a/src/files.h
> +++ b/src/files.h
> @@ -53,15 +53,9 @@ extern char *spec_xml_file;
>  /* File name specified with --header.  */
>  extern char *spec_header_file;
>
> -/* File name specified with --header, adjusted for mapped prefixes. */
> -extern char *spec_mapped_header_file;
> -
>  /* Directory prefix of output file names.  */
>  extern char *dir_prefix;
>
> -/* Directory prefix of output file name, adjusted for mapped prefixes. */
> -extern char *mapped_dir_prefix;
> -
>  /* The file name as given on the command line.
>     Not named "input_file" because Flex uses this name for an argument,
>     and therefore GCC warns about a name clash. */
> @@ -94,7 +88,10 @@ FILE *xfopen (const char *name, char const *mode);
>  void xfclose (FILE *ptr);
>  FILE *xfdopen (int fd, char const *mode);
>
> -char *map_file_name (char const *filename);
> +/* Given an input file path, return a string that contains the path
> +   with the file prefix mapping rules applied, or NULL if the input
> +   was NULL.  Do not free the return value.  */
> +const char *map_file_name (char const *filename);
>
>  /* Add a new file prefix mapping. If a file path starts with
>     oldprefix, it will be replaced with newprefix.  */
> diff --git a/src/muscle-tab.c b/src/muscle-tab.c
> index 3e7657ca1..0654a3f10 100644
> --- a/src/muscle-tab.c
> +++ b/src/muscle-tab.c
> @@ -204,10 +204,8 @@ static void
>  muscle_syncline_grow (char const *key, location loc)
>  {
>    obstack_printf (&muscle_obstack, "]b4_syncline(%d, ", loc.start.line);
> -  char *f = map_file_name (loc.start.file);
>    obstack_quote (&muscle_obstack,
> -                 quotearg_style (c_quoting_style, f));
> -  free (f);
> +                 quotearg_style (c_quoting_style, map_file_name
> (loc.start.file)));
>    obstack_sgrow (&muscle_obstack, ")dnl\n[");
>    char const *extension = obstack_finish0 (&muscle_obstack);
>    muscle_grow (key, extension, "", "");
> diff --git a/src/output.c b/src/output.c
> index 34dbc6714..4b1bb0d8e 100644
> --- a/src/output.c
> +++ b/src/output.c
> @@ -531,9 +531,7 @@ user_actions_output (FILE *out)
>            {
>              fprintf (out, "b4_syncline(%d, ",
>                       rules[r].action_loc.start.line);
> -            char *f = map_file_name (rules[r].action_loc.start.file);
> -            string_output (out, f);
> -            free(f);
> +            string_output (out, map_file_name
> (rules[r].action_loc.start.file));
>              fprintf (out, ")dnl\n");
>            }
>          fprintf (out, "[%*s%s]],\n[[",
> @@ -631,10 +629,8 @@ prepare_symbol_definitions (void)
>
>            if (p->code)
>              {
> -              char *f = map_file_name (p->location.start.file);
>                SET_KEY2 (pname, "file");
> -              MUSCLE_INSERT_C_STRING (key, f);
> -              free (f);
> +              MUSCLE_INSERT_C_STRING (key, map_file_name
> (p->location.start.file));
>
>                SET_KEY2 (pname, "line");
>                MUSCLE_INSERT_INT (key, p->location.start.line);
> @@ -852,6 +848,9 @@ prepare (void)
>
>    MUSCLE_INSERT_STRING ("file_name_all_but_ext", all_but_ext);
>
> +  const char *spec_mapped_header_file = map_file_name (spec_header_file);
> +  const char *mapped_dir_prefix = map_file_name (dir_prefix);
> +
>  #define DEFINE(Name) MUSCLE_INSERT_STRING (#Name, Name ? Name : "")
>    DEFINE (dir_prefix);
>    DEFINE (mapped_dir_prefix);
> diff --git a/src/scan-skel.l b/src/scan-skel.l
> index 1ec125af5..20049cfb3 100644
> --- a/src/scan-skel.l
> +++ b/src/scan-skel.l
> @@ -269,7 +269,5 @@ fail_for_invalid_at (char const *at)
>  static void
>  output_mapped_file (char const *name)
>  {
> -  char *f = map_file_name (name);
> -  fputs (quotearg_style (c_quoting_style, f), yyout);
> -  free (f);
> +  fputs (quotearg_style (c_quoting_style, map_file_name (name)), yyout);
>  }
>
>