Bug #15929
closedArray#minmax is much slower than calling both #min and #max
Description
this is similar to issue 15807 about Ranges and maybe also to 13917
current situation:
- calling
Array#minmax
incurs a performance penalty of almost 50% compared to calling both#min
and#max
require 'benchmark/ips'
arr = (1..1000).map { rand }
Benchmark.ips do |x|
x.report('min, max') { [arr.min, arr.max] }
x.report('minmax') { arr.minmax }
end
min, max 53.832k (± 1.8%) i/s - 270.861k in 5.033263s
minmax 30.093k (± 1.2%) i/s - 151.980k in 5.051078s
background:
-
#minmax
is included viaEnumerable
-
Enumerable#minmax
does not call array's optimized#min
and#max
implementations
possible solutions:
- a) change
Enumerable#minmax
and let itrb_funcall
min
andmax
as suggested here (will also fix 15807) - b) implement minmax in array.c to call
rb_ary_min
andrb_ary_max
Files
Updated by jeremyevans0 (Jeremy Evans) over 5 years ago
- File array-minmax.patch array-minmax.patch added
janosch-x (Janosch Müller) wrote:
possible solutions:
- a) change
Enumerable#minmax
and let itrb_funcall
min
andmax
as suggested here (will also fix 15807)
We cannot use this approach. Enumerable#each
can have side effects, and you cannot iterate twice for minmax
in the general case. Consider:
File.open('...', &:minmax)
- b) implement minmax in array.c to call
rb_ary_min
andrb_ary_max
I think this is the correct approach in Array and any other class that overrides min
/max
. Attached is a patch that implements it.
Updated by janosch-x (Janosch Müller) over 5 years ago
jeremyevans0 (Jeremy Evans) wrote:
We cannot use this approach. [...]
I see, thanks for the explanation!
Updated by jeremyevans (Jeremy Evans) over 5 years ago
- Status changed from Open to Closed
Applied in changeset git|ced640951b0e7164a12ea1770330eba3e6109fc2.
Implement Array#minmax
Array#minmax was previous not implemented, so calling #minmax on
array was actually calling Enumerable#minmax. This is a simple
implementation of #minmax by just calling rb_ary_min and
rb_ary_max, which improves performance significantly.
Fixes [Bug #15929]