Re: [cxx-conversion] New Hash Table (issue6244048)

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, May 24, 2012 at 09:43:42AM -0700, Lawrence Crowl wrote:
> Add a type-safe hash table, typed_htab.  Uses of this table replace
> uses of libiberty's htab_t.  The benefits include less boiler-plate
> code, full type safety, and improved performance.

You haven't looked at the most important problem of that approach -
code bloat.  hashtab.o .text is currently ~ 4KB on x86_64, in your version
only very small part out of it is shared.  In a cc1plus binary I count
roughly 179 htab_create{,_ggc} calls, ignoring the first parameter (initial
size) that is roughly 139 unique hash/eq/del fn combinations, thus you'll
end up with over hundred copies of most of the hashtab code, in many of the
cases performance critical and thus its I-cache friendliness is quite
important.

> Static functions are also not acceptable as template arguments, so
> this patch externalizes the functions.  To avoid potential name
> conflicts, the function names have been prefixed.

Ugh.  I guess the C++ way around this would be to put the functions into
anonymous namespace.

> +  /* Return the current size of this hash table.  */
> +
> +  size_t size()
> +  {
> +    return htab->size;
> +  }

(and various other places) - formatting is wrong, missing space between (.

> +void
> +typed_htab <Element, Hash, Equal, Remove, Allocator>
> +::dispose ()

Do we want to format methods this way?

	Jakub