On Mon, Aug 29, 2022 at 05:05:59PM +0200, Claudio Jeker wrote:
> The hash table for communities is not great. Instead of implementing
> dynamic hash resize use a RB tree. Also drop the hash calculation and
> just use memcmp() for now. My non scientific test seems to indicate that
> the overhead of SipHash is about the same as the memcmp().
Generally looks good. Two comments.
> +static inline int
> +communities_compare(struct rde_community *a, struct rde_community *b)
> {
> - uint32_t hs, i;
> + if (a->nentries != b->nentries)
> + return a->nentries - b->nentries;
I dislike comparison by subtraction, but I guess it's fine since nentries
should never be negative. Should this check and fatal?
> + if (a->flags != b->flags)
> + return a->flags - b->flags;
[...]
> struct rde_community *
> communities_link(struct rde_community *comm)
> {
> struct rde_community *n;
> - struct commhead *head;
>
> if ((n = malloc(sizeof(*n))) == NULL)
> fatal(__func__);
> -
> communities_copy(n, comm);
>
> - head = communities_hash(n);
> - LIST_INSERT_HEAD(head, n, entry);
> + RB_INSERT(comm_tree, &commtable, n);
I would prefer if this insertion was checked.
> n->refcnt = 1; /* initial reference by the cache */
>
> rdemem.comm_size += n->size;
> @@ -886,7 +817,7 @@ communities_unlink(struct rde_community
> if (comm->refcnt != 1)
> fatalx("%s: unlinking still referenced communities", __func__);
>
> - LIST_REMOVE(comm, entry);
> + RB_REMOVE(comm_tree, &commtable, comm);
>
> rdemem.comm_size -= comm->size;
> rdemem.comm_nmemb -= comm->nentries;
>
