gdb/dwarf: use gdb::unordered_set for cooked_index_storage::m_reader_hash
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
fail
|
Patch failed to apply
|
Commit Message
From: Simon Marchi <simon.marchi@polymtl.ca>
Replace an htab with gdb::unordered_set. I think we could also use the
dwarf2_per_cu pointer itself as the identity, basically have the
functional equivalent of:
gdb::unordered_map<dwarf2_per_cu *, cutu_reader_up>
But I kept the existing behavior of using dwarf2_per_cu::index as the
identity.
Change-Id: Ief3df9a71ac26ca7c07a7b79ca0c26c9d031c11d
---
gdb/dwarf2/cooked-index-storage.c | 56 ++++++++++++++++++-------------
gdb/dwarf2/cooked-index-storage.h | 26 +++++++++++---
2 files changed, 53 insertions(+), 29 deletions(-)
base-commit: 6eb9dab4c99725c1de4bccfeb99e766e7ee657a4
Comments
>>>>> "Simon" == simon marchi <simon.marchi@polymtl.ca> writes:
Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
Simon> Replace an htab with gdb::unordered_set. I think we could also use the
Simon> dwarf2_per_cu pointer itself as the identity, basically have the
Simon> functional equivalent of:
Simon> gdb::unordered_map<dwarf2_per_cu *, cutu_reader_up>
Simon> But I kept the existing behavior of using dwarf2_per_cu::index as the
Simon> identity.
Simon> + gdb::unordered_set<cutu_reader_up, cutu_reader_hash, cutu_reader_eq>
Simon> + m_reader_hash;
Thanks.
I'd written a version of this but yours is better, I used a map and just
duplicated the CU index number :)
Approved-By: Tom Tromey <tom@tromey.com>
Tom
On 3/18/25 3:00 PM, Tom Tromey wrote:
>>>>>> "Simon" == simon marchi <simon.marchi@polymtl.ca> writes:
>
> Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
> Simon> Replace an htab with gdb::unordered_set. I think we could also use the
> Simon> dwarf2_per_cu pointer itself as the identity, basically have the
> Simon> functional equivalent of:
>
> Simon> gdb::unordered_map<dwarf2_per_cu *, cutu_reader_up>
>
> Simon> But I kept the existing behavior of using dwarf2_per_cu::index as the
> Simon> identity.
>
> Simon> + gdb::unordered_set<cutu_reader_up, cutu_reader_hash, cutu_reader_eq>
> Simon> + m_reader_hash;
>
> Thanks.
>
> I'd written a version of this but yours is better, I used a map and just
> duplicated the CU index number :)
That's what I would have naturally done, but in a previous series you
had a comment complaining about that :). Granted, it was for something
that needed to be more space-efficient than this, either abbrevs or
dies, I don't recall.
> Approved-By: Tom Tromey <tom@tromey.com>
Thanks, pushed.
Simon
>> I'd written a version of this but yours is better, I used a map and just
>> duplicated the CU index number :)
Simon> That's what I would have naturally done, but in a previous series you
Simon> had a comment complaining about that :). Granted, it was for something
Simon> that needed to be more space-efficient than this, either abbrevs or
Simon> dies, I don't recall.
Yeah, I think set with a "projection" should be used if the size is
likely to matter; but otherwise map can be used.
We could make the set case simpler when it's just using an accessible
field by writing a couple of templates that take a pointer-to-member, if
we really wanted.
Tom
@@ -22,11 +22,7 @@
/* See cooked-index-storage.h. */
cooked_index_storage::cooked_index_storage ()
- : m_reader_hash (htab_create_alloc (10, hash_cutu_reader,
- eq_cutu_reader,
- htab_delete_entry<cutu_reader>,
- xcalloc, xfree)),
- m_shard (new cooked_index_shard)
+ : m_shard (new cooked_index_shard)
{
}
@@ -35,9 +31,8 @@ cooked_index_storage::cooked_index_storage ()
cutu_reader *
cooked_index_storage::get_reader (dwarf2_per_cu *per_cu)
{
- int index = per_cu->index;
- return (cutu_reader *) htab_find_with_hash (m_reader_hash.get (),
- &index, index);
+ auto it = m_reader_hash.find (*per_cu);
+ return it != m_reader_hash.end () ? it->get () : nullptr;
}
/* See cooked-index-storage.h. */
@@ -47,30 +42,43 @@ cooked_index_storage::preserve (cutu_reader_up reader)
{
m_abbrev_table_cache.add (reader->release_abbrev_table ());
- int index = reader->cu ()->per_cu->index;
- void **slot = htab_find_slot_with_hash (m_reader_hash.get (), &index,
- index, INSERT);
- gdb_assert (*slot == nullptr);
- cutu_reader *result = reader.get ();
- *slot = reader.release ();
- return result;
+ auto [it, inserted] = m_reader_hash.insert (std::move (reader));
+ gdb_assert (inserted);
+
+ return it->get();
+}
+
+/* See cooked-index-storage.h. */
+
+std::uint64_t
+cooked_index_storage::cutu_reader_hash::operator()
+ (const cutu_reader_up &reader) const noexcept
+{
+ return (*this) (*reader->cu ()->per_cu);
+}
+
+/* See cooked-index-storage.h. */
+
+std::uint64_t
+cooked_index_storage::cutu_reader_hash::operator() (const dwarf2_per_cu &per_cu)
+ const noexcept
+{
+ return per_cu.index;
}
/* See cooked-index-storage.h. */
-hashval_t
-cooked_index_storage::hash_cutu_reader (const void *a)
+bool
+cooked_index_storage::cutu_reader_eq::operator() (const cutu_reader_up &a,
+ const cutu_reader_up &b) const noexcept
{
- const cutu_reader *reader = (const cutu_reader *) a;
- return reader->cu ()->per_cu->index;
+ return (*this) (*a->cu ()->per_cu, b);
}
/* See cooked-index-storage.h. */
-int
-cooked_index_storage::eq_cutu_reader (const void *a, const void *b)
+bool cooked_index_storage::cutu_reader_eq::operator()
+ (const dwarf2_per_cu &per_cu, const cutu_reader_up &reader) const noexcept
{
- const cutu_reader *ra = (const cutu_reader *) a;
- const int *rb = (const int *) b;
- return ra->cu ()->per_cu->index == *rb;
+ return per_cu.index == reader->cu ()->per_cu->index;
}
@@ -90,18 +90,34 @@ class cooked_index_storage
}
private:
+ /* The abbrev table cache used by this indexer. */
+ abbrev_table_cache m_abbrev_table_cache;
/* Hash function for a cutu_reader. */
- static hashval_t hash_cutu_reader (const void *a);
+ struct cutu_reader_hash
+ {
+ using is_transparent = void;
+
+ std::uint64_t operator() (const cutu_reader_up &reader) const noexcept;
+ std::uint64_t operator() (const dwarf2_per_cu &per_cu) const noexcept;
+ };
/* Equality function for cutu_reader. */
- static int eq_cutu_reader (const void *a, const void *b);
+ struct cutu_reader_eq
+ {
+ using is_transparent = void;
- /* The abbrev table cache used by this indexer. */
- abbrev_table_cache m_abbrev_table_cache;
+ bool operator() (const cutu_reader_up &a,
+ const cutu_reader_up &b) const noexcept;
+
+ bool operator() (const dwarf2_per_cu &per_cu,
+ const cutu_reader_up &reader) const noexcept;
+ };
/* A hash table of cutu_reader objects. */
- htab_up m_reader_hash;
+ gdb::unordered_set<cutu_reader_up, cutu_reader_hash, cutu_reader_eq>
+ m_reader_hash;
+
/* The index shard that is being constructed. */
cooked_index_shard_up m_shard;