From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 0279C47FB2 for ; Mon, 5 Feb 2024 11:58:26 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 713C168D16E; Mon, 5 Feb 2024 13:58:25 +0200 (EET) Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05olkn2097.outbound.protection.outlook.com [40.92.89.97]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6C1DC68D13C for ; Mon, 5 Feb 2024 13:58:18 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HHEVT9WLz1RCIZMFDsJmRipNw44lPXRuaXBxzhIORjCOeSpvuZ5Zeil8diyWcYxckv9pzQIAZ1WhDSDm+CW5zOSIiBsKIT99B8sPf2QsWuxs0OVX7Eojnxfw11+LZm7SuslxxsPdDDMN9giLqS+8QSTIiiYqE6KsPb8L0y5ratCBNW7BUhSrieUWyEz55i5ZKewY1swTXK1qUoAvUBDcofdkPuCPtO9RMWtfx0ZtyQQQH/ZzVdoTKOST+D779LsDAa6XilEEPNF7+7sTOqCZ7uiZGKHgDZQCGVotWrVHu1h6ZyffP2V+O5rM1nJMKz1s2QgD17c8VDHa96uPBY0/iw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=+ppbddDEDvxYCU443qIWDCYyd3VgVariYzfao2q6je4=; b=ZSReeDw79/KyNwwuNjHW3an3/fBzvnH9kUP33ZKthojdzapLuEA0VYqN5bVM6nJXQNn7aEIr1LFhS1rJiuPof1NbqPyeK5LuPMCsQ+PbPlX2/QPsJn69wec1EVJWcDIXxH4sXDTey7h+XiAzmQ4qWSMDNxAMAOh67WKhNcGP7EoOgsYa5luSkjauJYm3K6jIVujMOV4zl+Z9m+PLpek48xg4QY6BBHRRcgww8YFY4m3z2iIV3FBZrM3lV/48aXq368tSqiChyI59hTmz8NpHvqlmTjMxF689dqNmOK6OBD3MGhyZX3xqa4SySANRXuONRXPx2aDxsnETzB4azFywGQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+ppbddDEDvxYCU443qIWDCYyd3VgVariYzfao2q6je4=; b=H3cbp9NiIrb5n7cGZmwC/9/lE1FO9WfnL94LBXx/CtsYyoHLuseRnF60wjAeULjOBjA5hone42sXYsFl+ZY0BDmmev5owjyke69CJIT2KA0LpApHP5S4CCYdRgTa5+jvWLcdl2QkhT+CZiP0iJi4CKwHWMiiATSGGBe9+3Akv++dD7whCCNDHHLL4++EMYGuzyO3CMLRAF8SUZrxf7oRvdF+5yw402ulBEE85GNG6JB23J6n695TIsaoF7v0oV4uYmPeSY2S2pTeU9mmKWSJh9t5GwA3f6vKuZOFpDU6aDAkBaitb5RNiKGMj6JCOwq2qNnTDxZaiY1YVwAgxk+RDw== Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) by PR3P250MB0385.EURP250.PROD.OUTLOOK.COM (2603:10a6:102:17c::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7249.34; Mon, 5 Feb 2024 11:58:14 +0000 Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::65aa:deb0:a18e:d48d]) by AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::65aa:deb0:a18e:d48d%5]) with mapi id 15.20.7249.032; Mon, 5 Feb 2024 11:58:14 +0000 Message-ID: Date: Mon, 5 Feb 2024 13:00:07 +0100 User-Agent: Mozilla Thunderbird Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20240205052724.26862-1-connorbworley@gmail.com> From: Andreas Rheinhardt In-Reply-To: <20240205052724.26862-1-connorbworley@gmail.com> X-TMN: [J13bUr8/wqk1O/lAWTG3jP8yn28QUmP9IGYwbuBNkJk=] X-ClientProxiedBy: ZR2P278CA0009.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:50::18) To AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) X-Microsoft-Original-Message-ID: <27999d67-b27b-4878-bac0-6399c15d3455@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P250MB0744:EE_|PR3P250MB0385:EE_ X-MS-Office365-Filtering-Correlation-Id: 6ee9d650-a56b-44d2-a0e6-08dc2641bbc5 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: S8SMsM5Za2+OvQFkosG5t/lKTgfbiRRYdF9i0dk/YYyhVW09ISbC7+1cZuLhaOZdwj0xuiIno+YTndL6f2q9gW1x0ZyYS7vqSHa5f78O0170Ys2Xq0qhb+qwz97WQMv7V6r8lSyF3OC+VyfDkMLNkAEUErAatmPFlMyT9mf/6+4T9dNYsnsEa5/YQeDXMdHrud6G8Xa6dpsYrY3qahFdsfbC9NwudI+8JWwWqrR1bGClPeHfRJLGry8Nh5c1FqedGNEqpih6CsF2GeOrzUd5QZE3x2SbXF7tVi2lGi3nhML07OLsIi1jXHsmpFii9+F96APxg4IAmE8bEymjTL71I77nqridxDDpXHeyX5LXwf73KQR367I5O52ggKSKZcPvV4I5er9pDGXUmWSFCaIa0DJBnS9/hNLe0vuxF+Unyl0AMjWoqDpoYAad8WAV7Ks+gHwbCNmeYpvr9qTsivKIHGfPp0N50IOwng0fJUTpPf8BQECTTvt8gGqznc2gHejWUhvQzlfq966pX7BL8sbPM6olXdKuWbc5yzqkf+b9r3A= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Q2NSRVJmZGxRQW5xeTZ2ZHovY05QZ05IeC9BWWR2YjZVRDM0d1l1S3FDelI4?= =?utf-8?B?L3l6TGc4MEhmbW0rV0VhcG56eE9hc1JvdlJYSEhMT0NBQ3NUL0VRL0puajFy?= =?utf-8?B?ZllDQndJYVVvZUNYd1FGZmg4TUx1RDZZZVdqWlY3Ry9yTHdWNlhNTXMxZHhh?= =?utf-8?B?M3o3clBZNkk4Tm1wZms4eWtrY3JSaUdBdUhrNzFXMjBzZ2NkVzBuMzdUckN3?= =?utf-8?B?OHRvQXFmZE92ckVkWXMrbWdwODRsaTVhcytXUGxyV0V1dFVCeHQzZ0xsRyt3?= =?utf-8?B?VVV4OENzMDlwdHQraGFWdnJaM3gzOUlEOEZKcFN5bXJicVRZMkFLVG9jbEk5?= =?utf-8?B?M2VsZFNOTitBRzNXNU95YzdBcnQ3MnI0eGNiMTVCTjVmTzFTV1MxZzV5WFpL?= =?utf-8?B?akxpWDlyaHpxamZlQXRDdVZDdks2VXdxbGVLRU1tNlNvSUxlSkQ1RDZCYUR5?= =?utf-8?B?dTZCL2padTEyY3hTWnJNQVd1cXhHSEF2b1o0NXdDUzNCYVVlM2NscnBoQ2ZC?= =?utf-8?B?TVRpTDBGR2FndWs4d0dVTE4wekhLQW1uNW51R1cyeVY1N3NtZjhvclpOa01G?= =?utf-8?B?b3B2dld1TWFlOGRmcDArNGl4SDlyU0xqTDkzdE81UjZldlZHeno4Y0lsZGxj?= =?utf-8?B?dU5la3ZObGcrdW1kSWRSMWVsclpqeGVwOTFPSGZoeGZkTXlwWlZFeGMzb2Y1?= =?utf-8?B?bXd5blJ2dE9CR3pKOXVycmFrd3l3VVhlM3JLNk5oeVoyMTArV29IejFFSjdx?= =?utf-8?B?TTNJTFBtb1NFM3QxYk1jeXltRWtFWTdRbzhFSTZYSzJ1SzBYakluNE51SHJF?= =?utf-8?B?ekFWWXVyRFJ2RUVTcmQxU3hWSHVEQXR5VkVNTlhrbUN5VThIcmVpYksycElm?= =?utf-8?B?ZDF4ejdodEpvdjVITmlJQ1MvT0ROc0NkNW5IYWQ3RFdtamxXbTJIRkUyQUlQ?= =?utf-8?B?dXNjSHgwZXp1SGhJTVcvM2N4Y0d1WStuRHlFN3ZLenA0eC9iZlZYUlpKanh5?= =?utf-8?B?VGYvdEtJeUVZTUtyeFZqNEllby9kS1d4eVI1QzhkdFR5dS9BRS9tQlAyUjJ3?= =?utf-8?B?NUVHbndHT0R2VFl6clNwbERFY0ttVVJjZ3ZGK01VL3o3aGtXY1VnaElYWHlC?= =?utf-8?B?NFppenhwSmxxMXFOQStUYlRKd0tMZTlZNEpqRWlmOFdhSTNRNlNzUlZLbytK?= =?utf-8?B?ZFdocnR4dERaTnlaYzJINGZoMzdJTGpmUEVJaHhaYUhSRDNWK0l2cGpuTkp1?= =?utf-8?B?T0NYS3hjWkoyN251ODltN04zSm0yNWRjeGpXUEpiaFhEOFZ6NFh0S3BaSkdQ?= =?utf-8?B?ZVB1dmtIbzFQTCsvSVZUWmFrUm9vMUVjMEd5enBFaU9wM2tWNnNlb2VHU1do?= =?utf-8?B?WkNzTktrQUpXRFpmeVgwNXFybHFiTFozc0dkeEpJWG1SRFVsdlVPUDFkcmpN?= =?utf-8?B?ZlV6Y1kxUzFqd0ZhM2JoOGtiYTNDc0l0SElmRjBPZ3FpckVDY09WSExnc1FD?= =?utf-8?B?Y1hEZkNReFRHTVJEUU95NlgyOER3Mis2d3h3MHdmN1lIbjdwbUp2WEtXV0Jp?= =?utf-8?B?VThUYkU4OTJEdHVIRVl0YWpOTWhxZkVnYlQ0VlJnK3IwalExanRUbS9LYytH?= =?utf-8?B?OWFGY0I3enl2eUhsM0FOdjhQVWhBQXRGaUVHajB2YUhCa0UrOXNzTWdhUGNi?= =?utf-8?Q?i2+LkTUjZLMLNjeAZE+z?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6ee9d650-a56b-44d2-a0e6-08dc2641bbc5 X-MS-Exchange-CrossTenant-AuthSource: AS8P250MB0744.EURP250.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Feb 2024 11:58:14.7091 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: PR3P250MB0385 Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavu/hashtable: create generic robin hood hash table X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Connor Worley: > Signed-off-by: Connor Worley > --- > libavutil/Makefile | 2 + > libavutil/hashtable.c | 172 ++++++++++++++++++++++++++++++++++++ > libavutil/hashtable.h | 62 +++++++++++++ > libavutil/tests/hashtable.c | 104 ++++++++++++++++++++++ > 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..fb0f0aae99 > --- /dev/null > +++ b/libavutil/hashtable.c > @@ -0,0 +1,172 @@ > +/* > + * Generic hashtable > + * Copyright (C) 2024 Connor Worley > + * > + * 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 > + > +#include "error.h" > +#include "mem.h" > +#include "hashtable.h" > +#include "string.h" We don't have a string.h header; the string.h from the C stdlib should of course be included via <>. > + > +#define ENTRY_OCC(entry) (entry) > +#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 1) > +#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + ctx->key_size) > +#define ENTRY_PSL(entry) (size_t*) (ENTRY_VAL(entry) + ctx->val_size) Unaligned access. You need to align entry_size to the required alignment of size_t and put this at the start of it the entry to avoid this. Apart from that: The layout of your entries implies that only two members are at a fixed offset from the start, so that accessing val and psl requires actual additions. If you make it psl, occ, key, val, then only val has a variable offset. This should be beneficial given that it is the less accessed of key and val (in particular if you combine several memcpy calls as I outline below). (This is of course a good reason not to make the layout of the entries public.) > + > +#define KEYS_EQUAL(k1, k2) !memcmp(k1, k2, ctx->key_size) > + > +int av_hashtable_init(AVHashtableContext* ctx, size_t key_size, size_t val_size, size_t max_entries) > +{ > + ctx->key_size = key_size; > + ctx->val_size = val_size; > + ctx->entry_size = 1 + key_size + val_size + sizeof(size_t); > + ctx->max_entries = max_entries; > + ctx->utilization = 0; > + ctx->crc = av_crc_get_table(AV_CRC_32_IEEE); > + if (!ctx->crc) > + return AVERROR_BUG; > + ctx->table = av_mallocz(ctx->entry_size * max_entries); av_calloc() > + if (!ctx->table) > + return AVERROR(ENOMEM); > + ctx->set_key = av_malloc(key_size); > + if (!ctx->set_key) > + return AVERROR(ENOMEM); > + ctx->set_val = av_malloc(key_size); Should be val_size. Your test should test scenarios with key_size != val_size to uncover this. > + if (!ctx->set_val) > + return AVERROR(ENOMEM); > + ctx->tmp_key = av_malloc(key_size); > + if (!ctx->tmp_key) > + return AVERROR(ENOMEM); > + ctx->tmp_val = av_malloc(val_size); > + if (!ctx->tmp_val) > + return AVERROR(ENOMEM); > + return 0; > +} (There are btw issues with what happens on error here, but we can ignore this in the first round.) > + > +static size_t hash_key(const AVHashtableContext* ctx, const void* key) > +{ > + return av_crc(ctx->crc, 0, key, ctx->key_size) % ctx->max_entries; > +} > + > +int av_hashtable_get(const 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) || *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(AVHashtableContext* ctx, const void* key, const void* val) > +{ > + int swapping = 0; > + size_t psl = 0; > + size_t hash = hash_key(ctx, key); > + > + memcpy(ctx->set_key, key, ctx->key_size); > + memcpy(ctx->set_val, 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), ctx->set_key))) { > + if (!*ENTRY_OCC(entry)) > + ctx->utilization++; > + *ENTRY_OCC(entry) = 1; > + memcpy(ENTRY_KEY(entry), ctx->set_key, ctx->key_size); > + memcpy(ENTRY_VAL(entry), ctx->set_val, ctx->val_size); > + *ENTRY_PSL(entry) = psl; > + return 1; > + } > + if (*ENTRY_PSL(entry) < psl) { > + if (ctx->utilization == ctx->max_entries) > + return 0; > + swapping = 1; > + memcpy(ctx->tmp_key, ctx->set_key, ctx->key_size); > + memcpy(ctx->tmp_val, ctx->set_val, ctx->val_size); Given that set_key/val are overwritten below, these two can be done via pointerswaps. > + memcpy(ctx->set_key, ENTRY_KEY(entry), ctx->key_size); > + memcpy(ctx->set_val, ENTRY_VAL(entry), ctx->val_size); > + memcpy(ENTRY_KEY(entry), ctx->tmp_key, ctx->key_size); > + memcpy(ENTRY_VAL(entry), ctx->tmp_val, ctx->val_size); > + FFSWAP(size_t, psl, *ENTRY_PSL(entry)); If you allocated foo_key and foo_val jointly, you can combine these memcpy's. In fact, you can allocate all four them jointly with a key-val-key-val layout and only keep one pointer in the context. You would then use an uint8_t *set, *val pointers on the stack of this function that are initialized at the beginning of this function. > + } > + psl++; > + } > + return 0; > +} > + > +int av_hashtable_delete(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) || *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) || !*ENTRY_PSL(next_entry)) { > + ctx->utilization--; > + return 1; > + } > + FFSWAP(uint8_t, *ENTRY_OCC(entry), *ENTRY_OCC(next_entry)); > + memcpy(ENTRY_KEY(entry), ENTRY_KEY(next_entry), ctx->key_size); > + memcpy(ENTRY_VAL(entry), ENTRY_VAL(next_entry), ctx->val_size); These two memcpy can be combined (if one lays out key and val sequentially in entry. Can even be combined with occ if you change the entry layout as I outlined above. > + *ENTRY_PSL(entry) = *ENTRY_PSL(next_entry) - 1; > + entry = next_entry; > + } > + } > + }; > + return 0; > +} > + > +void av_hashtable_clear(AVHashtableContext* ctx) > +{ > + memset(ctx->table, 0, ctx->entry_size * ctx->max_entries); > +} > + > +void av_hashtable_destroy(AVHashtableContext* ctx) > +{ > + av_freep(&ctx->table); > + av_freep(&ctx->set_key); > + av_freep(&ctx->set_val); > + av_freep(&ctx->tmp_key); > + av_freep(&ctx->tmp_val); > +} > \ No newline at end of file > diff --git a/libavutil/hashtable.h b/libavutil/hashtable.h > new file mode 100644 > index 0000000000..aac31ee922 > --- /dev/null > +++ b/libavutil/hashtable.h > @@ -0,0 +1,62 @@ > +/* > + * Generic hashtable > + * Copyright (C) 2024 Connor Worley > + * > + * 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 > + > +#include "crc.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. Such things should not be made public. Even if sizeof(AVHashtableContext) were part of the ABI. > + */ > + > +typedef struct AVHashtableContext { > + size_t key_size; > + size_t val_size; > + size_t entry_size; > + size_t max_entries; > + size_t utilization; > + const AVCRC *crc; You don't need the complete type here, so by using struct AVCRC *crc you can avoid the crc.h inclusion. > + uint8_t *table; > + /* used for swaps during insertion */ > + uint8_t *set_key; > + uint8_t *set_val; > + uint8_t *tmp_key; > + uint8_t *tmp_val; > +} AVHashtableContext; In patch 2/2 you are using this in a way that makes sizeof(AVHashtableContext) part of the ABI. Is this intended? Even if it is, the user should be forbidden from accessing anything in it. > + > +int av_hashtable_init(AVHashtableContext* ctx, size_t key_size, size_t val_size, size_t max_entries); We generally use > +int av_hashtable_get(const AVHashtableContext* ctx, const void* key, void* val); > +int av_hashtable_set(AVHashtableContext* ctx, const void* key, const void* val); > +int av_hashtable_delete(AVHashtableContext* ctx, const void* key); > +void av_hashtable_clear(AVHashtableContext* ctx); > +void av_hashtable_destroy(AVHashtableContext* ctx); > + > +#endif > \ No newline at end of file > diff --git a/libavutil/tests/hashtable.c b/libavutil/tests/hashtable.c > new file mode 100644 > index 0000000000..1cdbd7afd3 > --- /dev/null > +++ b/libavutil/tests/hashtable.c > @@ -0,0 +1,104 @@ > +/* > + * Generic hashtable tests > + * Copyright (C) 2024 Connor Worley > + * > + * 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) > +{ > + AVHashtableContext ctx; > + int k, v; > + > + // hashtable can store up to 3 int->int entries > + av_assert0(!av_hashtable_init(&ctx, sizeof(int), sizeof(int), 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_destroy(&ctx); > + > + return 0; > +} > \ No newline at end of file You should add these newlines, they are AFAIK required by the spec. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".