[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