From 7368be756895e0643a6e82e68fdc6313c1561ce6 Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Sat, 2 Aug 2025 13:26:02 +0900 Subject: [PATCH] khash: add kh_replace optimization for efficient copying Add kh_replace function that uses direct memory copying instead of element-by-element rehashing for improved performance. - Add kh_replace_name function with smart handling of different table types - Optimize kh_copy to use kh_replace instead of element iteration - Update Set operations to use kh_replace for copying - Remove redundant kset_copy_replace function The optimization provides O(1) memory copy vs O(n) hash operations, handles small tables and hash tables correctly, and avoids infinite recursion issues with self-referential data structures. Co-authored-by: Claude --- include/mruby/khash.h | 54 +++++++++++++++++++++++++------------ mrbgems/mruby-set/src/set.c | 15 ++--------- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/include/mruby/khash.h b/include/mruby/khash.h index 086d7df5b..d39732ebf 100644 --- a/include/mruby/khash.h +++ b/include/mruby/khash.h @@ -101,7 +101,8 @@ static const uint8_t __m_either[] = {0x03, 0x0c, 0x30, 0xc0}; void kh_del_##name(mrb_state *mrb, kh_##name##_t *h, khint_t x); \ kh_##name##_t *kh_copy_##name(mrb_state *mrb, kh_##name##_t *h); \ void kh_init_data_##name(mrb_state *mrb, kh_##name##_t *h, khint_t size); \ - void kh_destroy_data_##name(mrb_state *mrb, kh_##name##_t *h); + void kh_destroy_data_##name(mrb_state *mrb, kh_##name##_t *h); \ + void kh_replace_##name(mrb_state *mrb, kh_##name##_t *dst, const kh_##name##_t *src); /* define kh_xxx_funcs @@ -314,22 +315,8 @@ static const uint8_t __m_either[] = {0x03, 0x0c, 0x30, 0xc0}; } \ kh_##name##_t *kh_copy_##name(mrb_state *mrb, kh_##name##_t *h) \ { \ - kh_##name##_t *h2; \ - khiter_t k, k2; \ - /* Cache source hash addresses */ \ - khkey_t *keys = kh_keys_##name(h); \ - khval_t *vals = kh_vals_##name(h); \ - \ - h2 = kh_init_##name(mrb); \ - for (k = kh_begin(h); k != kh_end(h); k++) { \ - if (kh_exist(name, h, k)) { \ - k2 = kh_put_##name(mrb, h2, keys[k], NULL); \ - if (kh_is_map) { \ - khval_t *new_vals = kh_vals_##name(h2); \ - new_vals[k2] = vals[k]; \ - } \ - } \ - } \ + kh_##name##_t *h2 = (kh_##name##_t*)mrb_calloc(mrb, 1, sizeof(kh_##name##_t)); \ + kh_replace_##name(mrb, h2, h); \ return h2; \ } \ void kh_init_data_##name(mrb_state *mrb, kh_##name##_t *h, khint_t size) { \ @@ -355,6 +342,38 @@ static const uint8_t __m_either[] = {0x03, 0x0c, 0x30, 0xc0}; mrb_free(mrb, h->data); /* Free only the data allocation */ \ h->data = NULL; \ } \ + } \ + void kh_replace_##name(mrb_state *mrb, kh_##name##_t *dst, const kh_##name##_t *src) \ + { \ + if (!src || (src->n_buckets == 0 && src->size == 0)) { \ + /* Empty source */ \ + kh_destroy_data_##name(mrb, dst); \ + dst->data = NULL; \ + dst->n_buckets = 0; \ + dst->size = 0; \ + } \ + else if (src->n_buckets == 0) { \ + /* Small table case */ \ + size_t data_size = sizeof(khkey_t) * KHASH_SMALL_THRESHOLD + \ + (kh_is_map ? sizeof(khval_t) * KHASH_SMALL_THRESHOLD : 0); \ + dst->data = mrb_realloc(mrb, dst->data, data_size); \ + dst->size = src->size; \ + dst->n_buckets = 0; \ + /* Copy only the used portion of keys and values */ \ + size_t copy_size = sizeof(khkey_t) * src->size + \ + (kh_is_map ? sizeof(khval_t) * src->size : 0); \ + memcpy(dst->data, src->data, copy_size); \ + } \ + else { \ + /* Regular hash table case */ \ + size_t data_size = (sizeof(khkey_t) + (kh_is_map ? sizeof(khval_t) : 0)) * src->n_buckets + \ + src->n_buckets / 4; \ + dst->data = mrb_realloc(mrb, dst->data, data_size); \ + dst->size = src->size; \ + dst->n_buckets = src->n_buckets; \ + /* Copy the entire data block: [keys][vals][flags] */ \ + memcpy(dst->data, src->data, data_size); \ + } \ } @@ -372,6 +391,7 @@ static const uint8_t __m_either[] = {0x03, 0x0c, 0x30, 0xc0}; #define kh_copy(name, mrb, h) kh_copy_##name(mrb, h) #define kh_init_data(name, mrb, h, size) kh_init_data_##name(mrb, h, size) #define kh_destroy_data(name, mrb, h) kh_destroy_data_##name(mrb, h) +#define kh_replace(name, mrb, dst, src) kh_replace_##name(mrb, dst, src) /* BREAKING CHANGE: Field access macros now require type name as first parameter * The macros keep their familiar names but now need the hash type name. diff --git a/mrbgems/mruby-set/src/set.c b/mrbgems/mruby-set/src/set.c index 94c942a88..d88934a2c 100644 --- a/mrbgems/mruby-set/src/set.c +++ b/mrbgems/mruby-set/src/set.c @@ -74,17 +74,6 @@ kset_copy_merge(mrb_state *mrb, kset_t *dst, kset_t *src) } } -/* Replace dst with a copy of src */ -static void -kset_copy_replace(mrb_state *mrb, kset_t *dst, kset_t *src) -{ - kset_t *tmp = kh_copy(set_val, mrb, src); - if (tmp) { - kset_destroy_data(mrb, dst); - *dst = *tmp; - mrb_free(mrb, tmp); - } -} /* Embedded set structure in RSet - exactly 3 pointers */ struct RSet { @@ -194,7 +183,7 @@ set_init_copy(mrb_state *mrb, mrb_value self) kset_t *self_set = set_get_kset(mrb, self); kset_init_data(mrb, self_set, kset_size(orig_set)); - kset_copy_replace(mrb, self_set, orig_set); + kh_replace(set_val, mrb, self_set, orig_set); return self; } @@ -553,7 +542,7 @@ set_core_xor(mrb_state *mrb, mrb_value self) return result; } if (kset_is_empty(other_set)) { - kset_copy_replace(mrb, result_set, self_set); + kh_replace(set_val, mrb, result_set, self_set); return result; }