Re: [PATCH v2 05/13] [gdb/symtab] Resolve deferred entries, inter-shard case

[Tom Tromey <tom@tromey.com>]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +cooked_index_entry parent_map::deferred((sect_offset)0, (dwarf_tag)0,
Tom> +					(cooked_index_flag)0, nullptr,
Tom> +					nullptr, nullptr);

Several missing spaces.

However see the other note.  Maybe this isn't needed.

Tom>  cooked_index::cooked_index (vec_type &&vec)
Tom>    : m_vector (std::move (vec))
Tom>  {
Tom> +  handle_deferred_entries ();
Tom> +
Tom>    for (auto &idx : m_vector)
Tom>       idx->finalize ();

Here, parent resolution is single-threaded.  However, I think it can
probably be handled entirely in finalize -- and therefore distributed to
worker threads.  However, some changes are needed.

Tom> +const cooked_index_entry *
Tom> +cooked_index::find_parent_deferred_entry
Tom> +  (const cooked_index_shard::deferred_entry &entry) const
Tom> +{
Tom> +  const cooked_index_entry *parent_entry = nullptr;
Tom> +  for (auto &parent_map_shard : m_vector)
Tom> +    {
Tom> +      auto res = parent_map_shard->find_parent (entry.spec_offset);

The main issue with threading is that addrmaps are not thread-safe.
They are implemented as splay trees and these restructure themselves
during lookups.

However, I'm not sure we need splay trees.  Instead, it seems to me that
the relevant parts of the DIE tree can be handled with a sorted vector
that maps DIE ranges to entries.

Then, parent resolution can be done by binary search, and this can be
done by each shard in parallel -- and in particular, in do_finalize,
because that is already visiting every entry anyway.

Tom> +void
Tom> +cooked_index::handle_deferred_entries ()
Tom> +{
Tom> +  bool changed;
Tom> +  bool deferred;
Tom> +  do
Tom> +    {
Tom> +      deferred = false;
Tom> +      changed = false;
Tom> +      for (auto &shard : m_vector)
Tom> +	for (auto it = shard->m_deferred_entries->begin ();
Tom> +	     it != shard->m_deferred_entries->end (); )
Tom> +	  {
Tom> +	    const cooked_index_entry *parent_entry
Tom> +	      = find_parent_deferred_entry (*it);
Tom> +	    if (parent_entry == &parent_map::deferred)
Tom> +	      {
Tom> +		deferred = true;
Tom> +		it++;
Tom> +		continue;
Tom> +	      }
Tom> +	    shard->resolve_deferred_entry (*it, parent_entry);
Tom> +	    it = shard->m_deferred_entries->erase (it);
Tom> +	    changed = true;

I don't really understand this method.

What is the scenario leading to the need for 'deferred' and multiple
iterations?

Tom> +  for (auto &shard : m_vector)
Tom> +    {
Tom> +      shard->m_die_range_map.reset (nullptr);
Tom> +      shard->m_deferred_entries.reset (nullptr);
Tom> +    }
Tom> +}

In the background reading series, there's a new cooked_index_worker that
holds data that is needed before scanning is complete.  These things
could be put there instead.

Tom