[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 24 Jun 2015 09:29:25 +0300 (MSK)
From: Alexander Monakov <amonakov@...ras.ru>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/5] dynlink.c: use bloom filter in gnu hash
lookup
> > diff --git a/src/ldso/dynlink.c b/src/ldso/dynlink.c
> > index b77c6f6..fa91b39 100644
> > --- a/src/ldso/dynlink.c
> > +++ b/src/ldso/dynlink.c
> > @@ -54,6 +54,7 @@ struct dso {
> > Sym *syms;
> > uint32_t *hashtab;
> > uint32_t *ghashtab;
> > + uint32_t ghashmask;
> > int16_t *versym;
> > char *strings;
> > unsigned char *map;
> > @@ -200,6 +201,19 @@ static Sym *gnu_lookup(const char *s, uint32_t h1, struct dso *dso)
> > return 0;
> > }
> >
> > +static Sym *gnu_lookup_filtered(const char *s, uint32_t h1, struct dso *dso, uint32_t fofs, size_t fmask)
> > +{
> > + uint32_t *hashtab = dso->ghashtab;
> > + size_t *bloomwords = hashtab+4;
> > + size_t f = bloomwords[fofs & dso->ghashmask];
>
> Is this measurably faster than fofs & hashtab[2]-1 ?
It seems to give a small improvement for me, but with the other patches, too
small to measure reliably. As I'm using a big testcase, the impact on typical
libraries should be really tiny.
> If suspect not, in which case it seems desirable not to increase the
> size of struct dso. If it is worthwhile, at least don't sandwich a
> uint32_t between two pointers where it might incur another 32 bits of
> padding.
I wanted it to be nearby the hashtab pointer, which needs to be loaded anyway.
If ghashmask needs to move so that loading it will access a different cache
line, it's likely to diminish an already small improvement, at which point
it's easier to drop this patch :)
Would be nice if someone could test on a platform with slower caches.
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
