From 4c79a5e24d667ea1974d74119d758658369ead16 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 5 Jan 2016 00:48:29 -0600 Subject: [PATCH 01/36] Setup dummy test server [ci skip] Note: Serializer caching can be completely disabled by passing in `CACHE_ON=off bin/server_dummy start` since Serializer#_cache is only set at boot. --- bin/serve_dummy | 39 +++++++++++++ test/dummy/app.rb | 80 ++++++++++++++++++++++++++ test/dummy/config.ru | 3 + test/dummy/controllers.rb | 69 +++++++++++++++++++++++ test/dummy/fixtures.rb | 114 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 305 insertions(+) create mode 100755 bin/serve_dummy create mode 100644 test/dummy/app.rb create mode 100644 test/dummy/config.ru create mode 100644 test/dummy/controllers.rb create mode 100644 test/dummy/fixtures.rb diff --git a/bin/serve_dummy b/bin/serve_dummy new file mode 100755 index 000000000..960a7126d --- /dev/null +++ b/bin/serve_dummy @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +set -e + +case "$1" in + + start) + config="${CONFIG_RU:-test/dummy/config.ru}" + bundle exec ruby -Ilib -S rackup "$config" --daemonize --pid tmp/dummy_app.pid --warn --server webrick + until [ -f 'tmp/dummy_app.pid' ]; do + sleep 0.1 # give it time to start.. I don't know a better way + done + cat tmp/dummy_app.pid + true + ;; + + stop) + if [ -f 'tmp/dummy_app.pid' ]; then + kill -TERM $(cat tmp/dummy_app.pid) + else + echo 'No pidfile' + false + fi + ;; + + status) + if [ -f 'tmp/dummy_app.pid' ]; then + kill -0 $(cat tmp/dummy_app.pid) + [ "$?" -eq 0 ] + else + echo 'No pidfile' + false + fi + ;; + + *) + echo "Usage: $0 [start|stop|status]" + ;; + +esac diff --git a/test/dummy/app.rb b/test/dummy/app.rb new file mode 100644 index 000000000..2704d1949 --- /dev/null +++ b/test/dummy/app.rb @@ -0,0 +1,80 @@ +# https://github.com/rails-api/active_model_serializers/pull/872 +# approx ref 792fb8a9053f8db3c562dae4f40907a582dd1720 to test against +require 'bundler/setup' + +require 'rails' +require 'active_model' +require 'active_support' +require 'active_support/json' +require 'action_controller' +require 'action_controller/test_case' +require 'action_controller/railtie' +abort "Rails application already defined: #{Rails.application.class}" if Rails.application + +# ref: https://gist.github.com/bf4/8744473 +class DummyApp < Rails::Application + config.action_controller.perform_caching = ENV['CACHE_ON'] != 'off' + config.action_controller.cache_store = ActiveSupport::Cache.lookup_store(:memory_store) + + # Set up production configuration + config.eager_load = true + config.cache_classes = true + + config.active_support.test_order = :random + config.secret_token = '1234' + config.secret_key_base = 'abc123' + config.logger = Logger.new(IO::NULL) + + class DummyLogger < ActiveSupport::Logger + def initialize + @file = StringIO.new + super(@file) + end + + def messages + @file.rewind + @file.read + end + end +end + +require 'active_model_serializers' + +# Initialize app before any serializers are defined, for sanity's sake. +# Otherwise, you have to manually set perform caching. +# +# Details: +# +# 1. Upon load, when AMS.config.perform_caching is true, +# serializers inherit the cache store from ActiveModelSerializers.config.cache_store +# 1. If the serializers are loaded before Rails is initialized (`Rails.application.initialize!`), +# these values are nil, and are not applied to the already loaded serializers +# 1. If Rails is initialized before any serializers are loaded, then the configs are set, +# and are used when serializers are loaded +# 1. In either case, `ActiveModelSerializers.config.cache_store`, and +# `ActiveModelSerializers.config.perform_caching` can be set at any time before the serializers +# are loaded, +# e.g. `ActiveModel::Serializer.config.cache_store ||= +# ActiveSupport::Cache.lookup_store(ActionController::Base.cache_store || +# Rails.cache || :memory_store)` +# and `ActiveModelSerializers.config.perform_caching = true` +# 1. If the serializers are loaded before Rails is initialized, then, +# you can set the `_cache` store directly on the serializers. +# `ActiveModel::Serializer._cache ||= +# ActiveSupport::Cache.lookup_store(ActionController::Base.cache_store || +# Rails.cache || :memory_store` +# is sufficient. +# Setting `_cache` to a truthy value will cause the CachedSerializer +# to consider it cached, which will apply to all serializers (bug? :bug: ) +# +# This happens, in part, because the cache store is set for a serializer +# when `cache` is called, and cache is usually called when the serializer is defined. +# +# So, there's now a 'workaround', something to debug, and a starting point. +Rails.application.initialize! + +# HACK: Serializer::cache depends on the ActionController-dependent configs being set. +ActiveSupport.on_load(:action_controller) do + require_relative 'fixtures' +end +require_relative 'controllers' diff --git a/test/dummy/config.ru b/test/dummy/config.ru new file mode 100644 index 000000000..908eb28c4 --- /dev/null +++ b/test/dummy/config.ru @@ -0,0 +1,3 @@ +require File.expand_path(['..', 'app'].join(File::SEPARATOR), __FILE__) + +run Rails.application diff --git a/test/dummy/controllers.rb b/test/dummy/controllers.rb new file mode 100644 index 000000000..8bcadeecc --- /dev/null +++ b/test/dummy/controllers.rb @@ -0,0 +1,69 @@ +class PostController < ActionController::Base + POST = + begin + comment = Comment.new(id: 1, body: 'ZOMG A COMMENT') + author = Author.new(id: 1, name: 'Joao Moura.') + Post.new(id: 1, title: 'New Post', blog: nil, body: 'Body', comments: [comment], author: author) + end + + def render_with_caching_serializer + toggle_cache_status + render json: POST, serializer: CachingPostSerializer, adapter: :json, meta: { caching: perform_caching } + end + + def render_with_non_caching_serializer + toggle_cache_status + render json: POST, adapter: :json, meta: { caching: perform_caching } + end + + def render_cache_status + toggle_cache_status + # Uncomment to debug + # STDERR.puts cache_store.class + # STDERR.puts cache_dependencies + # ActiveSupport::Cache::Store.logger.debug [ActiveModelSerializers.config.cache_store, ActiveModelSerializers.config.perform_caching, CachingPostSerializer._cache, perform_caching, params].inspect + render json: { caching: perform_caching, meta: { cache_log: cache_messages, cache_status: cache_status } }.to_json + end + + def clear + ActionController::Base.cache_store.clear + # Test caching is on + logger = DummyApp::DummyLogger.new + ActiveSupport::Cache::Store.logger = logger # seems to be the best way + # the below is used in some rails tests but isn't available/working in all versions, so far as I can tell + # https://github.com/rails/rails/pull/15943 + # ActiveSupport::Notifications.subscribe(/^cache_(.*)\.active_support$/) do |*args| + # logger.debug ActiveSupport::Notifications::Event.new(*args) + # end + render json: 'ok'.to_json + end + + private + + def cache_status + { + controller: perform_caching, + app: Rails.configuration.action_controller.perform_caching, + serializers: Rails.configuration.serializers.each_with_object({}) {|serializer, data| data[serializer.name] = serializer._cache.present? } + } + end + + def cache_messages + ActiveSupport::Cache::Store.logger.is_a?(DummyApp::DummyLogger) && ActiveSupport::Cache::Store.logger.messages.split("\n") + end + + def toggle_cache_status + case params[:on] + when 'on'.freeze then self.perform_caching = true + when 'off'.freeze then self.perform_caching = false + else nil # no-op + end + end +end + +Rails.application.routes.draw do + get '/status(/:on)' => 'post#render_cache_status' + get '/clear' => 'post#clear' + get '/caching(/:on)' => 'post#render_with_caching_serializer' + get '/non_caching(/:on)' => 'post#render_with_non_caching_serializer' +end diff --git a/test/dummy/fixtures.rb b/test/dummy/fixtures.rb new file mode 100644 index 000000000..bcd248382 --- /dev/null +++ b/test/dummy/fixtures.rb @@ -0,0 +1,114 @@ +Rails.configuration.serializers = [] +class AuthorSerializer < ActiveModel::Serializer + attributes :id, :name + + has_many :posts, embed: :ids + has_one :bio +end +Rails.configuration.serializers << AuthorSerializer + +class BlogSerializer < ActiveModel::Serializer + attributes :id, :name +end +Rails.configuration.serializers << BlogSerializer + +class CommentSerializer < ActiveModel::Serializer + attributes :id, :body + + belongs_to :post + belongs_to :author +end +Rails.configuration.serializers << CommentSerializer + +class PostSerializer < ActiveModel::Serializer + attributes :id, :title, :body + + has_many :comments, serializer: CommentSerializer + belongs_to :blog, serializer: BlogSerializer + belongs_to :author, serializer: AuthorSerializer + + def blog + Blog.new(id: 999, name: 'Custom blog') + end +end +Rails.configuration.serializers << PostSerializer + +class CachingAuthorSerializer < AuthorSerializer + cache key: 'writer', only: [:body], skip_digest: true +end +Rails.configuration.serializers << CachingAuthorSerializer + +class CachingCommentSerializer < CommentSerializer + cache expires_in: 1.day, skip_digest: true +end +Rails.configuration.serializers << CachingCommentSerializer + +class CachingPostSerializer < PostSerializer + cache key: 'post', expires_in: 0.1, skip_digest: true + belongs_to :blog, serializer: BlogSerializer + belongs_to :author, serializer: CachingAuthorSerializer + has_many :comments, serializer: CachingCommentSerializer +end +Rails.configuration.serializers << CachingPostSerializer + +# ActiveModelSerializers::Model is a convenient +# serializable class to inherit from when making +# serializable non-activerecord objects. +class DummyModel + include ActiveModel::Model + include ActiveModel::Serializers::JSON + + attr_reader :attributes + + def initialize(attributes = {}) + @attributes = attributes + super + end + + # Defaults to the downcased model name. + def id + attributes.fetch(:id) { self.class.name.downcase } + end + + # Defaults to the downcased model name and updated_at + def cache_key + attributes.fetch(:cache_key) { "#{self.class.name.downcase}/#{id}-#{updated_at.strftime("%Y%m%d%H%M%S%9N")}" } + end + + # Defaults to the time the serializer file was modified. + def updated_at + attributes.fetch(:updated_at) { File.mtime(__FILE__) } + end + + def read_attribute_for_serialization(key) + if key == :id || key == 'id' + attributes.fetch(key) { id } + else + attributes[key] + end + end +end + +class Comment < DummyModel + attr_accessor :id, :body + + def cache_key + "#{self.class.name.downcase}/#{id}" + end +end + +class Author < DummyModel + attr_accessor :id, :name, :posts +end + +class Post < DummyModel + attr_accessor :id, :title, :body, :comments, :blog, :author + + def cache_key + 'benchmarking::post/1-20151215212620000000000' + end +end + +class Blog < DummyModel + attr_accessor :id, :name +end From 0317cbe1ae3d9e7dbfe3b2ef0064beb5ed894186 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 19 Jan 2016 00:23:02 -0500 Subject: [PATCH 02/36] wip --- bin/revision_runner | 212 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 bin/revision_runner diff --git a/bin/revision_runner b/bin/revision_runner new file mode 100644 index 000000000..0d8a3804f --- /dev/null +++ b/bin/revision_runner @@ -0,0 +1,212 @@ +#!/usr/bin/env ruby +require 'fileutils' +require 'pathname' +require 'shellwords' +require 'English' +require 'net/http' +require 'json' + +############################ +# +# A wrapper around git-bisect that stashes some code (bin/serve_dummy) for use in making requests +# running against each revision. +# +# USAGE +# +# bin/revision_runner +# defaults to the current branch +# defaults to the master branch +# Runs across every revisiion in the range, inclusive. +# Aborts when exit code is non-zero. +########################### + +class RevisionRunner + BadRevisionError = Class.new(StandardError) + ROOT = Pathname File.join(File.dirname(__FILE__), '..') + TMP_DIR_BASE = File.join('tmp', 'revision_runner') + TMP_DIR = File.join(ROOT, TMP_DIR_BASE) + + attr_accessor :url_base + + def initialize(prepend: '', append: '') + self.url_base = "http://localhost:9292" + refresh_temp_dir + end + + def temp_dir_empty? + Dir[File.join(TMP_DIR, '*')].none? + end + + def empty_temp_dir + return if temp_dir_empty? + FileUtils.mkdir_p(TMP_DIR) + Dir[File.join(TMP_DIR, '*')].each do |file| + if File.directory?(file) + FileUtils.rm_rf(file) + else + FileUtils.rm(file) + end + end + end + + def fill_temp_dir + Dir[File.join(ROOT, 'test', 'dummy', '*.{rb,ru}')].each do |file| + FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) + end + file = File.join('bin', 'serve_dummy') + FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) + at_exit { empty_temp_dir } + end + + def refresh_temp_dir + empty_temp_dir + fill_temp_dir + end + + module GitCommands + module_function + + def current_branch + @current_branch ||= `cat .git/HEAD | cut -d/ -f3,4,5`.chomp + end + + def revisions(start_ref, end_ref) + cmd = "git rev-list --reverse #{start_ref}..#{end_ref}" + `#{cmd}`.chomp.split("\n") + end + + def checkout_ref(ref) + debug { `git checkout #{ref}`.chomp } + abort "Checkout failed: #{ref}, #{$CHILD_STATUS.exitstatus}" unless $CHILD_STATUS.success? + end + + def revision_description(rev) + `git log --oneline -1 #{rev}` + end + + def bundle + `rm -f Gemfile.lock; bundle check || bundle --local --quiet || bundle --quiet` + end + + def clean_head + system('git reset --hard --quiet') + end + end + + def run_revisions(ref1: nil, ref2: nil) + ref0 = current_branch + ref1 ||= current_branch + ref2 ||= 'master' + reports = {} + + revisions(ref1, ref2).each do |rev| + STDERR.puts "Checking out: #{revision_description(rev)}" + + reports[rev] = run_at_ref(rev) + clean_head + end + checkout_ref(ref0) + debug { "OK for all revisions!" } + reports + rescue Exception # rubocop:disable Lint/RescueException + STDERR.puts $!.message + checkout_ref(ref0) + raise + ensure + return reports + end + + def run_at_ref(ref) + checkout_ref(ref) + bundle + restart_server + end + + def run_cmd + assert_responses(get_caching, get_non_caching) + rescue BadRevisionError => e + debug { e.message } + exit 1 + end + + def get_caching + get(url_base + "/caching/on") + end + + def get_non_caching + get(url_base + "/non_caching/on") + end + + def assert_responses(caching, non_caching) + assert_equal(caching[:code], '200', "Caching response failed: #{caching}") + assert_equal(caching[:body], expected, "Caching response format failed: \n+ #{caching[:body]}\n- #{expected}") + assert_equal(caching[:content_type], 'application/json', "Caching response content type failed: \n+ #{caching[:content_type]}\n- application/json") + assert_equal(non_caching[:code], '200', "Non caching response failed: #{non_caching}") + assert_equal(non_caching[:body], expected, "Non Caching response format failed: \n+ #{non_caching[:body]}\n- #{expected}") + assert_equal(non_caching[:content_type], 'application/json', "Non caching response content type failed: \n+ #{non_caching[:content_type]}\n- application/json") + end + + def get(url) + uri = URI(url) + response = Net::HTTP.get_response(uri) + { code: response.code, body: JSON.load(response.body), content_type: response.content_type } + end + + def expected + @expected ||= + { + "post" => { + "id" => 1, + "title" => "New Post", + "body" => "Body", + "comments" => [ + { + "id" => 1, + "body" => "ZOMG A COMMENT" + } + ], + "blog" => { + "id" => 999, + "name" => "Custom blog" + }, + "author" => { + "id" => 1, + "name" => "Joao Moura." + } + } + } + end + + def assert_equal(expected, actual, message) + return true if expected == actual + fail BadRevisionError, messsage + end + + def restart_server + server_script = File.join(TMP_DIR_BASE, 'serve_dummy') + system("#{server_script} stop") + at_exit { system("#{server_script} stop") } + config_ru = Shellwords.shellescape(File.join(TMP_DIR_BASE, 'config.ru')) + pid = `CONFIG_RU=#{config_ru} #{server_script} start`.chomp + abort "No pid" if pid.empty? + pid = Integer(pid) + Process.kill(0, pid) # confirm running, else it raises + end + + def debug(msg = '') + if block_given? && $DEBUG + STDOUT.puts yield + else + STDOUT.puts msg + end + end + +end + +if $PROGRAM_NAME == __FILE__ + runner = RevisionRunner.new + reports = runner.run_revisions(ref1: ARGV[0], ref2: ARGV[1]) + reports.each do |name, value| + STDERR.puts "#{name}\n\t#{value}" + end +end From b564a2e1591031c14fd791cbc552205abc82ae94 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 19 Jan 2016 00:28:03 -0500 Subject: [PATCH 03/36] wip2 --- bin/revision_runner | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) mode change 100644 => 100755 bin/revision_runner diff --git a/bin/revision_runner b/bin/revision_runner old mode 100644 new mode 100755 index 0d8a3804f..ad3b0a3ac --- a/bin/revision_runner +++ b/bin/revision_runner @@ -22,13 +22,13 @@ require 'json' class RevisionRunner BadRevisionError = Class.new(StandardError) - ROOT = Pathname File.join(File.dirname(__FILE__), '..') + ROOT = Pathname File.expand_path(['..', '..'].join(File::Separator), __FILE__) TMP_DIR_BASE = File.join('tmp', 'revision_runner') TMP_DIR = File.join(ROOT, TMP_DIR_BASE) attr_accessor :url_base - def initialize(prepend: '', append: '') + def initialize self.url_base = "http://localhost:9292" refresh_temp_dir end From 4c98387049a54e8ab8a5e28fe37b3295ef1805d6 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 19 Jan 2016 00:34:07 -0500 Subject: [PATCH 04/36] wip --- bin/revision_runner | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/bin/revision_runner b/bin/revision_runner index ad3b0a3ac..5bcaa9721 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -33,12 +33,16 @@ class RevisionRunner refresh_temp_dir end - def temp_dir_empty? + def refresh_temp_dir + empty_temp_dir + fill_temp_dir + end + + def temp_dir_empty? Dir[File.join(TMP_DIR, '*')].none? end def empty_temp_dir - return if temp_dir_empty? FileUtils.mkdir_p(TMP_DIR) Dir[File.join(TMP_DIR, '*')].each do |file| if File.directory?(file) @@ -58,10 +62,6 @@ class RevisionRunner at_exit { empty_temp_dir } end - def refresh_temp_dir - empty_temp_dir - fill_temp_dir - end module GitCommands module_function @@ -92,6 +92,7 @@ class RevisionRunner system('git reset --hard --quiet') end end + include GitCommands def run_revisions(ref1: nil, ref2: nil) ref0 = current_branch @@ -120,9 +121,11 @@ class RevisionRunner checkout_ref(ref) bundle restart_server + run_cmd end def run_cmd + debug { 'running' } assert_responses(get_caching, get_non_caching) rescue BadRevisionError => e debug { e.message } @@ -194,7 +197,7 @@ class RevisionRunner end def debug(msg = '') - if block_given? && $DEBUG + if block_given? && ENV['DEBUG'] =~ /\Atrue|on|0\z/i STDOUT.puts yield else STDOUT.puts msg From afc5eb923385ce7cfefc751e5b9550224a535677 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 19 Jan 2016 00:40:05 -0500 Subject: [PATCH 05/36] wip --- test/dummy/app.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/dummy/app.rb b/test/dummy/app.rb index 2704d1949..665ae4b85 100644 --- a/test/dummy/app.rb +++ b/test/dummy/app.rb @@ -13,6 +13,7 @@ # ref: https://gist.github.com/bf4/8744473 class DummyApp < Rails::Application + # CONFIG: CACHE_ON={on,off} config.action_controller.perform_caching = ENV['CACHE_ON'] != 'off' config.action_controller.cache_store = ActiveSupport::Cache.lookup_store(:memory_store) From 8956749453927a40acd57d1207762181bdd15d01 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 1 Feb 2016 23:24:11 -0600 Subject: [PATCH 06/36] Add benchmarking runner from ruby-bench-suite --- bin/bench | 95 ++++++++++++++++++++++++++++++ test/dummy/app.rb | 31 ++++++---- test/dummy/benchmarking_support.rb | 68 +++++++++++++++++++++ test/dummy/bm_caching.rb | 16 +++++ test/dummy/controllers.rb | 4 +- 5 files changed, 200 insertions(+), 14 deletions(-) create mode 100755 bin/bench create mode 100644 test/dummy/benchmarking_support.rb create mode 100644 test/dummy/bm_caching.rb diff --git a/bin/bench b/bin/bench new file mode 100755 index 000000000..547e60568 --- /dev/null +++ b/bin/bench @@ -0,0 +1,95 @@ +#!/usr/bin/env ruby +# ActiveModelSerializers Benchmark driver + +require 'json' +require 'pathname' +require 'optparse' +require 'digest' +require 'pathname' + +class BenchmarkDriver + ROOT = Pathname File.expand_path(File.join('..', '..'), __FILE__) + BASE = ROOT.join('test', 'dummy') + + def self.benchmark(options) + self.new(options).run + end + + def initialize(options) + @repeat_count = options[:repeat_count] + @pattern = options[:pattern] + end + + def run + files.each do |path| + next if !@pattern.empty? && /#{@pattern.join('|')}/ !~ File.basename(path) + run_single(path) + end + end + + private + + def files + Dir[BASE.join('bm_*')] + end + + def run_single(path) + script = "RAILS_ENV=production ruby #{path}" + + # FIXME: ` provides the full output but it'll return failed output as well. + results = measure(script) + + # TODO: + # "vs. earliest ref (#{first_entry_ref[0..8]}) (x faster)", + # "caching speedup (on / off)" + commit_hash = ENV['COMMIT_HASH'] || `git rev-parse --short HEAD`.chomp + + environment = `ruby -v`.chomp + results.each do |json| + result = { + 'name' => json['label'], + 'time[user]' => Float(json['user']).round(5), + 'commit_hash' => commit_hash, + 'version' => json['version'], + "total_allocated_objects_per_iteration" => json["total_allocated_objects_per_iteration"], + 'environment' => environment + } + puts result + end + end + + def measure(script) + results = [] + + @repeat_count.times do + output = `#{script}` + result = output.split("\n").map {|line| + JSON.parse(line) + } + results << result + end + + results.sort_by do |result| + result.first['iterations_per_second'] + end.last + end +end + +options = { + repeat_count: 1, + pattern: [] +} + +OptionParser.new do |opts| + opts.banner = "Usage: bin/bench [options]" + + opts.on("-r", "--repeat-count [NUM]", "Run benchmarks [NUM] times taking the best result") do |value| + options[:repeat_count] = value.to_i + end + + opts.on("-p", "--pattern ", "Benchmark name pattern") do |value| + options[:pattern] = value.split(',') + end +end.parse!(ARGV) + +BenchmarkDriver.benchmark(options) diff --git a/test/dummy/app.rb b/test/dummy/app.rb index 665ae4b85..1f0a5a016 100644 --- a/test/dummy/app.rb +++ b/test/dummy/app.rb @@ -11,6 +11,24 @@ require 'action_controller/railtie' abort "Rails application already defined: #{Rails.application.class}" if Rails.application +class NullLogger < Logger + def initialize(*args) + end + + def add(*args, &block) + end +end +class DummyLogger < ActiveSupport::Logger + def initialize + @file = StringIO.new + super(@file) + end + + def messages + @file.rewind + @file.read + end +end # ref: https://gist.github.com/bf4/8744473 class DummyApp < Rails::Application # CONFIG: CACHE_ON={on,off} @@ -24,19 +42,8 @@ class DummyApp < Rails::Application config.active_support.test_order = :random config.secret_token = '1234' config.secret_key_base = 'abc123' - config.logger = Logger.new(IO::NULL) + config.logger = NullLogger.new - class DummyLogger < ActiveSupport::Logger - def initialize - @file = StringIO.new - super(@file) - end - - def messages - @file.rewind - @file.read - end - end end require 'active_model_serializers' diff --git a/test/dummy/benchmarking_support.rb b/test/dummy/benchmarking_support.rb new file mode 100644 index 000000000..08b29fa9f --- /dev/null +++ b/test/dummy/benchmarking_support.rb @@ -0,0 +1,68 @@ +require 'benchmark' +require 'json' + +module Benchmark + module ActiveModelSerializers + module TestMethods + def request(method, path) + response = Rack::MockRequest.new(DummyApp).send(method, path) + if response.status.in?([404, 500]) + fail "omg, #{method}, #{path}, #{response.status}, #{response.body}" + end + response + end + end + def ams(label=nil, time: 1_000, warmup: 10, &block) + unless block_given? + raise ArgumentError.new, "block should be passed" + end + + # run_gc + GC.enable + GC.start + GC.disable + warmup.times do + yield + end + measurement = Benchmark.measure do + time.times do + yield + end + end + + user = measurement.utime + system = measurement.stime + total = measurement.total + real = measurement.real + output = { + label: label, + real: real, + total: total, + user: user, + system: system, + version: ::ActiveModel::Serializer::VERSION, + total_allocated_objects_per_iteration: get_total_allocated_objects(&block) + }.to_json + + puts output + end + + def get_total_allocated_objects + if block_given? + key = + if RUBY_VERSION < '2.2' + :total_allocated_object + else + :total_allocated_objects + end + + before = GC.stat[key] + yield + after = GC.stat[key] + after - before + end + end + end + + extend Benchmark::ActiveModelSerializers +end diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb new file mode 100644 index 000000000..bcf00330b --- /dev/null +++ b/test/dummy/bm_caching.rb @@ -0,0 +1,16 @@ +require_relative './benchmarking_support' +require_relative './app' +include Benchmark::ActiveModelSerializers::TestMethods + +Benchmark.ams("caching on: caching serializers") do + request(:get, "/caching/on") +end +Benchmark.ams("caching off: caching serializers") do + request(:get, "/caching/off") +end +Benchmark.ams("caching on: non-caching serializers") do + request(:get, "/caching/on") +end +Benchmark.ams("caching off: non-caching serializers") do + request(:get, "/caching/off") +end diff --git a/test/dummy/controllers.rb b/test/dummy/controllers.rb index 8bcadeecc..a8e1423dd 100644 --- a/test/dummy/controllers.rb +++ b/test/dummy/controllers.rb @@ -28,7 +28,7 @@ def render_cache_status def clear ActionController::Base.cache_store.clear # Test caching is on - logger = DummyApp::DummyLogger.new + logger = DummyLogger.new ActiveSupport::Cache::Store.logger = logger # seems to be the best way # the below is used in some rails tests but isn't available/working in all versions, so far as I can tell # https://github.com/rails/rails/pull/15943 @@ -49,7 +49,7 @@ def cache_status end def cache_messages - ActiveSupport::Cache::Store.logger.is_a?(DummyApp::DummyLogger) && ActiveSupport::Cache::Store.logger.messages.split("\n") + ActiveSupport::Cache::Store.logger.is_a?(DummyLogger) && ActiveSupport::Cache::Store.logger.messages.split("\n") end def toggle_cache_status From c8d5fb53f75f3e77105845e8070f296426cb69dd Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 1 Feb 2016 23:58:00 -0600 Subject: [PATCH 07/36] cleanup --- bin/bench | 10 ++-- bin/revision_runner | 103 ++++++++------------------------------- test/dummy/bm_caching.rb | 79 ++++++++++++++++++++++++++++++ test/dummy/fixtures.rb | 2 +- 4 files changed, 108 insertions(+), 86 deletions(-) diff --git a/bin/bench b/bin/bench index 547e60568..9a5338516 100755 --- a/bin/bench +++ b/bin/bench @@ -48,12 +48,16 @@ class BenchmarkDriver results.each do |json| result = { 'name' => json['label'], - 'time[user]' => Float(json['user']).round(5), 'commit_hash' => commit_hash, 'version' => json['version'], - "total_allocated_objects_per_iteration" => json["total_allocated_objects_per_iteration"], + "total_allocated_objects_per_measurement" => json["total_allocated_objects_per_measurement"], 'environment' => environment } + if json['error'] + result['error'] = json['error'] + else + result['time[user]'] = Float(json['user']).round(5) + end puts result end end @@ -70,7 +74,7 @@ class BenchmarkDriver end results.sort_by do |result| - result.first['iterations_per_second'] + result.first['time[user]'] end.last end end diff --git a/bin/revision_runner b/bin/revision_runner index 5bcaa9721..020771f53 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -8,7 +8,7 @@ require 'json' ############################ # -# A wrapper around git-bisect that stashes some code (bin/serve_dummy) for use in making requests +# A wrapper around git-bisect that stashes some code (test/dummy/*) for use in making requests # running against each revision. # # USAGE @@ -21,7 +21,6 @@ require 'json' ########################### class RevisionRunner - BadRevisionError = Class.new(StandardError) ROOT = Pathname File.expand_path(['..', '..'].join(File::Separator), __FILE__) TMP_DIR_BASE = File.join('tmp', 'revision_runner') TMP_DIR = File.join(ROOT, TMP_DIR_BASE) @@ -29,7 +28,6 @@ class RevisionRunner attr_accessor :url_base def initialize - self.url_base = "http://localhost:9292" refresh_temp_dir end @@ -57,8 +55,9 @@ class RevisionRunner Dir[File.join(ROOT, 'test', 'dummy', '*.{rb,ru}')].each do |file| FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) end - file = File.join('bin', 'serve_dummy') - FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) + # TODO: useful? + # file = File.join('bin', 'serve_dummy') + # FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) at_exit { empty_temp_dir } end @@ -94,7 +93,7 @@ class RevisionRunner end include GitCommands - def run_revisions(ref1: nil, ref2: nil) + def run_revisions(ref1: nil, ref2: nil, cmd:) ref0 = current_branch ref1 ||= current_branch ref2 ||= 'master' @@ -103,7 +102,7 @@ class RevisionRunner revisions(ref1, ref2).each do |rev| STDERR.puts "Checking out: #{revision_description(rev)}" - reports[rev] = run_at_ref(rev) + reports[rev] = run_at_ref(rev, cmd) clean_head end checkout_ref(ref0) @@ -117,84 +116,23 @@ class RevisionRunner return reports end - def run_at_ref(ref) + def run_at_ref(ref, cmd) checkout_ref(ref) bundle - restart_server - run_cmd + system(cmd) end - def run_cmd - debug { 'running' } - assert_responses(get_caching, get_non_caching) - rescue BadRevisionError => e - debug { e.message } - exit 1 - end - - def get_caching - get(url_base + "/caching/on") - end - - def get_non_caching - get(url_base + "/non_caching/on") - end - - def assert_responses(caching, non_caching) - assert_equal(caching[:code], '200', "Caching response failed: #{caching}") - assert_equal(caching[:body], expected, "Caching response format failed: \n+ #{caching[:body]}\n- #{expected}") - assert_equal(caching[:content_type], 'application/json', "Caching response content type failed: \n+ #{caching[:content_type]}\n- application/json") - assert_equal(non_caching[:code], '200', "Non caching response failed: #{non_caching}") - assert_equal(non_caching[:body], expected, "Non Caching response format failed: \n+ #{non_caching[:body]}\n- #{expected}") - assert_equal(non_caching[:content_type], 'application/json', "Non caching response content type failed: \n+ #{non_caching[:content_type]}\n- application/json") - end - - def get(url) - uri = URI(url) - response = Net::HTTP.get_response(uri) - { code: response.code, body: JSON.load(response.body), content_type: response.content_type } - end - - def expected - @expected ||= - { - "post" => { - "id" => 1, - "title" => "New Post", - "body" => "Body", - "comments" => [ - { - "id" => 1, - "body" => "ZOMG A COMMENT" - } - ], - "blog" => { - "id" => 999, - "name" => "Custom blog" - }, - "author" => { - "id" => 1, - "name" => "Joao Moura." - } - } - } - end - - def assert_equal(expected, actual, message) - return true if expected == actual - fail BadRevisionError, messsage - end - - def restart_server - server_script = File.join(TMP_DIR_BASE, 'serve_dummy') - system("#{server_script} stop") - at_exit { system("#{server_script} stop") } - config_ru = Shellwords.shellescape(File.join(TMP_DIR_BASE, 'config.ru')) - pid = `CONFIG_RU=#{config_ru} #{server_script} start`.chomp - abort "No pid" if pid.empty? - pid = Integer(pid) - Process.kill(0, pid) # confirm running, else it raises - end + # TODO: useful? + # def restart_server + # server_script = File.join(TMP_DIR_BASE, 'serve_dummy') + # system("#{server_script} stop") + # at_exit { system("#{server_script} stop") } + # config_ru = Shellwords.shellescape(File.join(TMP_DIR_BASE, 'config.ru')) + # pid = `CONFIG_RU=#{config_ru} #{server_script} start`.chomp + # abort "No pid" if pid.empty? + # pid = Integer(pid) + # Process.kill(0, pid) # confirm running, else it raises + # end def debug(msg = '') if block_given? && ENV['DEBUG'] =~ /\Atrue|on|0\z/i @@ -208,7 +146,8 @@ end if $PROGRAM_NAME == __FILE__ runner = RevisionRunner.new - reports = runner.run_revisions(ref1: ARGV[0], ref2: ARGV[1]) + args = ARGV.dup + reports = runner.run_revisions(ref1: args.shift, ref2: args.shift, cmd: args) reports.each do |name, value| STDERR.puts "#{name}\n\t#{value}" end diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb index bcf00330b..6f7b7273e 100644 --- a/test/dummy/bm_caching.rb +++ b/test/dummy/bm_caching.rb @@ -2,6 +2,85 @@ require_relative './app' include Benchmark::ActiveModelSerializers::TestMethods +class ApiAssertion + BadRevisionError = Class.new(StandardError) + + def valid? + caching = get_caching + caching[:body].delete('meta') + non_caching = get_non_caching + non_caching[:body].delete('meta') + assert_responses(caching, non_caching) + rescue BadRevisionError => e + msg = e.message + STDOUT.puts msg.to_json + exit 1 + end + + private + + def get_caching + get("/caching/on") + end + + def get_non_caching + get("/non_caching/on") + end + + def assert_responses(caching, non_caching) + assert_equal(caching[:code], 200, "Caching response failed: #{caching}") + assert_equal(caching[:body], expected, "Caching response format failed: \n+ #{caching[:body]}\n- #{expected}") + assert_equal(caching[:content_type], 'application/json; charset=utf-8', "Caching response content type failed: \n+ #{caching[:content_type]}\n- application/json") + assert_equal(non_caching[:code], 200, "Non caching response failed: #{non_caching}") + assert_equal(non_caching[:body], expected, "Non Caching response format failed: \n+ #{non_caching[:body]}\n- #{expected}") + assert_equal(non_caching[:content_type], 'application/json; charset=utf-8', "Non caching response content type failed: \n+ #{non_caching[:content_type]}\n- application/json") + end + + def get(url) + response = request(:get, url) + { code: response.status, body: JSON.load(response.body), content_type: response.content_type } + end + + def expected + @expected ||= + { + "post" => { + "id" => 1, + "title" => "New Post", + "body" => "Body", + "comments" => [ + { + "id" => 1, + "body" => "ZOMG A COMMENT" + } + ], + "blog" => { + "id" => 999, + "name" => "Custom blog" + }, + "author" => { + "id" => 1, + "name" => "Joao Moura." + } + } + } + end + + def assert_equal(expected, actual, message) + return true if expected == actual + fail BadRevisionError, message + end + + def debug(msg = '') + if block_given? && ENV['DEBUG'] =~ /\Atrue|on|0\z/i + STDOUT.puts yield + else + STDOUT.puts msg + end + end +end +ApiAssertion.new.valid? + Benchmark.ams("caching on: caching serializers") do request(:get, "/caching/on") end diff --git a/test/dummy/fixtures.rb b/test/dummy/fixtures.rb index bcd248382..f0795e183 100644 --- a/test/dummy/fixtures.rb +++ b/test/dummy/fixtures.rb @@ -34,7 +34,7 @@ def blog Rails.configuration.serializers << PostSerializer class CachingAuthorSerializer < AuthorSerializer - cache key: 'writer', only: [:body], skip_digest: true + cache key: 'writer', only: [:name], skip_digest: true end Rails.configuration.serializers << CachingAuthorSerializer From 8a652a3339942457eb4e8e967ad951073d4297cc Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:06:13 -0600 Subject: [PATCH 08/36] bench --- bin/revision_runner | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bin/revision_runner b/bin/revision_runner index 020771f53..54e416cfc 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -55,9 +55,8 @@ class RevisionRunner Dir[File.join(ROOT, 'test', 'dummy', '*.{rb,ru}')].each do |file| FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) end - # TODO: useful? - # file = File.join('bin', 'serve_dummy') - # FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) + file = File.join('bin', 'bench') + FileUtils.cp(file, File.join(TMP_DIR, File.basename(file))) at_exit { empty_temp_dir } end From f2e58308d96d481478a9400a9d503284943a0b8b Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:14:01 -0600 Subject: [PATCH 09/36] Dynamic benchmark base --- bin/bench | 2 +- bin/revision_runner | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/bench b/bin/bench index 9a5338516..56d966ab8 100755 --- a/bin/bench +++ b/bin/bench @@ -9,7 +9,7 @@ require 'pathname' class BenchmarkDriver ROOT = Pathname File.expand_path(File.join('..', '..'), __FILE__) - BASE = ROOT.join('test', 'dummy') + BASE = ENV.fetch('BASE') { ROOT.join('test', 'dummy') } def self.benchmark(options) self.new(options).run diff --git a/bin/revision_runner b/bin/revision_runner index 54e416cfc..5a8c0f6da 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -118,7 +118,7 @@ class RevisionRunner def run_at_ref(ref, cmd) checkout_ref(ref) bundle - system(cmd) + system("BASE=#{TMP_DIR_BASE}", cmd) end # TODO: useful? From 9af1bcab47d4e21fa0f32da17e8f208dc1e01cec Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:17:15 -0600 Subject: [PATCH 10/36] base --- bin/bench | 2 +- bin/revision_runner | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bin/bench b/bin/bench index 56d966ab8..ec639a773 100755 --- a/bin/bench +++ b/bin/bench @@ -30,7 +30,7 @@ class BenchmarkDriver private def files - Dir[BASE.join('bm_*')] + Dir[File.join(BASE, 'bm_*')] end def run_single(path) diff --git a/bin/revision_runner b/bin/revision_runner index 5a8c0f6da..7faa6b38f 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -118,7 +118,8 @@ class RevisionRunner def run_at_ref(ref, cmd) checkout_ref(ref) bundle - system("BASE=#{TMP_DIR_BASE}", cmd) + base = Shellwords.shellescape(TMP_DIR_BASE) + `BASE=#{base} #{Shellwords.shelljoin(cmd)}` end # TODO: useful? @@ -148,6 +149,6 @@ if $PROGRAM_NAME == __FILE__ args = ARGV.dup reports = runner.run_revisions(ref1: args.shift, ref2: args.shift, cmd: args) reports.each do |name, value| - STDERR.puts "#{name}\n\t#{value}" + STDERR.puts "report #{name}\n\t#{value}" end end From c1a947e1c4e88820f7f8c9e985c959530ec3670a Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:22:22 -0600 Subject: [PATCH 11/36] wip --- bin/revision_runner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/revision_runner b/bin/revision_runner index 7faa6b38f..04bc87f41 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -149,6 +149,6 @@ if $PROGRAM_NAME == __FILE__ args = ARGV.dup reports = runner.run_revisions(ref1: args.shift, ref2: args.shift, cmd: args) reports.each do |name, value| - STDERR.puts "report #{name}\n\t#{value}" + STDERR.puts "revision: #{name}\n\t#{value}" end end From e2204ebd33b4474f5ffedee7e0f195283fe42dac Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:24:47 -0600 Subject: [PATCH 12/36] wip --- bin/revision_runner | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/revision_runner b/bin/revision_runner index 04bc87f41..2e312cd10 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -119,7 +119,9 @@ class RevisionRunner checkout_ref(ref) bundle base = Shellwords.shellescape(TMP_DIR_BASE) - `BASE=#{base} #{Shellwords.shelljoin(cmd)}` + cmd = "BASE=#{base} #{Shellwords.shelljoin(cmd)}" + debug { cmd } + `#{cmd}` end # TODO: useful? From ab0e4972140c56e5f71123ea3f37b2567fd8cf89 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:26:28 -0600 Subject: [PATCH 13/36] wip --- bin/revision_runner | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/revision_runner b/bin/revision_runner index 2e312cd10..ee0efc428 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -121,7 +121,8 @@ class RevisionRunner base = Shellwords.shellescape(TMP_DIR_BASE) cmd = "BASE=#{base} #{Shellwords.shelljoin(cmd)}" debug { cmd } - `#{cmd}` + system(cmd) + $CHILD_STATUS.exitstatus end # TODO: useful? From 96d9eb23ec51e39b8641d92f95850fab426cb24e Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:28:49 -0600 Subject: [PATCH 14/36] wip --- bin/revision_runner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/revision_runner b/bin/revision_runner index ee0efc428..67426b212 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -79,7 +79,7 @@ class RevisionRunner end def revision_description(rev) - `git log --oneline -1 #{rev}` + `git log --oneline -1 #{rev}`.chomp end def bundle From 8653683abad88d50a513c2b7cdbf87502f9426e5 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:30:12 -0600 Subject: [PATCH 15/36] wip --- bin/bench | 1 + test/dummy/benchmarking_support.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/bench b/bin/bench index ec639a773..89dd27c43 100755 --- a/bin/bench +++ b/bin/bench @@ -46,6 +46,7 @@ class BenchmarkDriver environment = `ruby -v`.chomp results.each do |json| + puts json.keys result = { 'name' => json['label'], 'commit_hash' => commit_hash, diff --git a/test/dummy/benchmarking_support.rb b/test/dummy/benchmarking_support.rb index 08b29fa9f..0b5583540 100644 --- a/test/dummy/benchmarking_support.rb +++ b/test/dummy/benchmarking_support.rb @@ -41,7 +41,7 @@ def ams(label=nil, time: 1_000, warmup: 10, &block) user: user, system: system, version: ::ActiveModel::Serializer::VERSION, - total_allocated_objects_per_iteration: get_total_allocated_objects(&block) + total_allocated_objects_per_measurement: get_total_allocated_objects(&block) }.to_json puts output From 895e5869122856469288eca3175b568406c58a1a Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:31:10 -0600 Subject: [PATCH 16/36] wip --- bin/bench | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/bench b/bin/bench index 89dd27c43..6f5bf0aff 100755 --- a/bin/bench +++ b/bin/bench @@ -44,7 +44,7 @@ class BenchmarkDriver # "caching speedup (on / off)" commit_hash = ENV['COMMIT_HASH'] || `git rev-parse --short HEAD`.chomp - environment = `ruby -v`.chomp + environment = `ruby -v`.strip.chomp[/\W+/] results.each do |json| puts json.keys result = { From 6bd0f099b5113197941cfe4ea827397e64979c9b Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:32:14 -0600 Subject: [PATCH 17/36] wip --- bin/bench | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/bench b/bin/bench index 6f5bf0aff..cd2a34dfa 100755 --- a/bin/bench +++ b/bin/bench @@ -45,8 +45,8 @@ class BenchmarkDriver commit_hash = ENV['COMMIT_HASH'] || `git rev-parse --short HEAD`.chomp environment = `ruby -v`.strip.chomp[/\W+/] + puts "Benchmark results:" results.each do |json| - puts json.keys result = { 'name' => json['label'], 'commit_hash' => commit_hash, From edcdfe8e9f57b36c234ce28b63f5721ccf3ef3bf Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:33:10 -0600 Subject: [PATCH 18/36] wip --- bin/revision_runner | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/revision_runner b/bin/revision_runner index 67426b212..d9d4cec58 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -120,6 +120,7 @@ class RevisionRunner bundle base = Shellwords.shellescape(TMP_DIR_BASE) cmd = "BASE=#{base} #{Shellwords.shelljoin(cmd)}" + cmd.sub('bin/bench', 'tmp/revision_runner/bench') debug { cmd } system(cmd) $CHILD_STATUS.exitstatus From 4a4876e2486a921d46648156c586d1103d87d9a5 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:39:20 -0600 Subject: [PATCH 19/36] wip --- bin/bench | 12 ++++++++++-- bin/revision_runner | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/bin/bench b/bin/bench index cd2a34dfa..8eb953e6d 100755 --- a/bin/bench +++ b/bin/bench @@ -18,6 +18,7 @@ class BenchmarkDriver def initialize(options) @repeat_count = options[:repeat_count] @pattern = options[:pattern] + @env = options[:env].join(' ') end def run @@ -34,7 +35,7 @@ class BenchmarkDriver end def run_single(path) - script = "RAILS_ENV=production ruby #{path}" + script = "RAILS_ENV=production #{@env} ruby #{path}" # FIXME: ` provides the full output but it'll return failed output as well. results = measure(script) @@ -82,7 +83,10 @@ end options = { repeat_count: 1, - pattern: [] + pattern: [], + env: { + 'CACHE_ON' => 'on' + } } OptionParser.new do |opts| @@ -95,6 +99,10 @@ OptionParser.new do |opts| opts.on("-p", "--pattern ", "Benchmark name pattern") do |value| options[:pattern] = value.split(',') end + + opts.on("-e", "--env ", "ENV variables to pass in") do |value| + options[:env] = value.split(',') + end end.parse!(ARGV) BenchmarkDriver.benchmark(options) diff --git a/bin/revision_runner b/bin/revision_runner index d9d4cec58..b8f4b7de0 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -119,7 +119,7 @@ class RevisionRunner checkout_ref(ref) bundle base = Shellwords.shellescape(TMP_DIR_BASE) - cmd = "BASE=#{base} #{Shellwords.shelljoin(cmd)}" + cmd = "COMMIT_HASH=#{ref} BASE=#{base} #{Shellwords.shelljoin(cmd)}" cmd.sub('bin/bench', 'tmp/revision_runner/bench') debug { cmd } system(cmd) From cf2edfee6120ac0d13b10f7cf5a798ba79c1dd98 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:39:38 -0600 Subject: [PATCH 20/36] wip --- test/dummy/bm_caching.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb index 6f7b7273e..2ef003472 100644 --- a/test/dummy/bm_caching.rb +++ b/test/dummy/bm_caching.rb @@ -7,9 +7,9 @@ class ApiAssertion def valid? caching = get_caching - caching[:body].delete('meta') + STDERR.puts caching[:body].delete('meta') non_caching = get_non_caching - non_caching[:body].delete('meta') + STDERR.puts non_caching[:body].delete('meta') assert_responses(caching, non_caching) rescue BadRevisionError => e msg = e.message From 4b8520d846be6a2f10769e55acc82f18f15d2f51 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:40:23 -0600 Subject: [PATCH 21/36] wip --- bin/bench | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/bench b/bin/bench index 8eb953e6d..08073a622 100755 --- a/bin/bench +++ b/bin/bench @@ -36,6 +36,8 @@ class BenchmarkDriver def run_single(path) script = "RAILS_ENV=production #{@env} ruby #{path}" + puts script + exit # FIXME: ` provides the full output but it'll return failed output as well. results = measure(script) From 8ba629a4b1e8702d651dcf873f71c8bd1e029c70 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:43:48 -0600 Subject: [PATCH 22/36] wip --- test/dummy/bm_caching.rb | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb index 2ef003472..f815ad89b 100644 --- a/test/dummy/bm_caching.rb +++ b/test/dummy/bm_caching.rb @@ -17,14 +17,22 @@ def valid? exit 1 end + def get_status(on_off = 'on'.freeze) + get("/status/#{on_off}" + end + + def clear + get("/clear") + end + private - def get_caching - get("/caching/on") + def get_caching(on_off = 'on'.freeze) + get("/caching/#{on_off}") end - def get_non_caching - get("/non_caching/on") + def get_non_caching(on_off = 'on'.freeze) + get("/non_caching/#{on_off}") end def assert_responses(caching, non_caching) @@ -79,17 +87,26 @@ def debug(msg = '') end end end -ApiAssertion.new.valid? +assertion = ApiAssertion.new +assertion.valid? +STDERR.puts assertion.get_status Benchmark.ams("caching on: caching serializers") do request(:get, "/caching/on") end +STDERR.puts assertion.get_status +assertion.clear Benchmark.ams("caching off: caching serializers") do request(:get, "/caching/off") end +STDERR.puts assertion.get_status +assertion.clear Benchmark.ams("caching on: non-caching serializers") do request(:get, "/caching/on") end +STDERR.puts assertion.get_status +assertion.clear Benchmark.ams("caching off: non-caching serializers") do request(:get, "/caching/off") end +STDERR.puts assertion.get_status From 89cdc8edb6a8ca2e885a04296f6fb3a5ff2f32a1 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:43:59 -0600 Subject: [PATCH 23/36] wip --- bin/bench | 2 -- 1 file changed, 2 deletions(-) diff --git a/bin/bench b/bin/bench index 08073a622..8eb953e6d 100755 --- a/bin/bench +++ b/bin/bench @@ -36,8 +36,6 @@ class BenchmarkDriver def run_single(path) script = "RAILS_ENV=production #{@env} ruby #{path}" - puts script - exit # FIXME: ` provides the full output but it'll return failed output as well. results = measure(script) From 6c5220b1456d2d98b8ec586bdb28dff6b3cb9106 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:44:15 -0600 Subject: [PATCH 24/36] wip --- test/dummy/bm_caching.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb index f815ad89b..9d731360f 100644 --- a/test/dummy/bm_caching.rb +++ b/test/dummy/bm_caching.rb @@ -18,7 +18,7 @@ def valid? end def get_status(on_off = 'on'.freeze) - get("/status/#{on_off}" + get("/status/#{on_off}") end def clear From f3072531b0fb4a6246ef51f98a2086cafc3f8fe1 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:46:48 -0600 Subject: [PATCH 25/36] wip --- test/dummy/bm_caching.rb | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb index 9d731360f..02765a7b4 100644 --- a/test/dummy/bm_caching.rb +++ b/test/dummy/bm_caching.rb @@ -1,15 +1,15 @@ require_relative './benchmarking_support' require_relative './app' -include Benchmark::ActiveModelSerializers::TestMethods class ApiAssertion + include Benchmark::ActiveModelSerializers::TestMethods BadRevisionError = Class.new(StandardError) def valid? caching = get_caching - STDERR.puts caching[:body].delete('meta') + caching[:body].delete('meta') non_caching = get_non_caching - STDERR.puts non_caching[:body].delete('meta') + caching[:body].delete('meta') assert_responses(caching, non_caching) rescue BadRevisionError => e msg = e.message @@ -25,8 +25,6 @@ def clear get("/clear") end - private - def get_caching(on_off = 'on'.freeze) get("/caching/#{on_off}") end @@ -35,6 +33,8 @@ def get_non_caching(on_off = 'on'.freeze) get("/non_caching/#{on_off}") end + private + def assert_responses(caching, non_caching) assert_equal(caching[:code], 200, "Caching response failed: #{caching}") assert_equal(caching[:body], expected, "Caching response format failed: \n+ #{caching[:body]}\n- #{expected}") @@ -90,23 +90,23 @@ def debug(msg = '') assertion = ApiAssertion.new assertion.valid? -STDERR.puts assertion.get_status +# STDERR.puts assertion.get_status Benchmark.ams("caching on: caching serializers") do - request(:get, "/caching/on") + assertion.get_caching('on') end -STDERR.puts assertion.get_status +# STDERR.puts assertion.get_status assertion.clear Benchmark.ams("caching off: caching serializers") do - request(:get, "/caching/off") + assertion.get_caching('off') end -STDERR.puts assertion.get_status +# STDERR.puts assertion.get_status assertion.clear Benchmark.ams("caching on: non-caching serializers") do - request(:get, "/caching/on") + assertion.get_non_caching('on') end -STDERR.puts assertion.get_status +# STDERR.puts assertion.get_status assertion.clear Benchmark.ams("caching off: non-caching serializers") do - request(:get, "/caching/off") + assertion.get_non_caching('off') end -STDERR.puts assertion.get_status +# STDERR.puts assertion.get_status From 41481f29e64aea42e28c3b744380b2f4e825d8d5 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:47:38 -0600 Subject: [PATCH 26/36] wip --- bin/bench | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bin/bench b/bin/bench index 8eb953e6d..77bd5d32a 100755 --- a/bin/bench +++ b/bin/bench @@ -84,9 +84,7 @@ end options = { repeat_count: 1, pattern: [], - env: { - 'CACHE_ON' => 'on' - } + env: "CACHE_ON=on" } OptionParser.new do |opts| From 72875829b1a3edd67968bfeec722667690b67cf3 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:48:01 -0600 Subject: [PATCH 27/36] wip --- bin/bench | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/bench b/bin/bench index 77bd5d32a..2410853f3 100755 --- a/bin/bench +++ b/bin/bench @@ -18,7 +18,7 @@ class BenchmarkDriver def initialize(options) @repeat_count = options[:repeat_count] @pattern = options[:pattern] - @env = options[:env].join(' ') + @env = Array(options[:env]).join(' ') end def run From 43986fb3f5c8f4562a9bd3788674c6e8b61d3272 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:53:12 -0600 Subject: [PATCH 28/36] wip --- test/dummy/bm_caching.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb index 02765a7b4..4ad338094 100644 --- a/test/dummy/bm_caching.rb +++ b/test/dummy/bm_caching.rb @@ -9,11 +9,11 @@ def valid? caching = get_caching caching[:body].delete('meta') non_caching = get_non_caching - caching[:body].delete('meta') + non_caching[:body].delete('meta') assert_responses(caching, non_caching) rescue BadRevisionError => e msg = e.message - STDOUT.puts msg.to_json + STDOUT.puts msg exit 1 end From 013c116a9852ba559ebaad08e1d4de58c293abb6 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 00:56:02 -0600 Subject: [PATCH 29/36] wip --- bin/revision_runner | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bin/revision_runner b/bin/revision_runner index b8f4b7de0..abc660739 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -8,7 +8,7 @@ require 'json' ############################ # -# A wrapper around git-bisect that stashes some code (test/dummy/*) for use in making requests +# A wrapper around git-bisect that stashes some code (bin/bench,test/dummy/*) for use in making requests # running against each revision. # # USAGE @@ -17,7 +17,11 @@ require 'json' # defaults to the current branch # defaults to the master branch # Runs across every revisiion in the range, inclusive. -# Aborts when exit code is non-zero. +# TODO: Aborts when exit code is non-zero. +# +# EXAMPLE +# +# bin/revision_runner 792fb8a9053f8db3c562dae4f40907a582dd1720 master bin/bench -r 2 -e CACHE_ON=off ########################### class RevisionRunner From 7ad5e8f6ed556375fbee9d468e0b12f41463513a Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 01:01:18 -0600 Subject: [PATCH 30/36] wip --- test/dummy/benchmarking_support.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dummy/benchmarking_support.rb b/test/dummy/benchmarking_support.rb index 0b5583540..b26eb86b9 100644 --- a/test/dummy/benchmarking_support.rb +++ b/test/dummy/benchmarking_support.rb @@ -12,7 +12,7 @@ def request(method, path) response end end - def ams(label=nil, time: 1_000, warmup: 10, &block) + def ams(label=nil, time: 5_000, warmup: 10, &block) unless block_given? raise ArgumentError.new, "block should be passed" end From 28f78c0846d7e445a6786586d6c4b8fa45cf4221 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 01:04:11 -0600 Subject: [PATCH 31/36] rubocop --- test/dummy/app.rb | 5 ++--- test/dummy/benchmarking_support.rb | 33 +++++++++++++-------------- test/dummy/bm_caching.rb | 36 +++++++++++++++--------------- test/dummy/controllers.rb | 4 ++-- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/test/dummy/app.rb b/test/dummy/app.rb index 1f0a5a016..f19aedc18 100644 --- a/test/dummy/app.rb +++ b/test/dummy/app.rb @@ -12,10 +12,10 @@ abort "Rails application already defined: #{Rails.application.class}" if Rails.application class NullLogger < Logger - def initialize(*args) + def initialize(*_args) end - def add(*args, &block) + def add(*_args, &_block) end end class DummyLogger < ActiveSupport::Logger @@ -43,7 +43,6 @@ class DummyApp < Rails::Application config.secret_token = '1234' config.secret_key_base = 'abc123' config.logger = NullLogger.new - end require 'active_model_serializers' diff --git a/test/dummy/benchmarking_support.rb b/test/dummy/benchmarking_support.rb index b26eb86b9..e8527b7c1 100644 --- a/test/dummy/benchmarking_support.rb +++ b/test/dummy/benchmarking_support.rb @@ -12,10 +12,8 @@ def request(method, path) response end end - def ams(label=nil, time: 5_000, warmup: 10, &block) - unless block_given? - raise ArgumentError.new, "block should be passed" - end + def ams(label = nil, time: 5_000, warmup: 10, &block) + fail ArgumentError.new, 'block should be passed' unless block_given? # run_gc GC.enable @@ -41,26 +39,25 @@ def ams(label=nil, time: 5_000, warmup: 10, &block) user: user, system: system, version: ::ActiveModel::Serializer::VERSION, - total_allocated_objects_per_measurement: get_total_allocated_objects(&block) + total_allocated_objects_per_measurement: total_allocated_objects(&block) }.to_json puts output end - def get_total_allocated_objects - if block_given? - key = - if RUBY_VERSION < '2.2' - :total_allocated_object - else - :total_allocated_objects - end + def total_allocated_objects + return unless block_given? + key = + if RUBY_VERSION < '2.2' + :total_allocated_object + else + :total_allocated_objects + end - before = GC.stat[key] - yield - after = GC.stat[key] - after - before - end + before = GC.stat[key] + yield + after = GC.stat[key] + after - before end end diff --git a/test/dummy/bm_caching.rb b/test/dummy/bm_caching.rb index 4ad338094..2aa5a67a6 100644 --- a/test/dummy/bm_caching.rb +++ b/test/dummy/bm_caching.rb @@ -22,7 +22,7 @@ def get_status(on_off = 'on'.freeze) end def clear - get("/clear") + get('/clear') end def get_caching(on_off = 'on'.freeze) @@ -52,23 +52,23 @@ def get(url) def expected @expected ||= { - "post" => { - "id" => 1, - "title" => "New Post", - "body" => "Body", - "comments" => [ + 'post' => { + 'id' => 1, + 'title' => 'New Post', + 'body' => 'Body', + 'comments' => [ { - "id" => 1, - "body" => "ZOMG A COMMENT" + 'id' => 1, + 'body' => 'ZOMG A COMMENT' } ], - "blog" => { - "id" => 999, - "name" => "Custom blog" + 'blog' => { + 'id' => 999, + 'name' => 'Custom blog' }, - "author" => { - "id" => 1, - "name" => "Joao Moura." + 'author' => { + 'id' => 1, + 'name' => 'Joao Moura.' } } } @@ -91,22 +91,22 @@ def debug(msg = '') assertion.valid? # STDERR.puts assertion.get_status -Benchmark.ams("caching on: caching serializers") do +Benchmark.ams('caching on: caching serializers') do assertion.get_caching('on') end # STDERR.puts assertion.get_status assertion.clear -Benchmark.ams("caching off: caching serializers") do +Benchmark.ams('caching off: caching serializers') do assertion.get_caching('off') end # STDERR.puts assertion.get_status assertion.clear -Benchmark.ams("caching on: non-caching serializers") do +Benchmark.ams('caching on: non-caching serializers') do assertion.get_non_caching('on') end # STDERR.puts assertion.get_status assertion.clear -Benchmark.ams("caching off: non-caching serializers") do +Benchmark.ams('caching off: non-caching serializers') do assertion.get_non_caching('off') end # STDERR.puts assertion.get_status diff --git a/test/dummy/controllers.rb b/test/dummy/controllers.rb index a8e1423dd..663a35d7f 100644 --- a/test/dummy/controllers.rb +++ b/test/dummy/controllers.rb @@ -22,7 +22,7 @@ def render_cache_status # STDERR.puts cache_store.class # STDERR.puts cache_dependencies # ActiveSupport::Cache::Store.logger.debug [ActiveModelSerializers.config.cache_store, ActiveModelSerializers.config.perform_caching, CachingPostSerializer._cache, perform_caching, params].inspect - render json: { caching: perform_caching, meta: { cache_log: cache_messages, cache_status: cache_status } }.to_json + render json: { caching: perform_caching, meta: { cache_log: cache_messages, cache_status: cache_status } }.to_json end def clear @@ -44,7 +44,7 @@ def cache_status { controller: perform_caching, app: Rails.configuration.action_controller.perform_caching, - serializers: Rails.configuration.serializers.each_with_object({}) {|serializer, data| data[serializer.name] = serializer._cache.present? } + serializers: Rails.configuration.serializers.each_with_object({}) { |serializer, data| data[serializer.name] = serializer._cache.present? } } end From aceb5780545fc49d39eed4b69ebf37a0dfec13de Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 01:07:21 -0600 Subject: [PATCH 32/36] wip --- bin/bench | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/bench b/bin/bench index 2410853f3..89a8ffa75 100755 --- a/bin/bench +++ b/bin/bench @@ -45,7 +45,8 @@ class BenchmarkDriver # "caching speedup (on / off)" commit_hash = ENV['COMMIT_HASH'] || `git rev-parse --short HEAD`.chomp - environment = `ruby -v`.strip.chomp[/\W+/] + environment = `ruby -v`.chomp.strip[/\d+\.\d+\.\d+\w+/] + puts "Benchmark results:" results.each do |json| result = { From b03533de51f7b657214f7ed806fea7d1cf3a5def Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 01:35:30 -0600 Subject: [PATCH 33/36] turn off logger; possible performance issue --- test/dummy/controllers.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/dummy/controllers.rb b/test/dummy/controllers.rb index 663a35d7f..104f9c8be 100644 --- a/test/dummy/controllers.rb +++ b/test/dummy/controllers.rb @@ -28,7 +28,8 @@ def render_cache_status def clear ActionController::Base.cache_store.clear # Test caching is on - logger = DummyLogger.new + # logger = DummyLogger.new + logger = NullLogger.new ActiveSupport::Cache::Store.logger = logger # seems to be the best way # the below is used in some rails tests but isn't available/working in all versions, so far as I can tell # https://github.com/rails/rails/pull/15943 From 06c3a5d31493ff3cfad9f010d1b10c10e769a1a5 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 03:11:04 -0600 Subject: [PATCH 34/36] fix when NullLogger makes the cache blow up --- test/dummy/benchmarking_support.rb | 2 +- test/dummy/controllers.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/dummy/benchmarking_support.rb b/test/dummy/benchmarking_support.rb index e8527b7c1..2e5104ef6 100644 --- a/test/dummy/benchmarking_support.rb +++ b/test/dummy/benchmarking_support.rb @@ -7,7 +7,7 @@ module TestMethods def request(method, path) response = Rack::MockRequest.new(DummyApp).send(method, path) if response.status.in?([404, 500]) - fail "omg, #{method}, #{path}, #{response.status}, #{response.body}" + fail "omg, #{method}, #{path}, '#{response.status}', '#{response.body}'" end response end diff --git a/test/dummy/controllers.rb b/test/dummy/controllers.rb index 104f9c8be..59fe09156 100644 --- a/test/dummy/controllers.rb +++ b/test/dummy/controllers.rb @@ -29,7 +29,7 @@ def clear ActionController::Base.cache_store.clear # Test caching is on # logger = DummyLogger.new - logger = NullLogger.new + logger = Logger.new(IO::NULL) ActiveSupport::Cache::Store.logger = logger # seems to be the best way # the below is used in some rails tests but isn't available/working in all versions, so far as I can tell # https://github.com/rails/rails/pull/15943 From 9f252cf2162bc12f3622aef1e8880553e4bbb0b0 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 03:12:45 -0600 Subject: [PATCH 35/36] no set cache store logger --- test/dummy/benchmarking_support.rb | 2 +- test/dummy/controllers.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/test/dummy/benchmarking_support.rb b/test/dummy/benchmarking_support.rb index 2e5104ef6..998b83ac2 100644 --- a/test/dummy/benchmarking_support.rb +++ b/test/dummy/benchmarking_support.rb @@ -12,7 +12,7 @@ def request(method, path) response end end - def ams(label = nil, time: 5_000, warmup: 10, &block) + def ams(label = nil, time: 2_000, warmup: 10, &block) fail ArgumentError.new, 'block should be passed' unless block_given? # run_gc diff --git a/test/dummy/controllers.rb b/test/dummy/controllers.rb index 59fe09156..8b1fda8bf 100644 --- a/test/dummy/controllers.rb +++ b/test/dummy/controllers.rb @@ -29,8 +29,7 @@ def clear ActionController::Base.cache_store.clear # Test caching is on # logger = DummyLogger.new - logger = Logger.new(IO::NULL) - ActiveSupport::Cache::Store.logger = logger # seems to be the best way + # ActiveSupport::Cache::Store.logger = logger # seems to be the best way # the below is used in some rails tests but isn't available/working in all versions, so far as I can tell # https://github.com/rails/rails/pull/15943 # ActiveSupport::Notifications.subscribe(/^cache_(.*)\.active_support$/) do |*args| From af78396f7d55c8143a9b60cac1f26263d071f4d3 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 2 Feb 2016 08:19:24 -0600 Subject: [PATCH 36/36] oops --- bin/revision_runner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/revision_runner b/bin/revision_runner index abc660739..2fffafe34 100755 --- a/bin/revision_runner +++ b/bin/revision_runner @@ -78,7 +78,7 @@ class RevisionRunner end def checkout_ref(ref) - debug { `git checkout #{ref}`.chomp } + `git checkout #{ref}`.chomp abort "Checkout failed: #{ref}, #{$CHILD_STATUS.exitstatus}" unless $CHILD_STATUS.success? end