From e69b91fae4602b69c5ef45fcf82932adde8b31d8 Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Tue, 22 Nov 2022 21:16:11 -0500 Subject: Introduce BOP_CMP for optimized comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit the `OPTIMIZED_CMP` macro relied on a method lookup to determine whether `<=>` was overridden. The result of the lookup was cached, but only for the duration of the specific method that initialized the cmp_opt_data cache structure. With this method lookup, `[x,y].max` is slower than doing `x > y ? x : y` even though there's an optimized instruction for "new array max". (John noticed somebody a proposed micro-optimization based on this fact in https://github.com/mastodon/mastodon/pull/19903.) ```rb a, b = 1, 2 Benchmark.ips do |bm| bm.report('conditional') { a > b ? a : b } bm.report('method') { [a, b].max } bm.compare! end ``` Before: ``` Comparison: conditional: 22603733.2 i/s method: 19820412.7 i/s - 1.14x (± 0.00) slower ``` This commit replaces the method lookup with a new CMP basic op, which gives the examples above equivalent performance. After: ``` Comparison: method: 24022466.5 i/s conditional: 23851094.2 i/s - same-ish: difference falls within error ``` Relevant benchmarks show an improvement to Array#max and Array#min when not using the optimized newarray_max instruction as well. They are noticeably faster for small arrays with the relevant types, and the same or maybe a touch faster on larger arrays. ``` $ make benchmark COMPARE_RUBY= ITEM=array_min $ make benchmark COMPARE_RUBY= ITEM=array_max ``` The benchmarks added in this commit also look generally improved. Co-authored-by: John Hawthorn --- array.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) (limited to 'array.c') diff --git a/array.c b/array.c index adc4b3147a..cba1ed376a 100644 --- a/array.c +++ b/array.c @@ -3456,7 +3456,6 @@ rb_ary_rotate_m(int argc, VALUE *argv, VALUE ary) struct ary_sort_data { VALUE ary; VALUE receiver; - struct cmp_opt_data cmp_opt; }; static VALUE @@ -3502,15 +3501,15 @@ sort_2(const void *ap, const void *bp, void *dummy) VALUE a = *(const VALUE *)ap, b = *(const VALUE *)bp; int n; - if (FIXNUM_P(a) && FIXNUM_P(b) && CMP_OPTIMIZABLE(data->cmp_opt, Integer)) { + if (FIXNUM_P(a) && FIXNUM_P(b) && CMP_OPTIMIZABLE(INTEGER)) { if ((long)a > (long)b) return 1; if ((long)a < (long)b) return -1; return 0; } - if (STRING_P(a) && STRING_P(b) && CMP_OPTIMIZABLE(data->cmp_opt, String)) { + if (STRING_P(a) && STRING_P(b) && CMP_OPTIMIZABLE(STRING)) { return rb_str_cmp(a, b); } - if (RB_FLOAT_TYPE_P(a) && CMP_OPTIMIZABLE(data->cmp_opt, Float)) { + if (RB_FLOAT_TYPE_P(a) && CMP_OPTIMIZABLE(FLOAT)) { return rb_float_cmp(a, b); } @@ -3574,8 +3573,6 @@ rb_ary_sort_bang(VALUE ary) RBASIC_CLEAR_CLASS(tmp); data.ary = tmp; data.receiver = ary; - data.cmp_opt.opt_methods = 0; - data.cmp_opt.opt_inited = 0; RARRAY_PTR_USE(tmp, ptr, { ruby_qsort(ptr, len, sizeof(VALUE), rb_block_given_p()?sort_1:sort_2, &data); @@ -6056,7 +6053,6 @@ ary_max_opt_string(VALUE ary, long i, VALUE vmax) static VALUE rb_ary_max(int argc, VALUE *argv, VALUE ary) { - struct cmp_opt_data cmp_opt = { 0, 0 }; VALUE result = Qundef, v; VALUE num; long i; @@ -6076,13 +6072,13 @@ rb_ary_max(int argc, VALUE *argv, VALUE ary) else if (n > 0) { result = RARRAY_AREF(ary, 0); if (n > 1) { - if (FIXNUM_P(result) && CMP_OPTIMIZABLE(cmp_opt, Integer)) { + if (FIXNUM_P(result) && CMP_OPTIMIZABLE(INTEGER)) { return ary_max_opt_fixnum(ary, 1, result); } - else if (STRING_P(result) && CMP_OPTIMIZABLE(cmp_opt, String)) { + else if (STRING_P(result) && CMP_OPTIMIZABLE(STRING)) { return ary_max_opt_string(ary, 1, result); } - else if (RB_FLOAT_TYPE_P(result) && CMP_OPTIMIZABLE(cmp_opt, Float)) { + else if (RB_FLOAT_TYPE_P(result) && CMP_OPTIMIZABLE(FLOAT)) { return ary_max_opt_float(ary, 1, result); } else { @@ -6225,7 +6221,6 @@ ary_min_opt_string(VALUE ary, long i, VALUE vmin) static VALUE rb_ary_min(int argc, VALUE *argv, VALUE ary) { - struct cmp_opt_data cmp_opt = { 0, 0 }; VALUE result = Qundef, v; VALUE num; long i; @@ -6245,13 +6240,13 @@ rb_ary_min(int argc, VALUE *argv, VALUE ary) else if (n > 0) { result = RARRAY_AREF(ary, 0); if (n > 1) { - if (FIXNUM_P(result) && CMP_OPTIMIZABLE(cmp_opt, Integer)) { + if (FIXNUM_P(result) && CMP_OPTIMIZABLE(INTEGER)) { return ary_min_opt_fixnum(ary, 1, result); } - else if (STRING_P(result) && CMP_OPTIMIZABLE(cmp_opt, String)) { + else if (STRING_P(result) && CMP_OPTIMIZABLE(STRING)) { return ary_min_opt_string(ary, 1, result); } - else if (RB_FLOAT_TYPE_P(result) && CMP_OPTIMIZABLE(cmp_opt, Float)) { + else if (RB_FLOAT_TYPE_P(result) && CMP_OPTIMIZABLE(FLOAT)) { return ary_min_opt_float(ary, 1, result); } else { -- cgit v1.2.1