From 169654ad9b205d296e7e85dc46ad884179990cd3 Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Fri, 20 Jun 2025 13:49:07 +0900 Subject: [PATCH] mruby-set: refactor Set#subtract The code is written by Atlassian Rovodev. --- mrbgems/mruby-set/mrblib/set.rb | 18 +++++++++--- mrbgems/mruby-set/src/set.c | 52 ++++++++++++++------------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/mrbgems/mruby-set/mrblib/set.rb b/mrbgems/mruby-set/mrblib/set.rb index 31d4c12dc..16e35428b 100644 --- a/mrbgems/mruby-set/mrblib/set.rb +++ b/mrbgems/mruby-set/mrblib/set.rb @@ -20,10 +20,20 @@ class Set self end - # Helper method for subtract with enumerable - def __subtract_enum(enum) - __do_with_enum(enum) { |o| delete(o) } - self + # Deletes every element that appears in the given enumerable object and + # returns self. + # + # @param [Enumerable] enum The enumerable object containing elements to remove + # @return [Set] self + def subtract(enum) + if enum.is_a?(Set) + # Fast path: Call C-implemented function for Set-to-Set subtraction + __set_subtract(enum) + else + # General path: Remove each element from the enumerable + __do_with_enum(enum) { |o| delete(o) } + self + end end # Returns a new set containing elements common to the set and the given diff --git a/mrbgems/mruby-set/src/set.c b/mrbgems/mruby-set/src/set.c index b148f58a2..ad08139dd 100644 --- a/mrbgems/mruby-set/src/set.c +++ b/mrbgems/mruby-set/src/set.c @@ -368,43 +368,35 @@ set_merge(mrb_state *mrb, mrb_value self) } /* - * call-seq: - * set.subtract(enum) -> self - * - * Deletes every element that appears in the given enumerable object and - * returns self. + * Core implementation of Set-to-Set subtraction (mutating version) + * This is an internal method that will be called from Ruby */ static mrb_value -set_subtract(mrb_state *mrb, mrb_value self) +set_core_subtract(mrb_state *mrb, mrb_value self) { - mrb_value enum_obj; - khash_t(set) *kh; + mrb_value other; + khash_t(set) *self_kh, *other_kh; - mrb_get_args(mrb, "o", &enum_obj); - kh = set_get_khash(mrb, self); - if (!kh) return self; + mrb_get_args(mrb, "o", &other); - if (mrb_obj_class(mrb, enum_obj) == mrb_obj_class(mrb, self)) { - /* Optimized path for Set objects */ - khash_t(set) *other_kh = set_get_khash(mrb, enum_obj); - if (other_kh) { - khiter_t k, del_k; - int ai = mrb_gc_arena_save(mrb); - for (k = kh_begin(other_kh); k != kh_end(other_kh); k++) { - if (kh_exist(other_kh, k)) { - del_k = kh_get(set, mrb, kh, kh_key(other_kh, k)); - if (del_k != kh_end(kh)) { - kh_del(set, mrb, kh, del_k); - } - mrb_gc_arena_restore(mrb, ai); - } + self_kh = set_get_khash(mrb, self); + if (!self_kh) return self; + + other_kh = set_get_khash(mrb, other); + if (!other_kh) return self; + + /* Remove all elements that are in other set */ + khiter_t k, del_k; + int ai = mrb_gc_arena_save(mrb); + for (k = kh_begin(other_kh); k != kh_end(other_kh); k++) { + if (kh_exist(other_kh, k)) { + del_k = kh_get(set, mrb, self_kh, kh_key(other_kh, k)); + if (del_k != kh_end(self_kh)) { + kh_del(set, mrb, self_kh, del_k); } + mrb_gc_arena_restore(mrb, ai); } } - else { - /* General enumerable path - delegate to Ruby */ - return mrb_funcall_with_block(mrb, self, MRB_SYM(__subtract_enum), 1, &enum_obj, mrb_nil_value()); - } return self; } @@ -962,7 +954,7 @@ mrb_mruby_set_gem_init(mrb_state *mrb) mrb_define_method(mrb, set, "replace", set_replace, MRB_ARGS_REQ(1)); mrb_define_method(mrb, set, "merge", set_merge, MRB_ARGS_REQ(1)); - mrb_define_method(mrb, set, "subtract", set_subtract, MRB_ARGS_REQ(1)); + mrb_define_method(mrb, set, "__set_subtract", set_core_subtract, MRB_ARGS_REQ(1)); mrb_define_method(mrb, set, "__set_union", set_core_union, MRB_ARGS_REQ(1));