Patch: identifier GC, take 2

[Ian Lance Taylor]

Tom Tromey <tromey@redhat.com> writes:

> libcpp/ChangeLog:
> 2008-04-04  Tom Tromey  <tromey@redhat.com>
>
> 	* include/symtab.h (HT_ALLOCED): Remove.
> 	(ht_purge): Declare.
> 	* symtab.c (DELETED): New define.
> 	(ht_lookup): Update comment.
> 	(ht_lookup_with_hash): Handle deleted entries.  Remove HT_ALLOCED
> 	code.  Use subobject allocator for strings, if it exists.
> 	(ht_expand): Handle deleted entries.
> 	(ht_forall): Likewise.
> 	(ht_purge): New function.
> 	(ht_dump_statistics): Print deletion statistics.
>
> gcc/ChangeLog:
> 2008-04-04  Tom Tromey  <tromey@redhat.com>
>
> 	* ggc-page.c (gt_ggc_m_S): New function.
> 	* stringpool.c (string_stack): Remove.
> 	(init_stringpool): Update.
> 	(ggc_alloc_string): Use ggc_alloc.
> 	(maybe_delete_ident): New function.
> 	(ggc_mark_stringpool): Add 'marking' argument.  Purge hash table
> 	when asked.
> 	(gt_ggc_m_S): Remove.
> 	* ggc-common.c (ggc_protect_identifiers): New global.
> 	(ggc_mark_roots): Update calls to ggc_mark_stringpool.
> 	* ggc.h (ggc_protect_identifiers): Declare.
> 	(ggc_mark_stringpool): Update.
> 	(gt_ggc_m_S): Update.
> 	* toplev.c (compile_file): Set and reset ggc_protect_identifiers.
> 	* gengtype.c (write_types_process_field) <TYPE_STRING>: Remove
> 	special case.
> 	(write_root): Cast gt_ggc_m_S to gt_pointer_walker.
>
> gcc/cp/ChangeLog:
> 2008-03-14  Tom Tromey  <tromey@redhat.com>
>
> 	* mangle.c (save_partially_mangled_name): Remove.
> 	(restore_partially_mangled_name): Likewise.
> 	(write_encoding): Update.
> 	(write_unqualified_name): Likewise.
> 	(start_mangling): Always use name_obstack.  Remove 'ident_p'
> 	argument.
> 	(get_identifier_nocopy): Remove.
> 	(finish_mangling_internal): Rename from finish_mangling.
> 	(finish_mangling): New function.
> 	(finish_mangling_get_identifier): Likewise.
> 	(partially_mangled_name, partially_mangled_name_len): Remove.
> 	(mangle_decl_string): Change return type.  Update.
> 	(mangle_decl, mangle_type_string, mangle_special_for_type,
> 	mangle_ctor_vtbl_for_type, mangle_thunk, mangle_guard_variable,
> 	mangle_ref_init_variable): Update.



> +  if (table->alloc_subobject)
> +    {
> +      char *chars = table->alloc_subobject (len + 1);
> +      strncpy ((char *) chars, (char *) str, len);
> +      chars[len] = '\0';
> +      HT_STR (node) = (const unsigned char *) chars;
> +    }

I think that memcpy is better than strncpy here--both faster and more
obviously correct.


> +/* When true, protect the contents of the identifier hash table.  */
> +extern bool ggc_protect_identifiers;

I think this comment needs to be expanded.  When this is true, we
treat the identifier hash table as a GC root.  So this is false when
we will no longer be looking anything up in the hash table.

I think it would be appropriate to add an assert to get_identifier and
get_identifier_with_length: as far as I can see, they should never be
called when ggc_protect_identifiers is false.


> @@ -103,7 +106,8 @@
>        for (i = 0; i < rti->nelt; i++)
>  	(*rti->cb)(*(void **)((char *)rti->base + rti->stride * i));
>  
> -  ggc_mark_stringpool ();
> +  if (ggc_protect_identifiers)
> +    ggc_mark_stringpool (true);
>  
>    /* Now scan all hash tables that have objects which are to be deleted if
>       they are not already marked.  */
> @@ -115,6 +119,9 @@
>  	  htab_traverse_noresize (*cti->base, ggc_htab_delete, (void *) cti);
>  	  ggc_set_mark ((*cti->base)->entries);
>  	}
> +
> +  if (! ggc_protect_identifiers)
> +    ggc_mark_stringpool (false);
>  }

You are using the single function ggc_mark_stringpool to mean two
completely different things.  I think this should be two different
functions.  ggc_mark_stringpool should remain as it is.  You should
introduce ggc_purge_stringpool to purge the stringpool.


Can you confirm that it is possible in principle to write gt_ggc_m_S
for the zone allocator?


This is OK with those changes.

Thanks.

Ian