dwarf-reader: Use high bits of Bloom filter words.
Commit Message
Most of the bit values used for GNU hash ELF section Bloom filtering
were being ignored due to integer narrowing, reducing missing symbol
filtering efficiency considerably.
This patch fixes this.
* src/abg-dwarf-reader.cc (lookup_symbol_from_gnu_hash_tab):
Don't narrow calculated Bloom word to 8 bits before using it
to mask the fetched Bloom word.
---
src/abg-dwarf-reader.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
Hi!
On Wed, Mar 18, 2020 at 10:20:55AM +0000, Android Kernel Team wrote:
>Most of the bit values used for GNU hash ELF section Bloom filtering
>were being ignored due to integer narrowing, reducing missing symbol
>filtering efficiency considerably.
Nice! Maybe add the analysis result from the other mail thread to point
out the improvement made here (and the justification for this change).
>
>This patch fixes this.
>
> * src/abg-dwarf-reader.cc (lookup_symbol_from_gnu_hash_tab):
> Don't narrow calculated Bloom word to 8 bits before using it
> to mask the fetched Bloom word.
Please sign off your work.
Reviewed-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
>---
> src/abg-dwarf-reader.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>index 3454fcf5..5556bde5 100644
>--- a/src/abg-dwarf-reader.cc
>+++ b/src/abg-dwarf-reader.cc
>@@ -2025,7 +2025,7 @@ lookup_symbol_from_gnu_hash_tab(const environment* env,
> // filter, in bits.
> int c = get_elf_class_size_in_bytes(elf_handle) * 8;
> int n = (h1 / c) % ht.bf_nwords;
>- unsigned char bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
>+ GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
>
> // Test if the symbol is *NOT* present in this ELF file.
> if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask)
>--
>2.25.1.481.gfbce0eb801-goog
>
>
Will do.
I'll also change the commit name s/high/all/.
Giuliano.
On Wed, 18 Mar 2020 at 11:37, Matthias Maennich <maennich@google.com> wrote:
>
> Hi!
>
> On Wed, Mar 18, 2020 at 10:20:55AM +0000, Android Kernel Team wrote:
> >Most of the bit values used for GNU hash ELF section Bloom filtering
> >were being ignored due to integer narrowing, reducing missing symbol
> >filtering efficiency considerably.
>
> Nice! Maybe add the analysis result from the other mail thread to point
> out the improvement made here (and the justification for this change).
>
> >
> >This patch fixes this.
> >
> > * src/abg-dwarf-reader.cc (lookup_symbol_from_gnu_hash_tab):
> > Don't narrow calculated Bloom word to 8 bits before using it
> > to mask the fetched Bloom word.
> Please sign off your work.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >---
> > src/abg-dwarf-reader.cc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
> >index 3454fcf5..5556bde5 100644
> >--- a/src/abg-dwarf-reader.cc
> >+++ b/src/abg-dwarf-reader.cc
> >@@ -2025,7 +2025,7 @@ lookup_symbol_from_gnu_hash_tab(const environment* env,
> > // filter, in bits.
> > int c = get_elf_class_size_in_bytes(elf_handle) * 8;
> > int n = (h1 / c) % ht.bf_nwords;
> >- unsigned char bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
> >+ GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
> >
> > // Test if the symbol is *NOT* present in this ELF file.
> > if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask)
> >--
> >2.25.1.481.gfbce0eb801-goog
> >
> >
@@ -2025,7 +2025,7 @@ lookup_symbol_from_gnu_hash_tab(const environment* env,
// filter, in bits.
int c = get_elf_class_size_in_bytes(elf_handle) * 8;
int n = (h1 / c) % ht.bf_nwords;
- unsigned char bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
+ GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
// Test if the symbol is *NOT* present in this ELF file.
if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask)