Hi Aaron,
On Sat, Oct 31, 2026 at 02:47:25AM +0100, Mark Wielaard wrote:
> > @@ -491,17 +523,41 @@ TYPE
> > name##_find
> > FIND(NAME) (NAME *htab, HASHTYPE hval)
> > {
> > - /* If we cannot get the resize_rwl lock someone is resizing
> > - the hash table, try to help out by moving table data. */
> > - while (pthread_rwlock_tryrdlock(&htab->resize_rwl) != 0)
> > - resize_worker(htab);
>
> OK.
>
> > + size_t idx = 0;
> >
> > - size_t idx;
>
> Why is it necessary to init to zero now?
>
> > + /* Snapshot of the current table. This pointer stays valid even if a
> > + new table is allocated during resize. */
> > + ENTRY(NAME) *table = __atomic_load_n(&htab->table, __ATOMIC_ACQUIRE);
>
> OK.
>
> > + ANNOTATE_HAPPENS_AFTER (&htab->table);
>
> Ignoring.
>
> > /* Make the hash data nonzero. */
> > hval = hval ?: 1;
> > - idx = lookup(htab, hval);
> >
> > + /* Make sure the current table isn't in the middle of a resize with
> > + uninitialized entries present. */
> > + if (atomic_load_explicit(&htab->resizing_state, memory_order_acquire)
> > + == NO_RESIZING)
> > + {
> > + ANNOTATE_HAPPENS_AFTER (&htab->resizing_state);
> > +
> > + size_t size = atomic_load_explicit(&table[0].hashval,
> > + memory_order_acquire);
> > +
> > + /* Lock-free check for whether the entry is present. */
> > + idx = lookup(table, size, hval);
> > +
> > + if (idx == 0)
> > + return NULL;
>
> I think this is wrong. It might not be in this table, but it might be
> in the new table. So this should fallthrough to the code below that
> does take that lock.
Sleeping on it I now think this is actually OK. An insert could race
our lookup. But that isn't really a problem. It could also not have
raced. If we need the key/value to be inserted we can try to insert it
ourselves (it will return -1 if it [now] is there already).
Falling through every time we don't find something will make us try to
take the lock, which kind of defeats the purpose of this patch.
> > + return (TYPE) atomic_load_explicit(&table[idx].val_ptr,
> > + memory_order_acquire);
> > + }
>
> OK.
>
> > + /* If we cannot get the resize_rwl lock someone is resizing
> > + the hash table, try to help out by moving table data. */
> > + while (pthread_rwlock_tryrdlock(&htab->resize_rwl) != 0)
> > + resize_worker(htab);
> > +
> > + idx = lookup(htab->table, htab->size, hval);
> > if (idx == 0)
> > {
> > pthread_rwlock_unlock(&htab->resize_rwl);
>
> OK. But see above, if not found in old table (and there is now a new
> table) we do need to recheck.
So we don't need to do a recheck. Just let the caller deal with it.
Cheers,
Mark
