[ruby-dev:50263] [Ruby trunk Bug#13917] Comparable#clamp is slower than using Array#min, max.

From: naruse@...
Date: 2017-09-25 05:28:20 UTC
List: ruby-dev #50263
Issue #13917 has been updated by naruse (Yui NARUSE).


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)`.
```

----------------------------------------
Bug #13917: Comparable#clamp is slower than using Array#min,max.
https://bugs.ruby-lang.org/issues/13917#change-66863

* Author: kei-s (Kei Shiratsuchi)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
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. 




-- 
https://bugs.ruby-lang.org/

In This Thread

Prev Next