Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix breaking change introduced in https://github.com/CanCanCommunity/cancancan/pull/204 #614

Merged
merged 3 commits into from
Mar 6, 2020

Conversation

coorasse
Copy link
Member

context: #204 (comment)

Kernel.#Array calls not only Object#to_ary but also Object#to_a but some classes which represent cluster may implement to_a.

https://api.rubyonrails.org/classes/Array.html#method-c-wrap

I think that's why ActiveSupport has Array.wrap.

https://api.rubyonrails.org/classes/Array.html#method-c-wrap

I confirmed there's no performance regression via the following snippet:

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "benchmark-ips", "2.7.2"
  gem "cancancan", "2.3.0"
  gem "allocation_tracer", "0.6.3"
end

module CanCan
  class Rule
    def initialize(base_behavior, action, subject, conditions, block, __flag__:)
      both_block_and_hash_error = 'You are not able to supply a block with a hash of conditions in '\
                                  "#{action} #{subject} ability. Use either one."
      raise Error, both_block_and_hash_error if conditions.is_a?(Hash) && block

      @match_all = action.nil? && subject.nil?
      @base_behavior = base_behavior
      case __flag__
      when :array
        @actions = Array(action)
        @subjects = Array(subject)
      when :flatten
        @actions = [action].flatten
        @subjects = [subject].flatten
      when :wrap
        @actions = wrap(action)
        @subjects = wrap(subject)
      else
        raise
      end
      @conditions = conditions || {}
      @block = block
    end

    def wrap(object)
      if object.nil?
        []
      elsif object.respond_to?(:to_ary)
        object.to_ary || [object]
      else
        [object]
      end
    end
  end
end

def memprof(name)
  GC.disable
  before = `ps -o rss -p #{Process.pid}`.split("\n").last.to_i
  ObjectSpace::AllocationTracer.trace do
    10_000.times do
      yield
    end
  end
  after = `ps -o rss -p #{Process.pid}`.split("\n").last.to_i

  puts "[#{name}] Mem diff: #{(after - before).to_f/1024.0} MB"
  GC.enable
  GC.start
end

puts "Ruby #{RUBY_VERSION}"

memprof("Kernel.#Array") { CanCan::Rule.new(nil, [:read, :write], [:foo, :bar], nil, nil, __flag__: :array) }
memprof("Array#flatten") { CanCan::Rule.new(nil, [:read, :write], [:foo, :bar], nil, nil, __flag__: :flatten) }
memprof("Array.wrap")    { CanCan::Rule.new(nil, [:read, :write], [:foo, :bar], nil, nil, __flag__: :wrap) }

Benchmark.ips do |b|
  b.report("Kernel.#Array") { CanCan::Rule.new(nil, [:read, :write], [:foo, :bar], nil, nil, __flag__: :array) }
  b.report("Array#flatten") { CanCan::Rule.new(nil, [:read, :write], [:foo, :bar], nil, nil, __flag__: :flatten) }
  b.report("Array.wrap")    { CanCan::Rule.new(nil, [:read, :write], [:foo, :bar], nil, nil, __flag__: :wrap) }

  b.compare!
end

__END__
Ruby 2.6.1
[Kernel.#Array] Mem diff: 21.59765625 MB
[Array#flatten] Mem diff: 2.21484375 MB
[Array.wrap] Mem diff: 0.02734375 MB
Warming up --------------------------------------
       Kernel.#Array     5.024k i/100ms
       Array#flatten     3.863k i/100ms
          Array.wrap     5.217k i/100ms
Calculating -------------------------------------
       Kernel.#Array    204.954k (±21.6%) i/s -    964.608k in   5.003037s
       Array#flatten    143.892k (±17.3%) i/s -    695.340k in   5.005342s
          Array.wrap    195.569k (±18.5%) i/s -    933.843k in   5.003945s

Comparison:
       Kernel.#Array:   204954.3 i/s
          Array.wrap:   195569.0 i/s - same-ish: difference falls within error
       Array#flatten:   143892.4 i/s - same-ish: difference falls within error

Ruby 2.5.3
[Kernel.#Array] Mem diff: 21.98046875 MB
[Array#flatten] Mem diff: 0.93359375 MB
[Array.wrap] Mem diff: 0.0 MB
Warming up --------------------------------------
       Kernel.#Array     4.816k i/100ms
       Array#flatten     4.235k i/100ms
          Array.wrap     4.610k i/100ms
Calculating -------------------------------------
       Kernel.#Array    166.440k (±30.7%) i/s -    722.400k in   5.010362s
       Array#flatten    136.055k (±23.3%) i/s -    639.485k in   5.009729s
          Array.wrap    173.409k (±18.6%) i/s -    820.580k in   5.019846s

Comparison:
          Array.wrap:   173408.7 i/s
       Kernel.#Array:   166439.5 i/s - same-ish: difference falls within error
       Array#flatten:   136054.8 i/s - same-ish: difference falls within error

Ruby 2.4.5
[Kernel.#Array] Mem diff: 17.0859375 MB
[Array#flatten] Mem diff: 0.0234375 MB
[Array.wrap] Mem diff: 0.0 MB
Warming up --------------------------------------
      Kernel.#Array     4.746k i/100ms
      Array#flatten     4.019k i/100ms
          Array.wrap     4.347k i/100ms
Calculating -------------------------------------
      Kernel.#Array    195.099k (±17.4%) i/s -    939.708k in   4.994105s
      Array#flatten    117.881k (±19.3%) i/s -    562.660k in   5.012299s
          Array.wrap    160.679k (±13.9%) i/s -    782.460k in   5.005515s

Comparison:
      Kernel.#Array:   195099.4 i/s
         Array.wrap:   160678.6 i/s - same-ish: difference falls within error
      Array#flatten:   117881.1 i/s - 1.66x  slower

@coorasse coorasse self-assigned this Jan 18, 2020
@coorasse
Copy link
Member Author

coorasse commented Mar 6, 2020

tests break. closing.

@coorasse coorasse closed this Mar 6, 2020
@coorasse coorasse reopened this Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants