Project

General

Profile

Actions

Bug #13917

closed

Comparable#clamp is slower than using Array#min,max.

Added by kei-s (Kei Shiratsuchi) over 6 years ago. Updated over 4 years ago.

Status:
Rejected
Target version:
-
[ruby-dev:50254]

Description

Comparable#clamp is slower than using Array#min,max.
(I noticed it by @onk's tweet. https://twitter.com/onk/status/907856892604461056)

Performance

              user     system      total        real
minmax:   0.740000   0.000000   0.740000 (  0.732744)
clamp:    2.060000   0.010000   2.070000 (  2.072794)

Test Code

require 'benchmark'

Benchmark.bmbm do |x|
  v = Random.rand(-10..110)

  x.report "minmax:" do
    10000000.times { [99, [0, v].max].min }
  end

  x.report "clamp: " do
    10000000.times { v.clamp(0, 99) }
  end
end

Patch

I made patch for it. But I'm not sure this is good way.
https://gist.github.com/kei-s/b303aca105df5c26be9c98f833db80f7#file-compar-diff

After

              user     system      total        real
minmax:   0.820000   0.000000   0.820000 (  0.822517)
clamp:    1.090000   0.000000   1.090000 (  1.087491)

Other benchmark for this patch is here.
https://gist.github.com/kei-s/0c34cbe4e21a499601e8247077629082

Questions

  1. Should clamp version be faster than Array#min/max version?
    Array#min/max version would have overhead of array creation.

  2. Is OPTIMIZED_CMP in cmpint best way?
    Some method doesn't pass cmpint (e.g. Integer#>). But OPTMIZED_CMP checks Integer.

Updated by Hanmac (Hans Mackowiak) over 6 years ago

i can explain why Array#min/max isn't much slower, because it was optimized to not create Array overhead WHEN using variables
(interesting it isn't optimized when only using literals)

RubyVM::InstructionSequence.compile("[4,5].max").disasm
#0002 duparray         [4, 5]
#0004 opt_send_without_block <callinfo!mid:max, argc:0, ARGS_SIMPLE>, <callcache>

and this:

puts RubyVM::InstructionSequence.compile("x=4;[x,5].max").disasm
#0002 putobject        4
#0004 setlocal_OP__WC__0 3
#0006 getlocal_OP__WC__0 3
#0008 putobject        5
#0010 opt_newarray_max 2

clamp itself doesn't seems to be optimized like that

Updated by kei-s (Kei Shiratsuchi) over 6 years ago

Thank you, Hanmac. I understand why Array#min/max is so fast.
I guess clamp would be implemented in numeric.c and so on to be as fast as Array#min/max.

Updated by naruse (Yui NARUSE) over 6 years ago

Hanmac (Hans Mackowiak) wrote:

i can explain why Array#min/max isn't much slower, because it was optimized to not create Array overhead WHEN using variables
(interesting it isn't optimized when only using literals)

It's expected behavior.
doc/NEWS-2.4.0 says

* In some condition, `[x, y].max` and `[x, y].min` are optimized
  so that a temporal array is not created.  The concrete condition is
  an implementation detail: currently, the array literal must have no
  splat, must have at least one expression but literal, the length must
  be <= 0x100, and Array#max and min must not be redefined.  It will work
  in most casual and real-life use case where it is written with intent
  to `Math.max(x, y)`.

Updated by matz (Yukihiro Matsumoto) over 6 years ago

  • Status changed from Open to Rejected

Array#{min,max} are optimized because they are used frequently. I don't think clamp is used that much.

Matz.

Updated by naruse (Yui NARUSE) over 6 years ago

  • Status changed from Rejected to Assigned
  • Assignee set to nobu (Nobuyoshi Nakada)

The main topic "Comparable#clamp is slower than using Array#min,max." is valid.

@nobu (Nobuyoshi Nakada) Could you check the patch?

Updated by nobu (Nobuyoshi Nakada) over 6 years ago

It was same or slower with gcc 7.2.0.

benchmark results:
Execution time (sec)

name ruby 2.5.0dev (2017-10-08 trunk 60140) [x86_64-linux] built-ruby
comparable_object_between 2.600 2.736
comparable_object_clamp 3.590 3.579
comparable_object_less 1.632 1.679

Speedup ratio: compare with the result of `ruby 2.5.0dev (2017-10-08 trunk 60140) [x86_64-linux]' (greater is better)

name built-ruby
comparable_object_between 0.950
comparable_object_clamp 1.003
comparable_object_less 0.972
Actions #7

Updated by jeremyevans0 (Jeremy Evans) over 4 years ago

  • Status changed from Assigned to Rejected
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0