On Tue, 5 Nov 2019, Jan Hubicka wrote:
> Hi,
> here is updated patch. Now the hash is freed though it is not
> extraordinarily large (for firefox about 20k entries which roughly
> correspond to number of source files)
>
> Bootstrapped/regtested x86_64-linux. OK?
OK.
Note I was arguing that the "string_slot" indirection should go away
and we should put the whole string_slot into the hash-table rather
than just a pointer to a string_slot. We're using it in quite a few
places (some allocating the string_slot from an obstack). That would
have simplified the lto-streamer-in.c part of the patch but of course
requires surgery elsewhere (unless you make such an embedded hasher
just for the file location use).
> * lto-streamer-in.c: Include alloc-pool.h.
> (freeing_string_slot_hasher): Remove.
> (string_slot_allocator): New object allocator.
> (file_name_hash_table): Turn to hash_table<string_slot_hasher>.
> (file_name_obstack): New obstack.
> (canon_file_name): Allocate in obstack and allocator.
> (lto_reader_init): Initialize obstack and allocator.
> (lto_free_file_name_hash): New function.
> * lto-streamer.h (lto_free_file_name_hash): New.
> * lto.c (do_whole_program_analysis): Call lto_free_file_name_hash.
> Index: lto-streamer-in.c
> ===================================================================
> --- lto-streamer-in.c (revision 277796)
> +++ lto-streamer-in.c (working copy)
> @@ -42,21 +42,18 @@ along with GCC; see the file COPYING3.
> #include "cgraph.h"
> #include "cfgloop.h"
> #include "debug.h"
> +#include "alloc-pool.h"
>
> -
> -struct freeing_string_slot_hasher : string_slot_hasher
> -{
> - static inline void remove (value_type *);
> -};
> -
> -inline void
> -freeing_string_slot_hasher::remove (value_type *v)
> -{
> - free (v);
> -}
> +/* Allocator used to hold string slot entries for line map streaming. */
> +static struct object_allocator<struct string_slot> *string_slot_allocator;
>
> /* The table to hold the file names. */
> -static hash_table<freeing_string_slot_hasher> *file_name_hash_table;
> +static hash_table<string_slot_hasher> *file_name_hash_table;
> +
> +/* This obstack holds file names used in locators. Line map datastructures
> + points here and thus it needs to be kept allocated as long as linemaps
> + exists. */
> +static struct obstack file_name_obstack;
>
>
> /* Check that tag ACTUAL has one of the given values. NUM_TAGS is the
> @@ -113,8 +110,8 @@ canon_file_name (const char *string)
> char *saved_string;
> struct string_slot *new_slot;
>
> - saved_string = (char *) xmalloc (len + 1);
> - new_slot = XCNEW (struct string_slot);
> + saved_string = XOBNEWVEC (&file_name_obstack, char, len + 1);
> + new_slot = string_slot_allocator->allocate ();
> memcpy (saved_string, string, len + 1);
> new_slot->s = saved_string;
> new_slot->len = len;
> @@ -1722,7 +1719,23 @@ lto_reader_init (void)
> {
> lto_streamer_init ();
> file_name_hash_table
> - = new hash_table<freeing_string_slot_hasher> (37);
> + = new hash_table<string_slot_hasher> (37);
> + string_slot_allocator = new object_allocator <struct string_slot>
> + ("line map file name hash");
> + gcc_obstack_init (&file_name_obstack);
> +}
> +
> +/* Free hash table used to stream in location file names. */
> +
> +void
> +lto_free_file_name_hash (void)
> +{
> + delete file_name_hash_table;
> + file_name_hash_table = NULL;
> + delete string_slot_allocator;
> + string_slot_allocator = NULL;
> + /* file_name_obstack must stay allocated since it is referred to by
> + line map table. */
> }
>
>
> Index: lto-streamer.h
> ===================================================================
> --- lto-streamer.h (revision 277796)
> +++ lto-streamer.h (working copy)
> @@ -874,6 +874,7 @@ extern void lto_streamer_hooks_init (voi
> /* In lto-streamer-in.c */
> extern void lto_input_cgraph (struct lto_file_decl_data *, const char *);
> extern void lto_reader_init (void);
> +extern void lto_free_file_name_hash (void);
> extern void lto_input_function_body (struct lto_file_decl_data *,
> struct cgraph_node *,
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c (revision 277796)
> +++ lto/lto.c (working copy)
> @@ -475,6 +476,9 @@ do_whole_program_analysis (void)
> /* We are about to launch the final LTRANS phase, stop the WPA timer. */
> timevar_pop (TV_WHOPR_WPA);
>
> + /* We are no longer going to stream in anything. Free some memory. */
> + lto_free_file_name_hash ();
> +
> timevar_push (TV_WHOPR_PARTITIONING);
>
> gcc_assert (!dump_file);
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
