diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2016-03-09 13:39:48 +0300 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-03-10 13:15:43 +0300 |
commit | d69dff5b4b1c2dd4944269703ff7af32869adf13 (patch) | |
tree | c018e9f3325574ef6c1e040030e6ae6f67bf40e2 | |
parent | 491ac7ce4b79c901e23799d2062f9f013f08c6c3 (diff) |
Removed benchmark suite and its documentation
The rationale for this can be found in
https://gitlab.com/gitlab-org/gitlab-ce/issues/13718 but in short the
benchmark suite no longer serves a good purpose now that we have proper
production monitoring in place.
Fixes gitlab-org/gitlab-ce#13718
-rw-r--r-- | .gitlab-ci.yml | 27 | ||||
-rw-r--r-- | doc/development/README.md | 1 | ||||
-rw-r--r-- | doc/development/benchmarking.md | 69 | ||||
-rw-r--r-- | lib/tasks/spec.rake | 13 | ||||
-rw-r--r-- | spec/benchmarks/finders/issues_finder_spec.rb | 55 | ||||
-rw-r--r-- | spec/benchmarks/finders/trending_projects_finder_spec.rb | 14 | ||||
-rw-r--r-- | spec/benchmarks/lib/gitlab/markdown/reference_filter_spec.rb | 41 | ||||
-rw-r--r-- | spec/benchmarks/models/milestone_spec.rb | 17 | ||||
-rw-r--r-- | spec/benchmarks/models/project_spec.rb | 50 | ||||
-rw-r--r-- | spec/benchmarks/models/project_team_spec.rb | 23 | ||||
-rw-r--r-- | spec/benchmarks/models/user_spec.rb | 78 | ||||
-rw-r--r-- | spec/benchmarks/services/projects/create_service_spec.rb | 28 | ||||
-rw-r--r-- | spec/spec_helper.rb | 2 | ||||
-rw-r--r-- | spec/support/matchers/benchmark_matchers.rb | 61 |
14 files changed, 3 insertions, 476 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d21785f7af2..bd013d50faa 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -71,15 +71,6 @@ spec:services: - ruby - mysql -spec:benchmark: - stage: test - script: - - RAILS_ENV=test bundle exec rake spec:benchmark - tags: - - ruby - - mysql - allow_failure: true - spec:other: stage: test script: @@ -243,22 +234,6 @@ spec:services:ruby22: - ruby - mysql -spec:benchmark:ruby22: - stage: test - image: ruby:2.2 - only: - - master - script: - - RAILS_ENV=test bundle exec rake spec:benchmark - cache: - key: "ruby22" - paths: - - vendor - tags: - - ruby - - mysql - allow_failure: true - spec:other:ruby22: stage: test image: ruby:2.2 @@ -332,4 +307,4 @@ notify:slack: - master@gitlab-org/gitlab-ce - tags@gitlab-org/gitlab-ce - master@gitlab-org/gitlab-ee - - tags@gitlab-org/gitlab-ee
\ No newline at end of file + - tags@gitlab-org/gitlab-ee diff --git a/doc/development/README.md b/doc/development/README.md index f5c3107ff44..1b281809afc 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -1,7 +1,6 @@ # Development - [Architecture](architecture.md) of GitLab -- [Benchmarking](benchmarking.md) - [CI setup](ci_setup.md) for testing GitLab - [Gotchas](gotchas.md) to avoid - [How to dump production data to staging](db_dump.md) diff --git a/doc/development/benchmarking.md b/doc/development/benchmarking.md deleted file mode 100644 index 88e18ee95f9..00000000000 --- a/doc/development/benchmarking.md +++ /dev/null @@ -1,69 +0,0 @@ -# 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 0985ef3a669..2cf7a25a0fd 100644 --- a/lib/tasks/spec.rake +++ b/lib/tasks/spec.rake @@ -46,20 +46,11 @@ 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 --tag ~@models --tag ~@lib --tag ~@services --tag ~@benchmark) + %W(rspec spec --tag ~@api --tag ~@feature --tag ~@models --tag ~@lib --tag ~@services) ] run_commands(cmds) end @@ -69,7 +60,7 @@ desc "GitLab | Run specs" task :spec do cmds = [ %W(rake gitlab:setup), - %W(rspec spec --tag ~@benchmark), + %W(rspec spec), ] run_commands(cmds) end diff --git a/spec/benchmarks/finders/issues_finder_spec.rb b/spec/benchmarks/finders/issues_finder_spec.rb deleted file mode 100644 index b57a33004a4..00000000000 --- a/spec/benchmarks/finders/issues_finder_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -require 'spec_helper' - -describe IssuesFinder, benchmark: true do - describe '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project, :public) } - - let(:label1) { create(:label, project: project, title: 'A') } - let(:label2) { create(:label, project: project, title: 'B') } - - before do - 10.times do |n| - issue = create(:issue, author: user, project: project) - - if n > 4 - create(:label_link, label: label1, target: issue) - create(:label_link, label: label2, target: issue) - end - end - end - - describe 'retrieving issues without labels' do - let(:finder) do - IssuesFinder.new(user, scope: 'all', label_name: Label::None.title, - state: 'opened') - end - - benchmark_subject { finder.execute } - - it { is_expected.to iterate_per_second(2000) } - end - - describe 'retrieving issues with labels' do - let(:finder) do - IssuesFinder.new(user, scope: 'all', label_name: label1.title, - state: 'opened') - end - - benchmark_subject { finder.execute } - - it { is_expected.to iterate_per_second(1000) } - end - - describe 'retrieving issues for a single project' do - let(:finder) do - IssuesFinder.new(user, scope: 'all', label_name: Label::None.title, - state: 'opened', project_id: project.id) - end - - benchmark_subject { finder.execute } - - it { is_expected.to iterate_per_second(2000) } - end - end -end diff --git a/spec/benchmarks/finders/trending_projects_finder_spec.rb b/spec/benchmarks/finders/trending_projects_finder_spec.rb deleted file mode 100644 index 551ce21840d..00000000000 --- a/spec/benchmarks/finders/trending_projects_finder_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'spec_helper' - -describe TrendingProjectsFinder, benchmark: true do - describe '#execute' do - let(:finder) { described_class.new } - let(:user) { create(:user) } - - # to_a is used to force actually running the query (instead of just building - # it). - benchmark_subject { finder.execute(user).non_archived.to_a } - - it { is_expected.to iterate_per_second(500) } - end -end diff --git a/spec/benchmarks/lib/gitlab/markdown/reference_filter_spec.rb b/spec/benchmarks/lib/gitlab/markdown/reference_filter_spec.rb deleted file mode 100644 index 3855763b200..00000000000 --- a/spec/benchmarks/lib/gitlab/markdown/reference_filter_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::ReferenceFilter, benchmark: true do - let(:input) do - html = <<-EOF -<p>Hello @alice and @bob, how are you doing today?</p> -<p>This is simple @dummy text to see how the @ReferenceFilter class performs -when @processing HTML.</p> - EOF - - Nokogiri::HTML.fragment(html) - end - - let(:project) { create(:empty_project) } - - let(:filter) { described_class.new(input, project: project) } - - describe '#replace_text_nodes_matching' do - let(:iterations) { 6000 } - - describe 'with identical input and output HTML' do - benchmark_subject do - filter.replace_text_nodes_matching(User.reference_pattern) do |content| - content - end - end - - it { is_expected.to iterate_per_second(iterations) } - end - - describe 'with different input and output HTML' do - benchmark_subject do - filter.replace_text_nodes_matching(User.reference_pattern) do |content| - '@eve' - end - end - - it { is_expected.to iterate_per_second(iterations) } - end - end -end diff --git a/spec/benchmarks/models/milestone_spec.rb b/spec/benchmarks/models/milestone_spec.rb deleted file mode 100644 index a94afc4c40d..00000000000 --- a/spec/benchmarks/models/milestone_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -require 'spec_helper' - -describe Milestone, benchmark: true do - describe '#sort_issues' do - let(:milestone) { create(:milestone) } - - let(:issue1) { create(:issue, milestone: milestone) } - let(:issue2) { create(:issue, milestone: milestone) } - let(:issue3) { create(:issue, milestone: milestone) } - - let(:issue_ids) { [issue3.id, issue2.id, issue1.id] } - - benchmark_subject { milestone.sort_issues(issue_ids) } - - it { is_expected.to iterate_per_second(500) } - end -end diff --git a/spec/benchmarks/models/project_spec.rb b/spec/benchmarks/models/project_spec.rb deleted file mode 100644 index cee0949edc5..00000000000 --- a/spec/benchmarks/models/project_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'spec_helper' - -describe Project, benchmark: true do - describe '.trending' do - let(:group) { create(:group) } - let(:project1) { create(:empty_project, :public, group: group) } - let(:project2) { create(:empty_project, :public, group: group) } - - let(:iterations) { 500 } - - before do - 2.times do - create(:note_on_commit, project: project1) - end - - create(:note_on_commit, project: project2) - end - - describe 'without an explicit start date' do - benchmark_subject { described_class.trending.to_a } - - it { is_expected.to iterate_per_second(iterations) } - end - - describe 'with an explicit start date' do - let(:date) { 1.month.ago } - - benchmark_subject { described_class.trending(date).to_a } - - it { is_expected.to iterate_per_second(iterations) } - end - end - - describe '.find_with_namespace' do - let(:group) { create(:group, name: 'sisinmaru') } - let(:project) { create(:project, name: 'maru', namespace: group) } - - describe 'using a capitalized namespace' do - benchmark_subject { described_class.find_with_namespace('sisinmaru/MARU') } - - it { is_expected.to iterate_per_second(600) } - end - - describe 'using a lowercased namespace' do - benchmark_subject { described_class.find_with_namespace('sisinmaru/maru') } - - it { is_expected.to iterate_per_second(600) } - end - end -end diff --git a/spec/benchmarks/models/project_team_spec.rb b/spec/benchmarks/models/project_team_spec.rb deleted file mode 100644 index 8b039ef7317..00000000000 --- a/spec/benchmarks/models/project_team_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe ProjectTeam, benchmark: true do - describe '#max_member_access' do - let(:group) { create(:group) } - let(:project) { create(:empty_project, group: group) } - let(:user) { create(:user) } - - before do - project.team << [user, :master] - - 5.times do - project.team << [create(:user), :reporter] - - project.group.add_user(create(:user), :reporter) - end - end - - benchmark_subject { project.team.max_member_access(user.id) } - - it { is_expected.to iterate_per_second(35000) } - end -end diff --git a/spec/benchmarks/models/user_spec.rb b/spec/benchmarks/models/user_spec.rb deleted file mode 100644 index 1be7a8d3ed9..00000000000 --- a/spec/benchmarks/models/user_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -require 'spec_helper' - -describe User, benchmark: true do - describe '.all' do - before do - 10.times { create(:user) } - end - - benchmark_subject { User.all.to_a } - - it { is_expected.to iterate_per_second(500) } - end - - 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 - - # The iteration count is based on the query taking little over 1 ms when - # using PostgreSQL. - let(:iterations) { 900 } - - 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 - - describe '.find_by_any_email' do - let(:user) { create(:user) } - - describe 'using a user with only a single Email address' do - let(:email) { user.email } - - benchmark_subject { User.find_by_any_email(email) } - - it { is_expected.to iterate_per_second(1000) } - end - - describe 'using a user with multiple Email addresses' do - let(:email) { user.emails.first.email } - - benchmark_subject { User.find_by_any_email(email) } - - before do - 10.times do - user.emails.create(email: FFaker::Internet.email) - end - end - - it { is_expected.to iterate_per_second(1000) } - end - end -end diff --git a/spec/benchmarks/services/projects/create_service_spec.rb b/spec/benchmarks/services/projects/create_service_spec.rb deleted file mode 100644 index 25ed48c34fd..00000000000 --- a/spec/benchmarks/services/projects/create_service_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'spec_helper' - -describe Projects::CreateService, benchmark: true do - describe '#execute' do - let(:user) { create(:user, :admin) } - - let(:group) do - group = create(:group) - - create(:group_member, group: group, user: user) - - group - end - - benchmark_subject do - name = SecureRandom.hex - service = described_class.new(user, - name: name, - path: name, - namespace_id: group.id, - visibility_level: Gitlab::VisibilityLevel::PUBLIC) - - service.execute - end - - it { is_expected.to iterate_per_second(0.5) } - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 159fb964171..7d939ca7509 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,7 +14,6 @@ require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' require 'shoulda/matchers' require 'sidekiq/testing/inline' -require 'benchmark/ips' require 'rspec/retry' # Requires supporting ruby files with custom matchers and macros, etc, @@ -38,7 +37,6 @@ RSpec.configure do |config| config.include ActiveJob::TestHelper 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 deleted file mode 100644 index 84f655c2119..00000000000 --- a/spec/support/matchers/benchmark_matchers.rb +++ /dev/null @@ -1,61 +0,0 @@ -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 do - instance_eval(&block) - end - 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 |