-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Replace excess &block in arguments to yield construction #1308
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
Conversation
Thanks for the update. Is this solving a specific problem you ran into? |
@cupakromer for me Benchmarks(all developers love benchmarks 😃) Code for compare yield and blockrequire 'benchmark/ips'
def yield_proc
yield
end
def block_proc(&block)
block.call
end
Benchmark.ips do |x|
x.report('yield'){ yield_proc{ 1 + 1 } }
x.report('block'){ block_proc{ 1 + 1 } }
end Results
|
That's a legit reason 😸 Would be interesting to see what sort of difference that makes in an actual spec. |
@cupakromer i use this code for run benchmarks require 'rails_helper'
require 'benchmark/ips'
# A very simple Rails engine
module MyEngine
class Engine < ::Rails::Engine
isolate_namespace MyEngine
end
Engine.routes.draw do
resources :widgets, only: [:show] do
get :random, on: :collection
end
end
class WidgetsController < ::ActionController::Base
def random
@random_widget = Widget.all.shuffle.first
redirect_to widget_path(@random_widget)
end
def show
@widget = Widget.find(params[:id])
render text: @widget.name
end
end
end
RSpec.describe MyEngine::WidgetsController, type: :controller do
Benchmark.ips do |x|
x.report('#routes with yield') do
routes { MyEngine::Engine.routes }
end
end
end This engine i found here And here are the results:
|
Thanks for the extra benchmark. As I was reviewing it I realized that it's not actually measuring the change since the block isn't actually invoked on the call to The results are very interesting:
and
Given this information and how require 'benchmark/ips'
def before_n_times(n, &block)
n.times { instance_exec(&block) }
end
def yield_n_times(n)
before_n_times(n) { yield }
end
def capture_block_and_yield_n_times(n, &block)
before_n_times(n) { yield }
end
def capture_block_and_call_n_times(n, &block)
before_n_times(n) { block.call }
end
[10, 25, 50, 100, 1000, 10000].each do |count|
puts "\n\nInvoking the block #{count} times\n"
Benchmark.ips do |x|
x.report("Yield #{count} times ") do
yield_n_times(count) { }
end
x.report("Capture block and yield #{count} times") do
capture_block_and_yield_n_times(count) { }
end
x.report("Capture block and call #{count} times ") do
capture_block_and_call_n_times(count) { }
end
end
end My results using Ruby 2.2.0:
This seems to show that the error margin is enough to negate any benefit from capturing the block initially. It also shows that not capturing the block is still faster at low rates of calling. If this holds for your system, I think this PR is good as is and we won't need to capture the block in the |
@cupakromer wow! thanks for this great answer and the investigation of the problem 💛 💚 💜 |
@cupakromer ping 😃 What do we with this PR? |
Replace excess &block in arguments to yield construction
Thanks again for the work on this. 😸 |
@cupakromer thanks you too! 💚 💛 ❤️ |
No description provided.