[m-rev.] for review: Optimise hash table bucket lookup/set.
Zoltan Somogyi
zoltan.somogyi at runbox.com
Thu Dec 1 17:18:32 AEDT 2022
On Thu, 1 Dec 2022 16:29:31 +1100, Peter Wang <novalazy at gmail.com> wrote:
> On Thu, 01 Dec 2022 13:56:04 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> >
> >
> > On Thu, 1 Dec 2022 13:13:46 +1100, Peter Wang <novalazy at gmail.com> wrote:
> >
> > > hash_table is implemented on top of array, and version_hash_table is
> > > implemented on top of version_array. To look up an array element, it is
> > > necessary to pass a type_info representing the element type to the
> > > lookup predicate. But the type_info is never used, so constructing it
> > > dynamically is a waste of time.
> >
> > Are some lines missing from the front here? If not, please capitalize
> > the first word, and add a summary for git.
>
> The summary is in the Subject line.
Fine.
> > What was the time with the old code before the diff, at -O3? The
> > transformation you are doing by hand should have very similar,
> > if not identical, effect due to the unused arg elimination, if it
> > works with intermodule optimization (which I don't remember
> > whether it is supposed to or not).
>
> Oh, I thought the compiler couldn't eliminate the typeinfo arguments,
Actually, the original purpose of unused_args.m was specifically
the elimination of typeinfo arguments. Most user predicates
don't have any *visible* unused arguments, but the invisible
typeinfos have a much higher chance of being unused.
> I think we should add the pragma inlines to force version_array.lookup
> predicate to be opt-exported. It will help other users of version_array
> as well.
Agreed.
> The typecast change is ugly but is contained within a few lines,
> so it might be okay to keep it.
I think so too.
Zoltan.
More information about the reviews
mailing list