From 8944a064d0fd7947b8c2b6c761be3e3a0c9073af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Ondruch?= Date: Fri, 22 Dec 2023 14:16:48 +0100 Subject: [PATCH 1/2] Revert "compare_by_identity: remove alloc for non-empty Hash" This reverts commit 11fa76b1b521072c200c78ea023960221ff426d6. --- hash.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/hash.c b/hash.c index 78e9d9a2d6..f6525ba4a5 100644 --- a/hash.c +++ b/hash.c @@ -4377,16 +4377,13 @@ rb_hash_compare_by_id(VALUE hash) if (hash_iterating_p(hash)) { rb_raise(rb_eRuntimeError, "compare_by_identity during iteration"); } + ar_force_convert_table(hash, __FILE__, __LINE__); + HASH_ASSERT(RHASH_ST_TABLE_P(hash)); if (RHASH_TABLE_EMPTY_P(hash)) { // Fast path: There's nothing to rehash, so we don't need a `tmp` table. - // We're most likely an AR table, so this will need an allocation. - ar_force_convert_table(hash, __FILE__, __LINE__); - HASH_ASSERT(RHASH_ST_TABLE_P(hash)); - RHASH_ST_TABLE(hash)->type = &identhash; - } - else { + } else { // Slow path: Need to rehash the members of `self` into a new // `tmp` table using the new `identhash` compare/hash functions. tmp = hash_alloc(0); @@ -4394,10 +4391,8 @@ rb_hash_compare_by_id(VALUE hash) identtable = RHASH_ST_TABLE(tmp); rb_hash_foreach(hash, rb_hash_rehash_i, (VALUE)tmp); - rb_hash_free(hash); - // We know for sure `identtable` is an st table, - // so we can skip `ar_force_convert_table` here. + rb_hash_free(hash); RHASH_ST_TABLE_SET(hash, identtable); RHASH_ST_CLEAR(tmp); } From f5c415300ffe63e41e46c6b88b8634a3bad0c7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Ondruch?= Date: Fri, 22 Dec 2023 14:17:14 +0100 Subject: [PATCH 2/2] Revert "compare_by_identity: remove alloc for empty Hash" This reverts commit b5c6c0122f5b010cb5f43e7a236c4ba2b1d56a2a. --- hash.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/hash.c b/hash.c index f6525ba4a5..cf83675c70 100644 --- a/hash.c +++ b/hash.c @@ -4380,22 +4380,15 @@ rb_hash_compare_by_id(VALUE hash) ar_force_convert_table(hash, __FILE__, __LINE__); HASH_ASSERT(RHASH_ST_TABLE_P(hash)); - if (RHASH_TABLE_EMPTY_P(hash)) { - // Fast path: There's nothing to rehash, so we don't need a `tmp` table. - RHASH_ST_TABLE(hash)->type = &identhash; - } else { - // Slow path: Need to rehash the members of `self` into a new - // `tmp` table using the new `identhash` compare/hash functions. - tmp = hash_alloc(0); - hash_st_table_init(tmp, &identhash, RHASH_SIZE(hash)); - identtable = RHASH_ST_TABLE(tmp); + tmp = hash_alloc(0); + hash_st_table_init(tmp, &identhash, RHASH_SIZE(hash)); + identtable = RHASH_ST_TABLE(tmp); - rb_hash_foreach(hash, rb_hash_rehash_i, (VALUE)tmp); + rb_hash_foreach(hash, rb_hash_rehash_i, (VALUE)tmp); - rb_hash_free(hash); - RHASH_ST_TABLE_SET(hash, identtable); - RHASH_ST_CLEAR(tmp); - } + rb_hash_free(hash); + RHASH_ST_TABLE_SET(hash, identtable); + RHASH_ST_CLEAR(tmp); return hash; }