Re: [PATCH 4/7] Class-fy partial_die_info

[Pedro Alves <palves@redhat.com>]

On 01/29/2018 01:15 AM, Simon Marchi wrote:

> From what I understand, the only reason to have that private constructor is
> to construct a temporary partial_die_info object used to search in the htab,
> is that right?  If so, converting that htab_t to an std::unordered_map would
> remove the need for all this, since you don't need to construct an object
> to search it.  See the diff below that applies on top of this patch.
> 
> It's not thoroughly tested and I am not sure of the validity of the
> per_cu->cu->partial_dies.empty () call in find_partial_die, but I think it
> should work.  Plus, it adds some type-safety, which I am a big fan of.
> 
> But otherwise, the patch is fine with me.

Careful here.  This could do with some benchmarking.  The DWARF reading code
is performance (both timing and memory) sensitive.  This is trading an open
addressing hash table (htab_t), for a node-based closed addressing hash table.
The keys/elements in the map are small so I'd expect this to make
a difference.  Also, this is trading a in-principle cache-friendly
obstack allocation scheme for the standard new allocator.

Thanks,
Pedro Alves