[FFmpeg-devel] [PATCH v2 1/2] lavu/hashtable: create generic robin hood hash table
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Wed Feb 7 12:18:22 EET 2024
Connor Worley:
> Signed-off-by: Connor Worley <connorbworley at gmail.com>
> ---
> libavutil/Makefile | 2 +
> libavutil/hashtable.c | 183 ++++++++++++++++++++++++++++++++++++
> libavutil/hashtable.h | 47 +++++++++
> libavutil/tests/hashtable.c | 108 +++++++++++++++++++++
> 4 files changed, 340 insertions(+)
> create mode 100644 libavutil/hashtable.c
> create mode 100644 libavutil/hashtable.h
> create mode 100644 libavutil/tests/hashtable.c
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index e7709b97d0..be75d464fc 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -138,6 +138,7 @@ OBJS = adler32.o \
> fixed_dsp.o \
> frame.o \
> hash.o \
> + hashtable.o \
> hdr_dynamic_metadata.o \
> hdr_dynamic_vivid_metadata.o \
> hmac.o \
> @@ -251,6 +252,7 @@ TESTPROGS = adler32 \
> file \
> fifo \
> hash \
> + hashtable \
> hmac \
> hwdevice \
> integer \
> diff --git a/libavutil/hashtable.c b/libavutil/hashtable.c
> new file mode 100644
> index 0000000000..6fa2af8af3
> --- /dev/null
> +++ b/libavutil/hashtable.c
> @@ -0,0 +1,183 @@
> +/*
> + * Generic hashtable
> + * Copyright (C) 2024 Connor Worley <connorbworley at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include "crc.h"
> +#include "error.h"
> +#include "mem.h"
> +#include "hashtable.h"
> +
> +struct AVHashtableContext {
> + size_t key_size;
> + size_t val_size;
> + size_t entry_size;
> + size_t max_entries;
> + size_t utilization;
> + const AVCRC *crc;
> + uint8_t *table;
> + uint8_t *swapbuf;
> +};
> +
> +#define ENTRY_PSL(entry) (entry)
> +#define ENTRY_OCC(entry) (ENTRY_PSL(entry) + sizeof(size_t))
> +#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 4)
> +#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + FFALIGN(ctx->key_size, 4))
> +
It seems you misunderstood what I said about alignment in my last
review. You should align entry_size and not align the size of key and
val fields. The way you are doing it now does not prevent unaligned
accesses: Imagine the common case of size_t having an alignment
requirement of 8 and key_size and val_size being four. Then entry_size
is not a multiple of 8 and therefore of two consecutive entries, only
one can be properly aligned (e.g. the second entry
ctx->table+ctx->entry_size is misaligned).
(For the same reason, the alignment of a struct is a multiple of the
alignment of every member of said struct.)
The way you are doing it also adds FFALIGN to every access to val.
> +#define KEYS_EQUAL(k1, k2) !memcmp(k1, k2, ctx->key_size)
> +
> +int av_hashtable_alloc(struct AVHashtableContext** ctx, size_t key_size, size_t val_size, size_t max_entries)
> +{
> + struct AVHashtableContext *res = av_malloc(sizeof(struct AVHashtableContext));
> + if (!res)
> + return AVERROR(ENOMEM);
> + res->key_size = key_size;
> + res->val_size = val_size;
You mostly only need key_size and key_size+val_size; val_size is not
used that often, so if you only store two sizes, it should not be stored.
> + res->entry_size = sizeof(size_t) + 4 + FFALIGN(key_size, 4) + FFALIGN(val_size, 4);
> + res->max_entries = max_entries;
> + res->utilization = 0;
> + res->crc = av_crc_get_table(AV_CRC_32_IEEE);
> + if (!res->crc) {
> + av_hashtable_freep(&res);
> + return AVERROR_BUG;
> + }
> + res->table = av_calloc(res->max_entries, res->entry_size);
> + if (!res->table) {
> + av_hashtable_freep(&res);
> + return AVERROR(ENOMEM);
> + }
> + res->swapbuf = av_calloc(2, FFALIGN(key_size, 4) + FFALIGN(val_size, 4));
> + if (!res->swapbuf) {
> + av_hashtable_freep(&res);
> + return AVERROR(ENOMEM);
> + }
> + *ctx = res;
> + return 0;
> +}
> +
> +static size_t hash_key(const struct AVHashtableContext* ctx, const void* key)
> +{
> + return av_crc(ctx->crc, 0, key, ctx->key_size) % ctx->max_entries;
> +}
> +
> +int av_hashtable_get(const struct AVHashtableContext* ctx, const void* key, void* val)
> +{
> + size_t hash = hash_key(ctx, key);
> +
> + if (!ctx->utilization)
> + return 0;
> +
> + for (size_t psl = 0; psl < ctx->max_entries; psl++) {
> + size_t wrapped_index = (hash + psl) % ctx->max_entries;
> + uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
> + if (!*ENTRY_OCC(entry) || *(size_t*)ENTRY_PSL(entry) < psl)
> + return 0;
> + if (KEYS_EQUAL(ENTRY_KEY(entry), key)) {
> + memcpy(val, ENTRY_VAL(entry), ctx->val_size);
> + return 1;
> + }
> + }
> + return 0;
> +}
> +
> +int av_hashtable_set(struct AVHashtableContext* ctx, const void* key, const void* val)
> +{
> + int swapping = 0;
> + size_t psl = 0;
> + size_t hash = hash_key(ctx, key);
> + uint8_t *set = ctx->swapbuf;
> + uint8_t *tmp = ctx->swapbuf + FFALIGN(ctx->key_size, 4) + FFALIGN(ctx->val_size, 4);
> +
> + memcpy(set, key, ctx->key_size);
> + memcpy(set + FFALIGN(ctx->key_size, 4), val, ctx->val_size);
> +
> + for (size_t i = 0; i < ctx->max_entries; i++) {
> + size_t wrapped_index = (hash + i) % ctx->max_entries;
> + uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
> + if (!*ENTRY_OCC(entry) || (!swapping && KEYS_EQUAL(ENTRY_KEY(entry), set))) {
> + if (!*ENTRY_OCC(entry))
> + ctx->utilization++;
> + *(size_t*)ENTRY_PSL(entry) = psl;
> + *ENTRY_OCC(entry) = 1;
> + memcpy(ENTRY_KEY(entry), set, FFALIGN(ctx->key_size, 4) + ctx->val_size);
> + return 1;
> + }
> + if (*(size_t*)ENTRY_PSL(entry) < psl) {
> + if (ctx->utilization == ctx->max_entries)
> + return 0;
> + swapping = 1;
> + // set needs to swap with entry
> + memcpy(tmp, ENTRY_KEY(entry), FFALIGN(ctx->key_size, 4) + ctx->val_size);
> + memcpy(ENTRY_KEY(entry), set, FFALIGN(ctx->key_size, 4) + ctx->val_size);
> + FFSWAP(uint8_t*, set, tmp);
> + FFSWAP(size_t, psl, *(size_t*)ENTRY_PSL(entry));
> + }
> + psl++;
> + }
> + return 0;
> +}
> +
> +int av_hashtable_delete(struct AVHashtableContext* ctx, const void* key)
> +{
> + uint8_t *next_entry;
> + size_t hash = hash_key(ctx, key);
> +
> + if (!ctx->utilization)
> + return 0;
> +
> + for (size_t psl = 0; psl < ctx->max_entries; psl++) {
> + size_t wrapped_index = (hash + psl) % ctx->max_entries;
> + uint8_t *entry = ctx->table + wrapped_index * ctx->entry_size;
> + if (!*ENTRY_OCC(entry) || *(size_t*)ENTRY_PSL(entry) < psl)
> + return 0;
> + if (KEYS_EQUAL(ENTRY_KEY(entry), key)) {
> + *ENTRY_OCC(entry) = 0;
> + for (psl++; psl < ctx->max_entries; psl++) {
> + wrapped_index = (hash + psl) % ctx->max_entries;
> + next_entry = ctx->table + wrapped_index * ctx->entry_size;
> + if (!*ENTRY_OCC(next_entry) || !*(size_t*)ENTRY_PSL(next_entry)) {
> + ctx->utilization--;
> + return 1;
> + }
> + memcpy(ENTRY_OCC(entry), ENTRY_OCC(next_entry), ctx->entry_size);
> + *(size_t*)ENTRY_PSL(entry)--;
> + *ENTRY_OCC(next_entry) = 0;
> + entry = next_entry;
> + }
> + }
> + };
This ';' is a null statement.
> + return 0;
> +}
> +
> +void av_hashtable_clear(struct AVHashtableContext* ctx)
> +{
> + memset(ctx->table, 0, ctx->entry_size * ctx->max_entries);
> +}
> +
> +void av_hashtable_freep(struct AVHashtableContext** ctx)
> +{
> + if (*ctx) {
> + av_freep(&(*ctx)->table);
> + av_freep(&(*ctx)->swapbuf);
> + }
> + av_freep(ctx);
> +}
> diff --git a/libavutil/hashtable.h b/libavutil/hashtable.h
> new file mode 100644
> index 0000000000..90c6cad68a
> --- /dev/null
> +++ b/libavutil/hashtable.h
> @@ -0,0 +1,47 @@
> +/*
> + * Generic hashtable
> + * Copyright (C) 2024 Connor Worley <connorbworley at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVUTIL_HASHTABLE_H
> +#define AVUTIL_HASHTABLE_H
> +
> +#include <stddef.h>
> +
> +/* Implements a hash table using Robin Hood open addressing.
> + * See: https://cs.uwaterloo.ca/research/tr/1986/CS-86-14.pdf
> + *
> + * Entry slots are layed out in memory as follows:
> + * - [1 byte] occupancy flag that indicates whether a slot if occupied.
> + * - [key_size bytes] entry key.
> + * - [val_size bytes] entry value.
> + * - [sizeof(size_t) bytes] PSL (probe sequence length), used by the Robin Hood
> + * algorithm.
This is no longer true and it definitely does not belong in a header.
> + */
> +
> +struct AVHashtableContext;
> +
> +int av_hashtable_alloc(struct AVHashtableContext** ctx, size_t key_size, size_t val_size, size_t max_entries);
> +int av_hashtable_get(const struct AVHashtableContext* ctx, const void* key, void* val);
> +int av_hashtable_set(struct AVHashtableContext* ctx, const void* key, const void* val);
> +int av_hashtable_delete(struct AVHashtableContext* ctx, const void* key);
> +void av_hashtable_clear(struct AVHashtableContext* ctx);
> +void av_hashtable_freep(struct AVHashtableContext** ctx);
> +
> +#endif
> diff --git a/libavutil/tests/hashtable.c b/libavutil/tests/hashtable.c
> new file mode 100644
> index 0000000000..0b38b34c5c
> --- /dev/null
> +++ b/libavutil/tests/hashtable.c
> @@ -0,0 +1,108 @@
> +/*
> + * Generic hashtable tests
> + * Copyright (C) 2024 Connor Worley <connorbworley at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/hashtable.h"
> +
> +int main(void)
> +{
> + struct AVHashtableContext *ctx;
> + uint8_t k;
> + uint64_t v;
> +
> + // impossibly large allocation should fail gracefully
Actually, av_hashtable_alloc is missing overflow checks.
> + av_assert0(av_hashtable_alloc(&ctx, -1, -1, -1) < 0);
> +
> + // hashtable can store up to 3 uint8_t->uint64_t entries
> + av_assert0(!av_hashtable_alloc(&ctx, sizeof(k), sizeof(v), 3));
> +
> + // unsuccessful deletes return 0
> + k = 1;
> + av_assert0(!av_hashtable_delete(ctx, &k));
> +
> + // unsuccessful gets return 0
> + k = 1;
> + av_assert0(!av_hashtable_get(ctx, &k, &v));
> +
> + // successful sets returns 1
> + k = 1;
> + v = 1;
> + av_assert0(av_hashtable_set(ctx, &k, &v));
> +
> + // get should now contain 1
> + k = 1;
> + v = 0;
> + av_assert0(av_hashtable_get(ctx, &k, &v));
> + av_assert0(v == 1);
> +
> + // updating sets should return 1
> + k = 1;
> + v = 2;
> + av_assert0(av_hashtable_set(ctx, &k, &v));
> +
> + // get should now contain 2
> + k = 1;
> + v = 0;
> + av_assert0(av_hashtable_get(ctx, &k, &v));
> + av_assert0(v == 2);
> +
> + // fill the table
> + k = 2;
> + v = 2;
> + av_assert0(av_hashtable_set(ctx, &k, &v));
> + k = 3;
> + v = 3;
> + av_assert0(av_hashtable_set(ctx, &k, &v));
> +
> + // inserting sets on a full table should return 0
> + k = 4;
> + v = 4;
> + av_assert0(!av_hashtable_set(ctx, &k, &v));
> +
> + // updating sets on a full table should return 1
> + k = 1;
> + v = 4;
> + av_assert0(av_hashtable_set(ctx, &k, &v));
> + v = 0;
> + av_assert0(av_hashtable_get(ctx, &k, &v));
> + av_assert0(v == 4);
> +
> + // successful deletes should return 1
> + k = 1;
> + av_assert0(av_hashtable_delete(ctx, &k));
> +
> + // get should now return 0
> + av_assert0(!av_hashtable_get(ctx, &k, &v));
> +
> + // sanity check remaining keys
> + k = 2;
> + v = 0;
> + av_assert0(av_hashtable_get(ctx, &k, &v));
> + av_assert0(v == 2);
> + k = 3;
> + v = 0;
> + av_assert0(av_hashtable_get(ctx, &k, &v));
> + av_assert0(v == 3);
> +
> + av_hashtable_freep(&ctx);
> +
> + return 0;
> +}
More information about the ffmpeg-devel
mailing list