[v2,04/13,gdb/symtab] Factor out m_deferred_entries usage
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
Factor out usage of cooked_indexer::m_deferred_entries in new member
functions defer_entry, handle_deferred_entries and resolve_deferred_entry.
Tested on x86_64-linux.
---
gdb/dwarf2/read.c | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
Comments
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> Factor out usage of cooked_indexer::m_deferred_entries in new member
Tom> functions defer_entry, handle_deferred_entries and resolve_deferred_entry.
I don't mind this, but when reading through the whole series, it seems
like the code has to now iterate over a lot of the entries trying to fix
up the parentage later.
I wonder then about just sticking this info directly into the
cooked_index_entry object, and then doing fixups directly on these in
the shard.
That is, instead of keeping separate "deferred" entries, just making
ordinary entries. cooked_index_entry::parent_entry could be a union
holding either the parent (if known) or a CORE_ADDR; and then there
could be a new flag in cooked_index_flag_enum indicating which one is in
use.
Then I think parent_map::deferrred also would not be needed.
I'm still kind of mid-reading so my apologies if this doesn't really
make sense.
Tom
On 12/12/23 19:39, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> Factor out usage of cooked_indexer::m_deferred_entries in new member
> Tom> functions defer_entry, handle_deferred_entries and resolve_deferred_entry.
>
> I don't mind this, but when reading through the whole series, it seems
> like the code has to now iterate over a lot of the entries trying to fix
> up the parentage later.
>
> I wonder then about just sticking this info directly into the
> cooked_index_entry object, and then doing fixups directly on these in
> the shard.
>
> That is, instead of keeping separate "deferred" entries, just making
> ordinary entries. cooked_index_entry::parent_entry could be a union
> holding either the parent (if known) or a CORE_ADDR; and then there
> could be a new flag in cooked_index_flag_enum indicating which one is in
> use.
>
> Then I think parent_map::deferrred also would not be needed.
>
> I'm still kind of mid-reading so my apologies if this doesn't really
> make sense.
I've also considered something like this: rather than deferring creating
entries, creating them immediately with some marker that work is left to
be done. My initial though there was to use parent_map::deferred as
parent, and then keep lists of:
...
struct {
CORE_ADDR get_parent_from_here;
cooked_index_entry **patch_parent_here;
};
...
The union+flag approach would also work but doesn't offer a way to
iterate over them quickly, which might matter for large shards. Though
we could do a side-table approach I suppose.
We could choose to not care about such lists (in which case you'd need
the union+flag solution) and resolve these on demand, but then that'll
have to take care of cycle detection, so atm I'm not convinced that's a
good idea.
Thanks,
- Tom
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> I've also considered something like this: rather than deferring
Tom> creating entries, creating them immediately with some marker that work
Tom> is left to be done.
Also worth noting, in many cases no extra work will need to be done.
In these cases the parent entry for a DIE range can be recorded directly.
Tom> The union+flag approach would also work but doesn't offer a way to
Tom> iterate over them quickly, which might matter for large shards.
Tom> Though we could do a side-table approach I suppose.
Instead of an addrmap I'm proposing a sequence of vectors, and replacing
splay-tree search with a binary search. So the side thing is still
needed (it's these vectors), but the idea is that now the processing can
be parallelized; in fact stuck into the existing finalize stage.
Tom> We could choose to not care about such lists (in which case you'd need
Tom> the union+flag solution) and resolve these on demand, but then that'll
Tom> have to take care of cycle detection, so atm I'm not convinced that's
Tom> a good idea.
Doing it on demand means keeping the parentage information around
forever, or alternately re-reading abbrevs and then scanning DIEs at
lookup time. I don't think we should do either of these.
Tom
@@ -4641,6 +4641,32 @@ class cooked_indexer
we'll know the containing context of all the DIEs that we might
have scanned. This vector stores these deferred entries. */
std::vector<deferred_entry> m_deferred_entries;
+
+ /* Defer creating a cooked_index_entry corresponding to deferred_entry DE. */
+ void defer_entry (const deferred_entry &de)
+ {
+ m_deferred_entries.push_back (de);
+ }
+
+ /* Create a cooked_index_entry corresponding to deferred_entry DE with
+ parent PARENT_ENTRY. */
+ const cooked_index_entry *resolve_deferred_entry
+ (const deferred_entry &de, const cooked_index_entry *parent_entry)
+ {
+ return m_index_storage->add (de.die_offset, de.tag, de.flags, de.name,
+ parent_entry, m_per_cu);
+ }
+
+ /* Create cooked_index_entries for the deferred entries. */
+ void handle_deferred_entries ()
+ {
+ for (const auto &entry : m_deferred_entries)
+ {
+ const cooked_index_entry *parent_entry
+ = find_parent (entry.spec_offset);
+ resolve_deferred_entry (entry, parent_entry);
+ }
+ }
};
/* Subroutine of dwarf2_build_psymtabs_hard to simplify it.
@@ -16371,7 +16397,7 @@ cooked_indexer::index_dies (cutu_reader *reader,
if (name != nullptr)
{
if (defer != 0)
- m_deferred_entries.push_back ({
+ defer_entry ({
this_die, name, defer, abbrev->tag, flags
});
else
@@ -16476,12 +16502,7 @@ cooked_indexer::make_index (cutu_reader *reader)
return;
index_dies (reader, reader->info_ptr, nullptr, false);
- for (const auto &entry : m_deferred_entries)
- {
- const cooked_index_entry *parent = find_parent (entry.spec_offset);
- m_index_storage->add (entry.die_offset, entry.tag, entry.flags,
- entry.name, parent, m_per_cu);
- }
+ handle_deferred_entries ();
}
/* An implementation of quick_symbol_functions for the cooked DWARF