Project

General

Profile

Bug #15929

Array#minmax is much slower than calling both #min and #max

Added by janosch-x (Janosch Müller) about 1 month ago. Updated 21 days ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:93189]

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 via Enumerable
  • Enumerable#minmax does not call array's optimized #min and #max implementations

possible solutions:

  • a) change Enumerable#minmax and let it rb_funcall min and max as suggested here (will also fix 15807)
  • b) implement minmax in array.c to call rb_ary_min and rb_ary_max

Files

array-minmax.patch (2.65 KB) array-minmax.patch jeremyevans0 (Jeremy Evans), 06/17/2019 06:47 PM

Associated revisions

Revision ced64095
Added by jeremyevans (Jeremy Evans) 21 days ago

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]

History

Updated by jeremyevans0 (Jeremy Evans) about 1 month ago

janosch-x (Janosch Müller) wrote:

possible solutions:

  • a) change Enumerable#minmax and let it rb_funcall min and max 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 and rb_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) about 1 month ago

jeremyevans0 (Jeremy Evans) wrote:

We cannot use this approach. [...]

I see, thanks for the explanation!

#3

Updated by jeremyevans (Jeremy Evans) 21 days 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]

Also available in: Atom PDF