summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2015-10-05 12:56:52 +0000
committerYorick Peterse <yorickpeterse@gmail.com>2015-10-05 12:56:52 +0000
commitb6808f437df40ab1e2f60a028a6fa27f1cd4433e (patch)
tree2255f2a64c044202714d111cb523350e05cd1f33
parent3137d155caeeebc5de7ad8887456f64e86296729 (diff)
parent0bef64911b983f4fc1c130e9c5c7b730adb30072 (diff)
downloadgitlab-ce-b6808f437df40ab1e2f60a028a6fa27f1cd4433e.tar.gz
Merge branch 'benchmark-suite' into 'master'
Basic RSpec/benchmark-ips powered benchmark suite Corresponding issue: #2909, see the commit messages for more details. A few things to note: 1. The current use of `subject` isn't exactly easy on the eyes due to them having to return a Proc, I'm not sure yet how (and if) we can work around this. 2. The maximum amount of iterations in the current `User.by_login` benchmark is arbitrary, we might have to adjust it once said method's performance has been improved. 3. Benchmarks currently take 2 seconds to warm up and 5 seconds to run (benchmark-ips defaults). 4. The custom RSpec matcher file (`benchmark_matchers.rb`) is a bit messy, any feedback on this would be appreciated Any comments/feedback on this would be greatly appreciated. See merge request !1503
-rw-r--r--.gitlab-ci.yml8
-rw-r--r--doc/development/README.md1
-rw-r--r--doc/development/benchmarking.md69
-rw-r--r--lib/tasks/spec.rake11
-rw-r--r--spec/benchmarks/models/user_spec.rb40
-rw-r--r--spec/spec_helper.rb3
-rw-r--r--spec/support/matchers/benchmark_matchers.rb59
-rw-r--r--spec/support/setup_builds_storage.rb6
8 files changed, 193 insertions, 4 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ddf4e31204a..cf6d28b01af 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -24,6 +24,14 @@ spec:api:
- ruby
- mysql
+spec:benchmark:
+ script:
+ - RAILS_ENV=test bundle exec rake spec:benchmark
+ tags:
+ - ruby
+ - mysql
+ allow_failure: true
+
spec:other:
script:
- RAILS_ENV=test SIMPLECOV=true bundle exec rake spec:other
diff --git a/doc/development/README.md b/doc/development/README.md
index 6bc8e1888db..d5bf166ad32 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -8,3 +8,4 @@
- [UI guide](ui_guide.md) for building GitLab with existing css styles and elements
- [Migration Style Guide](migration_style_guide.md) for creating safe migrations
- [How to dump production data to staging](dump_db.md)
+- [Benchmarking](benchmarking.md)
diff --git a/doc/development/benchmarking.md b/doc/development/benchmarking.md
new file mode 100644
index 00000000000..88e18ee95f9
--- /dev/null
+++ b/doc/development/benchmarking.md
@@ -0,0 +1,69 @@
+# Benchmarking
+
+GitLab CE comes with a set of benchmarks that are executed for every build. This
+makes it easier to measure performance of certain components over time.
+
+Benchmarks are written as RSpec tests using a few extra helpers. To write a
+benchmark, first tag the top-level `describe`:
+
+```ruby
+describe MaruTheCat, benchmark: true do
+
+end
+```
+
+This ensures the benchmark is executed separately from other test collections.
+It also exposes the various RSpec matchers used for writing benchmarks to the
+test group.
+
+Next, lets write the actual benchmark:
+
+```ruby
+describe MaruTheCat, benchmark: true do
+ let(:maru) { MaruTheChat.new }
+
+ describe '#jump_in_box' do
+ benchmark_subject { maru.jump_in_box }
+
+ it { is_expected.to iterate_per_second(9000) }
+ end
+end
+```
+
+Here `benchmark_subject` is a small wrapper around RSpec's `subject` method that
+makes it easier to specify the subject of a benchmark. Using RSpec's regular
+`subject` would require us to write the following instead:
+
+```ruby
+subject { -> { maru.jump_in_box } }
+```
+
+The `iterate_per_second` matcher defines the amount of times per second a
+subject should be executed. The higher the amount of iterations the better.
+
+By default the allowed standard deviation is a maximum of 30%. This can be
+adjusted by chaining the `with_maximum_stddev` on the `iterate_per_second`
+matcher:
+
+```ruby
+it { is_expected.to iterate_per_second(9000).with_maximum_stddev(50) }
+```
+
+This can be useful if the code in question depends on external resources of
+which the performance can vary a lot (e.g. physical HDDs, network calls, etc).
+However, in most cases 30% should be enough so only change this when really
+needed.
+
+## Benchmarks Location
+
+Benchmarks should be stored in `spec/benchmarks` and should follow the regular
+Rails specs structure. That is, model benchmarks go in `spec/benchmark/models`,
+benchmarks for code in the `lib` directory go in `spec/benchmarks/lib`, etc.
+
+## Underlying Technology
+
+The benchmark setup uses [benchmark-ips][benchmark-ips] which takes care of the
+heavy lifting such as warming up code, calculating iterations, standard
+deviation, etc.
+
+[benchmark-ips]: https://github.com/evanphx/benchmark-ips
diff --git a/lib/tasks/spec.rake b/lib/tasks/spec.rake
index 831746815d7..3ae5c250694 100644
--- a/lib/tasks/spec.rake
+++ b/lib/tasks/spec.rake
@@ -19,11 +19,20 @@ namespace :spec do
run_commands(cmds)
end
+ desc 'GitLab | Rspec | Run benchmark specs'
+ task :benchmark do
+ cmds = [
+ %W(rake gitlab:setup),
+ %W(rspec spec --tag @benchmark)
+ ]
+ run_commands(cmds)
+ end
+
desc 'GitLab | Rspec | Run other specs'
task :other do
cmds = [
%W(rake gitlab:setup),
- %W(rspec spec --tag ~@api --tag ~@feature)
+ %W(rspec spec --tag ~@api --tag ~@feature --tag ~@benchmark)
]
run_commands(cmds)
end
diff --git a/spec/benchmarks/models/user_spec.rb b/spec/benchmarks/models/user_spec.rb
new file mode 100644
index 00000000000..168be20b7a5
--- /dev/null
+++ b/spec/benchmarks/models/user_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe User, benchmark: true do
+ describe '.by_login' do
+ before do
+ %w{Alice Bob Eve}.each do |name|
+ create(:user,
+ email: "#{name}@gitlab.com",
+ username: name,
+ name: name)
+ end
+ end
+
+ let(:iterations) { 1000 }
+
+ describe 'using a capitalized username' do
+ benchmark_subject { User.by_login('Alice') }
+
+ it { is_expected.to iterate_per_second(iterations) }
+ end
+
+ describe 'using a lowercase username' do
+ benchmark_subject { User.by_login('alice') }
+
+ it { is_expected.to iterate_per_second(iterations) }
+ end
+
+ describe 'using a capitalized Email address' do
+ benchmark_subject { User.by_login('Alice@gitlab.com') }
+
+ it { is_expected.to iterate_per_second(iterations) }
+ end
+
+ describe 'using a lowercase Email address' do
+ benchmark_subject { User.by_login('alice@gitlab.com') }
+
+ it { is_expected.to iterate_per_second(iterations) }
+ end
+ end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index dfe855926c6..05bb32baa90 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -14,6 +14,7 @@ require File.expand_path("../../config/environment", __FILE__)
require 'rspec/rails'
require 'shoulda/matchers'
require 'sidekiq/testing/inline'
+require 'benchmark/ips'
# Requires supporting ruby files with custom matchers and macros, etc,
# in spec/support/ and its subdirectories.
@@ -32,7 +33,7 @@ RSpec.configure do |config|
config.include TestEnv
config.include StubGitlabCalls
config.include StubGitlabData
-
+ config.include BenchmarkMatchers, benchmark: true
config.infer_spec_type_from_file_location!
config.raise_errors_for_deprecations!
diff --git a/spec/support/matchers/benchmark_matchers.rb b/spec/support/matchers/benchmark_matchers.rb
new file mode 100644
index 00000000000..b73a53917f0
--- /dev/null
+++ b/spec/support/matchers/benchmark_matchers.rb
@@ -0,0 +1,59 @@
+module BenchmarkMatchers
+ extend RSpec::Matchers::DSL
+
+ def self.included(into)
+ into.extend(ClassMethods)
+ end
+
+ matcher :iterate_per_second do |min_iterations|
+ supports_block_expectations
+
+ match do |block|
+ @max_stddev ||= 30
+
+ @entry = benchmark(&block)
+
+ expect(@entry.ips).to be >= min_iterations
+ expect(@entry.stddev_percentage).to be <= @max_stddev
+ end
+
+ chain :with_maximum_stddev do |value|
+ @max_stddev = value
+ end
+
+ description do
+ "run at least #{min_iterations} iterations per second"
+ end
+
+ failure_message do
+ ips = @entry.ips.round(2)
+ stddev = @entry.stddev_percentage.round(2)
+
+ "expected at least #{min_iterations} iterations per second " \
+ "with a maximum stddev of #{@max_stddev}%, instead of " \
+ "#{ips} iterations per second with a stddev of #{stddev}%"
+ end
+ end
+
+ # Benchmarks the given block and returns a Benchmark::IPS::Report::Entry.
+ def benchmark(&block)
+ report = Benchmark.ips(quiet: true) do |bench|
+ bench.report(&block)
+ end
+
+ report.entries[0]
+ end
+
+ module ClassMethods
+ # Wraps around rspec's subject method so you can write:
+ #
+ # benchmark_subject { SomeClass.some_method }
+ #
+ # instead of:
+ #
+ # subject { -> { SomeClass.some_method } }
+ def benchmark_subject(&block)
+ subject { block }
+ end
+ end
+end
diff --git a/spec/support/setup_builds_storage.rb b/spec/support/setup_builds_storage.rb
index a3e59646187..a4f21e95338 100644
--- a/spec/support/setup_builds_storage.rb
+++ b/spec/support/setup_builds_storage.rb
@@ -10,8 +10,10 @@ RSpec.configure do |config|
end
config.after(:suite) do
- Dir.chdir(builds_path) do
- `ls | grep -v .gitkeep | xargs rm -r`
+ Dir[File.join(builds_path, '*')].each do |path|
+ next if File.basename(path) == '.gitkeep'
+
+ FileUtils.rm_rf(path)
end
end
end