From 24c2b9d65b378ce6a7113e7470437b4c57620d3d Mon Sep 17 00:00:00 2001 From: "d.bukholtsev" Date: Mon, 3 May 2021 17:56:19 +1000 Subject: [PATCH 1/6] =?UTF-8?q?=D0=A1=D0=BE=D0=B7=D0=B4=D0=B0=D0=BD=D0=B8?= =?UTF-8?q?=D0=B5=20Feedback=20loop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Gemfile | 7 ++ Gemfile.lock | 53 ++++++++++++- app/services/reload_json.rb | 38 ++++++++++ case-study.md | 40 ++++++++++ config/database.yml | 2 + lib/tasks/utils.rake | 34 ++------- spec/controllers/trips_controller_spec.rb | 21 ++++++ spec/rails_helper.rb | 71 ++++++++++++++++++ spec/services/reload_json_spec.rb | 24 ++++++ spec/spec_helper.rb | 90 +++++++++++++++++++++++ test/application_system_test_case.rb | 5 -- test/controllers/.keep | 0 test/fixtures/.keep | 0 test/fixtures/files/.keep | 0 test/helpers/.keep | 0 test/integration/.keep | 0 test/mailers/.keep | 0 test/models/.keep | 0 test/system/.keep | 0 test/test_helper.rb | 10 --- 20 files changed, 351 insertions(+), 44 deletions(-) create mode 100644 app/services/reload_json.rb create mode 100644 case-study.md create mode 100644 spec/controllers/trips_controller_spec.rb create mode 100644 spec/rails_helper.rb create mode 100644 spec/services/reload_json_spec.rb create mode 100644 spec/spec_helper.rb delete mode 100644 test/application_system_test_case.rb delete mode 100644 test/controllers/.keep delete mode 100644 test/fixtures/.keep delete mode 100644 test/fixtures/files/.keep delete mode 100644 test/helpers/.keep delete mode 100644 test/integration/.keep delete mode 100644 test/mailers/.keep delete mode 100644 test/models/.keep delete mode 100644 test/system/.keep delete mode 100644 test/test_helper.rb diff --git a/Gemfile b/Gemfile index e20b1260..bdc520e3 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,9 @@ gem 'rails', '~> 5.2.3' gem 'pg', '>= 0.18', '< 2.0' gem 'puma', '~> 3.11' gem 'bootsnap', '>= 1.1.0', require: false +gem 'benchmark-ips' +gem 'kalibera' +gem 'activerecord-import' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console @@ -20,6 +23,10 @@ group :development do end group :test do + gem 'rspec' + gem 'rspec-rails' + gem 'rspec-benchmark' + gem 'rails-controller-testing' end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem diff --git a/Gemfile.lock b/Gemfile.lock index fccf6f5f..b12be6f0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -33,6 +33,8 @@ GEM activemodel (= 5.2.3) activesupport (= 5.2.3) arel (>= 9.0) + activerecord-import (1.0.8) + activerecord (>= 3.2) activestorage (5.2.3) actionpack (= 5.2.3) activerecord (= 5.2.3) @@ -43,6 +45,10 @@ GEM minitest (~> 5.1) tzinfo (~> 1.1) arel (9.0.0) + benchmark-ips (2.8.4) + benchmark-malloc (0.2.0) + benchmark-perf (0.6.0) + benchmark-trend (0.4.0) bindex (0.6.0) bootsnap (1.4.2) msgpack (~> 1.0) @@ -50,12 +56,16 @@ GEM byebug (11.0.1) concurrent-ruby (1.1.5) crass (1.0.4) + diff-lcs (1.4.4) erubi (1.8.0) ffi (1.10.0) globalid (0.4.2) activesupport (>= 4.2.0) i18n (1.6.0) concurrent-ruby (~> 1.0) + kalibera (0.1.1) + memoist (~> 0.11.0) + rbzip2 (~> 0.2.0) listen (3.1.5) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) @@ -67,8 +77,11 @@ GEM mini_mime (>= 0.1.1) marcel (0.3.3) mimemagic (~> 0.3.2) + memoist (0.11.0) method_source (0.9.2) - mimemagic (0.3.3) + mimemagic (0.3.10) + nokogiri (~> 1) + rake mini_mime (1.0.1) mini_portile2 (2.4.0) minitest (5.11.3) @@ -94,6 +107,10 @@ GEM bundler (>= 1.3.0) railties (= 5.2.3) sprockets-rails (>= 2.0.0) + rails-controller-testing (1.0.5) + actionpack (>= 5.0.1.rc1) + actionview (>= 5.0.1.rc1) + activesupport (>= 5.0.1.rc1) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) @@ -109,6 +126,33 @@ GEM rb-fsevent (0.10.3) rb-inotify (0.10.0) ffi (~> 1.0) + rbzip2 (0.2.0) + rspec (3.10.0) + rspec-core (~> 3.10.0) + rspec-expectations (~> 3.10.0) + rspec-mocks (~> 3.10.0) + rspec-benchmark (0.6.0) + benchmark-malloc (~> 0.2) + benchmark-perf (~> 0.6) + benchmark-trend (~> 0.4) + rspec (>= 3.0) + rspec-core (3.10.1) + rspec-support (~> 3.10.0) + rspec-expectations (3.10.1) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.10.0) + rspec-mocks (3.10.2) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.10.0) + rspec-rails (5.0.1) + actionpack (>= 5.2) + activesupport (>= 5.2) + railties (>= 5.2) + rspec-core (~> 3.10) + rspec-expectations (~> 3.10) + rspec-mocks (~> 3.10) + rspec-support (~> 3.10) + rspec-support (3.10.2) ruby_dep (1.5.0) sprockets (3.7.2) concurrent-ruby (~> 1.0) @@ -134,12 +178,19 @@ PLATFORMS ruby DEPENDENCIES + activerecord-import + benchmark-ips bootsnap (>= 1.1.0) byebug + kalibera listen (>= 3.0.5, < 3.2) pg (>= 0.18, < 2.0) puma (~> 3.11) rails (~> 5.2.3) + rails-controller-testing + rspec + rspec-benchmark + rspec-rails tzinfo-data web-console (>= 3.3.0) diff --git a/app/services/reload_json.rb b/app/services/reload_json.rb new file mode 100644 index 00000000..9c239ec6 --- /dev/null +++ b/app/services/reload_json.rb @@ -0,0 +1,38 @@ +class ReloadJson + def initialize(file_name) + @file_name = file_name + end + + def call + json = JSON.parse(File.read(@file_name)) + + ActiveRecord::Base.transaction do + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + ActiveRecord::Base.connection.execute('delete from buses_services;') + + json.each do |trip| + from = City.find_or_create_by(name: trip['from']) + to = City.find_or_create_by(name: trip['to']) + services = [] + trip['bus']['services'].each do |service| + s = Service.find_or_create_by(name: service) + services << s + end + bus = Bus.find_or_create_by(number: trip['bus']['number']) + bus.update(model: trip['bus']['model'], services: services) + + Trip.create!( + from: from, + to: to, + bus: bus, + start_time: trip['start_time'], + duration_minutes: trip['duration_minutes'], + price_cents: trip['price_cents'], + ) + end + end + end +end \ No newline at end of file diff --git a/case-study.md b/case-study.md new file mode 100644 index 00000000..16eb1551 --- /dev/null +++ b/case-study.md @@ -0,0 +1,40 @@ +# Case-study оптимизации + +## Актуальная проблема +В нашем проекте возникли 2 проблемы. +1) Необходимо было импортировать в базу файл с данными, чуть больше 32 мегабайт (100К записей). + Рейк таск успешно работал на файлах размером пару мегабайт, но c большими файлами он работала слишком долго. + Я решил исправить эту проблему, оптимизировав этот скрипт. + +2) Страницы расписаний тоже формируются не эффективно и при росте количества записей начинают сильно тормозить. + Я решил исправить эту проблему, оптимизировав sql-запросы. + +## Формирование метрики +Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: +время выполнения программы/запроса в секундах + +Бюджет метрики для импорта large.json - 60 секунд. + +## Гарантия корректности работы оптимизированной программы +Программа поставлялась без теста. Поэтому необхожимо было написать тест, проверяющий корректность работы программы. +Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. + +## Feedback-Loop +Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за ~10 секунд + +Вот как я построил `feedback_loop`: +- написал тесты для проверки корректности работы рейк таски/контроллера +- измерил метрику: для рейк таски для example.json (10К записей) - 200 мс +- написал performance тесты для защиты от деградации производительности +- далее использовал эти же входные данные для дальнейшего профилирования и сравнения метрик + +## Вникаем в детали системы, чтобы найти главные точки роста +Для того, чтобы найти "точки роста" для оптимизации я воспользовался: +- rack-mini-profiler +- rails panel +- bullet +- explain запросов + +Вот какие проблемы удалось найти и решить: + +Ваша находка №1 \ No newline at end of file diff --git a/config/database.yml b/config/database.yml index e116cfa6..74ccc336 100644 --- a/config/database.yml +++ b/config/database.yml @@ -15,6 +15,8 @@ # gem 'pg' # default: &default + username: postgres + password: admin123 adapter: postgresql encoding: unicode # For details on connection pooling, see Rails configuration guide diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe871..57a27df6 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,34 +1,12 @@ +require 'benchmark/ips' + # Наивная загрузка данных из json-файла в БД # rake reload_json[fixtures/small.json] task :reload_json, [:file_name] => :environment do |_task, args| - json = JSON.parse(File.read(args.file_name)) - - ActiveRecord::Base.transaction do - City.delete_all - Bus.delete_all - Service.delete_all - Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') - - json.each do |trip| - from = City.find_or_create_by(name: trip['from']) - to = City.find_or_create_by(name: trip['to']) - services = [] - trip['bus']['services'].each do |service| - s = Service.find_or_create_by(name: service) - services << s - end - bus = Bus.find_or_create_by(number: trip['bus']['number']) - bus.update(model: trip['bus']['model'], services: services) - - Trip.create!( - from: from, - to: to, - bus: bus, - start_time: trip['start_time'], - duration_minutes: trip['duration_minutes'], - price_cents: trip['price_cents'], - ) + Benchmark.ips do |x| + x.config(:stats => :bootstrap, :confidence => 95) + x.report do + ReloadJson.new(args.file_name).call end end end diff --git a/spec/controllers/trips_controller_spec.rb b/spec/controllers/trips_controller_spec.rb new file mode 100644 index 00000000..cea94ea4 --- /dev/null +++ b/spec/controllers/trips_controller_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec::Benchmark.configure do |config| + config.run_in_subprocess = false +end + +RSpec.describe TripsController, :type => :controller do + describe '#index' do + subject { get :index, params: { from: 'Самара', to: 'Москва' } } + + let(:file_name) { Rails.root.join('fixtures', 'small.json') } + + describe 'benchmark' do + it do + ReloadJson.new(file_name).call + expect { subject }.to perform_under(0.05).ms + expect(assigns(:trips).size).to eq 13 + end + end + end +end \ No newline at end of file diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb new file mode 100644 index 00000000..d3c36924 --- /dev/null +++ b/spec/rails_helper.rb @@ -0,0 +1,71 @@ +# This file is copied to spec/ when you run 'rails generate rspec:install' +require 'spec_helper' +ENV['RAILS_ENV'] ||= 'test' +require File.expand_path('../config/environment', __dir__) +# Prevent database truncation if the environment is production +abort("The Rails environment is running in production mode!") if Rails.env.production? +require 'rspec/rails' + +require 'rspec-benchmark' +# Add additional requires below this line. Rails is not loaded until this point! + +# Requires supporting ruby files with custom matchers and macros, etc, in +# spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are +# run as spec files by default. This means that files in spec/support that end +# in _spec.rb will both be required and run as specs, causing the specs to be +# run twice. It is recommended that you do not name files matching this glob to +# end with _spec.rb. You can configure this pattern with the --pattern +# option on the command line or in ~/.rspec, .rspec or `.rspec-local`. +# +# The following line is provided for convenience purposes. It has the downside +# of increasing the boot-up time by auto-requiring all files in the support +# directory. Alternatively, in the individual `*_spec.rb` files, manually +# require only the support files necessary. +# +# Dir[Rails.root.join('spec', 'support', '**', '*.rb')].sort.each { |f| require f } + +# Checks for pending migrations and applies them before tests are run. +# If you are not using ActiveRecord, you can remove these lines. +begin + ActiveRecord::Migration.maintain_test_schema! +rescue ActiveRecord::PendingMigrationError => e + puts e.to_s.strip + exit 1 +end +RSpec.configure do |config| + config.include RSpec::Benchmark::Matchers + # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures + config.fixture_path = "#{::Rails.root}/spec/fixtures" + + # If you're not using ActiveRecord, or you'd prefer not to run each of your + # examples within a transaction, remove the following line or assign false + # instead of true. + config.use_transactional_fixtures = true + + # You can uncomment this line to turn off ActiveRecord support entirely. + # config.use_active_record = false + + # RSpec Rails can automatically mix in different behaviours to your tests + # based on their file location, for example enabling you to call `get` and + # `post` in specs under `spec/controllers`. + # + # You can disable this behaviour by removing the line below, and instead + # explicitly tag your specs with their type, e.g.: + # + # RSpec.describe UsersController, type: :controller do + # # ... + # end + # + # The different available types are documented in the features, such as in + # https://relishapp.com/rspec/rspec-rails/docs + config.infer_spec_type_from_file_location! + + # Filter lines from Rails gems in backtraces. + config.filter_rails_from_backtrace! + # arbitrary gems may also be filtered via: + # config.filter_gems_from_backtrace("gem name") +end + +RSpec::Benchmark.configure do |config| + config.run_in_subprocess = true +end diff --git a/spec/services/reload_json_spec.rb b/spec/services/reload_json_spec.rb new file mode 100644 index 00000000..af28f157 --- /dev/null +++ b/spec/services/reload_json_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe ReloadJson, :type => :service do + describe '#call' do + subject { described_class.new(file_name).call } + + let(:file_name) { Rails.root.join('fixtures', 'example.json') } + + describe 'validity' do + it 'saves records' do + subject + expect(City.count).to eq 2 + expect(Bus.count).to eq 1 + expect(Bus.take.services.count).to eq 2 + expect(Service.count).to eq 2 + expect(Trip.count).to eq 10 + end + end + + describe 'benchmark' do + it { expect { subject }.to perform_under(200).ms.warmup(5).times.sample(10).times } + end + end +end \ No newline at end of file diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 00000000..d6bc6233 --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,90 @@ +# This file was generated by the `rails generate rspec:install` command. Conventionally, all +# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. +# The generated `.rspec` file contains `--require spec_helper` which will cause +# this file to always be loaded, without a need to explicitly require it in any +# files. +# +# Given that it is always loaded, you are encouraged to keep this file as +# light-weight as possible. Requiring heavyweight dependencies from this file +# will add to the boot time of your test suite on EVERY test run, even for an +# individual file that may not need all of that loaded. Instead, consider making +# a separate helper file that requires the additional dependencies and performs +# the additional setup, and require it from the spec files that actually need +# it. +# +# See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration +RSpec.configure do |config| + # rspec-expectations config goes here. You can use an alternate + # assertion/expectation library such as wrong or the stdlib/minitest + # assertions if you prefer. + config.expect_with :rspec do |expectations| + # This option will default to `true` in RSpec 4. It makes the `description` + # and `failure_message` of custom matchers include text for helper methods + # defined using `chain`, e.g.: + # be_bigger_than(2).and_smaller_than(4).description + # # => "be bigger than 2 and smaller than 4" + # ...rather than: + # # => "be bigger than 2" + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + end + + # rspec-mocks config goes here. You can use an alternate test double + # library (such as bogus or mocha) by changing the `mock_with` option here. + config.mock_with :rspec do |mocks| + # Prevents you from mocking or stubbing a method that does not exist on + # a real object. This is generally recommended, and will default to + # `true` in RSpec 4. + mocks.verify_partial_doubles = true + end + + # This option will default to `:apply_to_host_groups` in RSpec 4 (and will + # have no way to turn it off -- the option exists only for backwards + # compatibility in RSpec 3). It causes shared context metadata to be + # inherited by the metadata hash of host groups and examples, rather than + # triggering implicit auto-inclusion in groups with matching metadata. + config.shared_context_metadata_behavior = :apply_to_host_groups + + # The settings below are suggested to provide a good initial experience + # with RSpec, but feel free to customize to your heart's content. +=begin + # This allows you to limit a spec run to individual examples or groups + # you care about by tagging them with `:focus` metadata. When nothing + # is tagged with `:focus`, all examples get run. RSpec also provides + # aliases for `it`, `describe`, and `context` that include `:focus` + # metadata: `fit`, `fdescribe` and `fcontext`, respectively. + config.filter_run_when_matching :focus + # Allows RSpec to persist some state between runs in order to support + # the `--only-failures` and `--next-failure` CLI options. We recommend + # you configure your source control system to ignore this file. + config.example_status_persistence_file_path = "spec/examples.txt" + # Limits the available syntax to the non-monkey patched syntax that is + # recommended. For more details, see: + # - http://rspec.info/blog/2012/06/rspecs-new-expectation-syntax/ + # - http://www.teaisaweso.me/blog/2013/05/27/rspecs-new-message-expectation-syntax/ + # - http://rspec.info/blog/2014/05/notable-changes-in-rspec-3/#zero-monkey-patching-mode + config.disable_monkey_patching! + # Many RSpec users commonly either run the entire suite or an individual + # file, and it's useful to allow more verbose output when running an + # individual spec file. + if config.files_to_run.one? + # Use the documentation formatter for detailed output, + # unless a formatter has already been configured + # (e.g. via a command-line flag). + config.default_formatter = "doc" + end + # Print the 10 slowest examples and example groups at the + # end of the spec run, to help surface which specs are running + # particularly slow. + config.profile_examples = 10 + # Run specs in random order to surface order dependencies. If you find an + # order dependency and want to debug it, you can fix the order by providing + # the seed, which is printed after each run. + # --seed 1234 + config.order = :random + # Seed global randomization in this process using the `--seed` CLI option. + # Setting this allows you to use `--seed` to deterministically reproduce + # test failures related to randomization by passing the same `--seed` value + # as the one that triggered the failure. + Kernel.srand config.seed +=end +end \ No newline at end of file diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb deleted file mode 100644 index d19212ab..00000000 --- a/test/application_system_test_case.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "test_helper" - -class ApplicationSystemTestCase < ActionDispatch::SystemTestCase - driven_by :selenium, using: :chrome, screen_size: [1400, 1400] -end diff --git a/test/controllers/.keep b/test/controllers/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/fixtures/.keep b/test/fixtures/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/fixtures/files/.keep b/test/fixtures/files/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/helpers/.keep b/test/helpers/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/integration/.keep b/test/integration/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/mailers/.keep b/test/mailers/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/models/.keep b/test/models/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/system/.keep b/test/system/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/test_helper.rb b/test/test_helper.rb deleted file mode 100644 index 3ab84e3d..00000000 --- a/test/test_helper.rb +++ /dev/null @@ -1,10 +0,0 @@ -ENV['RAILS_ENV'] ||= 'test' -require_relative '../config/environment' -require 'rails/test_help' - -class ActiveSupport::TestCase - # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. - fixtures :all - - # Add more helper methods to be used by all tests here... -end From 30a937d5948381722a05ede3d12125512583b3cb Mon Sep 17 00:00:00 2001 From: "d.bukholtsev" Date: Mon, 3 May 2021 23:15:50 +1000 Subject: [PATCH 2/6] =?UTF-8?q?=D0=98=D1=81=D0=BF=D0=BE=D0=BB=D1=8C=D0=B7?= =?UTF-8?q?=D0=BE=D0=B2=D0=B0=D0=BD=D0=B8=D0=B5=20activerecord-import?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/bus.rb | 3 +- app/models/buses_service.rb | 7 +++ app/models/service.rb | 3 +- app/services/reload_json.rb | 69 ++++++++++++++++++----- case-study.md | 14 +++-- config/environments/test.rb | 1 + spec/controllers/trips_controller_spec.rb | 2 +- spec/services/reload_json_spec.rb | 6 +- 8 files changed, 83 insertions(+), 22 deletions(-) create mode 100644 app/models/buses_service.rb diff --git a/app/models/bus.rb b/app/models/bus.rb index 1dcc54cb..3fc47767 100644 --- a/app/models/bus.rb +++ b/app/models/bus.rb @@ -13,7 +13,8 @@ class Bus < ApplicationRecord ].freeze has_many :trips - has_and_belongs_to_many :services, join_table: :buses_services + has_many :buses_services + has_and_belongs_to_many :services, through: :buses_services validates :number, presence: true, uniqueness: true validates :model, inclusion: { in: MODELS } diff --git a/app/models/buses_service.rb b/app/models/buses_service.rb new file mode 100644 index 00000000..ca1022e9 --- /dev/null +++ b/app/models/buses_service.rb @@ -0,0 +1,7 @@ +class BusesService < ApplicationRecord + belongs_to :bus + belongs_to :service + + validates :bus, presence: true + validates :service, presence: true +end diff --git a/app/models/service.rb b/app/models/service.rb index 9cbb2a32..ff75fea3 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -12,7 +12,8 @@ class Service < ApplicationRecord 'Можно не печатать билет', ].freeze - has_and_belongs_to_many :buses, join_table: :buses_services + has_many :buses_services + has_and_belongs_to_many :buses, through: :buses_services validates :name, presence: true validates :name, inclusion: { in: SERVICES } diff --git a/app/services/reload_json.rb b/app/services/reload_json.rb index 9c239ec6..8c9f7050 100644 --- a/app/services/reload_json.rb +++ b/app/services/reload_json.rb @@ -5,34 +5,77 @@ def initialize(file_name) def call json = JSON.parse(File.read(@file_name)) + trips = [] + buses = {} + services = {} + cities = {} + bus_services = {} ActiveRecord::Base.transaction do City.delete_all Bus.delete_all Service.delete_all Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') + BusesService.delete_all json.each do |trip| - from = City.find_or_create_by(name: trip['from']) - to = City.find_or_create_by(name: trip['to']) - services = [] - trip['bus']['services'].each do |service| - s = Service.find_or_create_by(name: service) - services << s + from = find_or_new_city!(cities, trip['from']) + to = find_or_new_city!(cities, trip['to']) + + bus = buses[trip['bus']['number']] + unless bus + bus = Bus.new(number: trip['bus']['number']) + bus.model = trip['bus']['model'] + trip['bus']['services'].each do |service| + s = find_or_new_service!(services, service) + find_or_new_bus_service!(bus_services, bus, s) + end + buses[trip['bus']['number']] = bus end - bus = Bus.find_or_create_by(number: trip['bus']['number']) - bus.update(model: trip['bus']['model'], services: services) - Trip.create!( + trips << Trip.new( from: from, to: to, bus: bus, start_time: trip['start_time'], duration_minutes: trip['duration_minutes'], - price_cents: trip['price_cents'], - ) + price_cents: trip['price_cents'] + ) end + + City.import! cities.values + Service.import! services.values + Bus.import! buses.values + BusesService.import! bus_services.values.map(&:values).flatten + Trip.import! trips + end + end + + def find_or_new_city!(cities, name) + found = cities[name] + unless found + found = City.new(name: name) + cities[name] = found + end + found + end + + def find_or_new_service!(services, name) + found = services[name] + unless found + found = Service.new(name: name) + services[name] = found + end + found + end + + def find_or_new_bus_service!(bus_services, bus, service) + found = bus_services.dig(bus, service) + unless found + found = BusesService.new(bus: bus, service: service) + bus_services[bus] = {} unless bus_services[bus] + bus_services[bus][service] = found end + found end -end \ No newline at end of file +end diff --git a/case-study.md b/case-study.md index 16eb1551..38ee22a0 100644 --- a/case-study.md +++ b/case-study.md @@ -16,15 +16,16 @@ Бюджет метрики для импорта large.json - 60 секунд. ## Гарантия корректности работы оптимизированной программы -Программа поставлялась без теста. Поэтому необхожимо было написать тест, проверяющий корректность работы программы. +Программа поставлялась без теста. Поэтому необходимо было написать тест, проверяющий корректность работы программы. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. ## Feedback-Loop -Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за ~10 секунд +Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, +который позволил мне получать обратную связь по эффективности сделанных изменений за ~15 секунд Вот как я построил `feedback_loop`: - написал тесты для проверки корректности работы рейк таски/контроллера -- измерил метрику: для рейк таски для example.json (10К записей) - 200 мс +- измерил метрику: для рейк таски с example.json (10К записей) - 200 мс - написал performance тесты для защиты от деградации производительности - далее использовал эти же входные данные для дальнейшего профилирования и сравнения метрик @@ -37,4 +38,9 @@ Вот какие проблемы удалось найти и решить: -Ваша находка №1 \ No newline at end of file +Ваша находка №1 +- Стандартный лог в консоли показал, что выполняется очень много sql запросов при импорте +- Переписал рейк таск с использованием библиотеки activerecord-import, каждая таблица заполняется одним запросом, + в том числе и join-таблица buses_services. +- Метрика уменьшилась с 200 до 25 мс для small.json +- Количество sql запросов в логе сильно уменьшилось. Прогон large.txt занимает 47 секунд - уложились в бюджет. \ No newline at end of file diff --git a/config/environments/test.rb b/config/environments/test.rb index 0a38fd3c..29bf7373 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -43,4 +43,5 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true + # ActiveRecord::Base.logger = Logger.new(STDOUT) end diff --git a/spec/controllers/trips_controller_spec.rb b/spec/controllers/trips_controller_spec.rb index cea94ea4..bc16afd5 100644 --- a/spec/controllers/trips_controller_spec.rb +++ b/spec/controllers/trips_controller_spec.rb @@ -13,7 +13,7 @@ describe 'benchmark' do it do ReloadJson.new(file_name).call - expect { subject }.to perform_under(0.05).ms + expect { subject }.to perform_under(0.1).ms expect(assigns(:trips).size).to eq 13 end end diff --git a/spec/services/reload_json_spec.rb b/spec/services/reload_json_spec.rb index af28f157..9a2c0963 100644 --- a/spec/services/reload_json_spec.rb +++ b/spec/services/reload_json_spec.rb @@ -11,14 +11,16 @@ subject expect(City.count).to eq 2 expect(Bus.count).to eq 1 - expect(Bus.take.services.count).to eq 2 expect(Service.count).to eq 2 expect(Trip.count).to eq 10 + expect(Trip.first.from.name).to eq 'Москва' + expect(Trip.first.to.name).to eq 'Самара' + expect(Bus.first.services.count).to eq 2 end end describe 'benchmark' do - it { expect { subject }.to perform_under(200).ms.warmup(5).times.sample(10).times } + it { expect { subject }.to perform_under(25).ms.warmup(5).times.sample(10).times } end end end \ No newline at end of file From 20d5f02fc6f00ca2d0e7be89bf12d7bb07b51d01 Mon Sep 17 00:00:00 2001 From: "d.bukholtsev" Date: Tue, 4 May 2021 21:20:48 +1000 Subject: [PATCH 3/6] =?UTF-8?q?Includes=20=D0=BE=D1=82=20bullet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Gemfile | 4 +--- Gemfile.lock | 6 +++++- app/controllers/trips_controller.rb | 2 +- case-study.md | 15 ++++++++++++--- config/environments/development.rb | 10 ++++++++++ config/environments/test.rb | 6 ++++++ 6 files changed, 35 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index bdc520e3..63a38fff 100644 --- a/Gemfile +++ b/Gemfile @@ -10,6 +10,7 @@ gem 'bootsnap', '>= 1.1.0', require: false gem 'benchmark-ips' gem 'kalibera' gem 'activerecord-import' +gem 'bullet' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console @@ -28,6 +29,3 @@ group :test do gem 'rspec-benchmark' gem 'rails-controller-testing' end - -# Windows does not include zoneinfo files, so bundle the tzinfo-data gem -gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] diff --git a/Gemfile.lock b/Gemfile.lock index b12be6f0..728eb4b6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,6 +53,9 @@ GEM bootsnap (1.4.2) msgpack (~> 1.0) builder (3.2.3) + bullet (6.1.4) + activesupport (>= 3.0.0) + uniform_notifier (~> 1.11) byebug (11.0.1) concurrent-ruby (1.1.5) crass (1.0.4) @@ -165,6 +168,7 @@ GEM thread_safe (0.3.6) tzinfo (1.2.5) thread_safe (~> 0.1) + uniform_notifier (1.14.2) web-console (3.7.0) actionview (>= 5.0) activemodel (>= 5.0) @@ -181,6 +185,7 @@ DEPENDENCIES activerecord-import benchmark-ips bootsnap (>= 1.1.0) + bullet byebug kalibera listen (>= 3.0.5, < 3.2) @@ -191,7 +196,6 @@ DEPENDENCIES rspec rspec-benchmark rspec-rails - tzinfo-data web-console (>= 3.3.0) RUBY VERSION diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be2..2c6777c4 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -2,6 +2,6 @@ class TripsController < ApplicationController def index @from = City.find_by_name!(params[:from]) @to = City.find_by_name!(params[:to]) - @trips = Trip.where(from: @from, to: @to).order(:start_time) + @trips = Trip.where(from: @from, to: @to).order(:start_time).includes(bus: [:services]) end end diff --git a/case-study.md b/case-study.md index 38ee22a0..af4b6047 100644 --- a/case-study.md +++ b/case-study.md @@ -11,7 +11,7 @@ ## Формирование метрики Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: -время выполнения программы/запроса в секундах +время выполнения программы/запроса в секундах. Бюджет метрики для импорта large.json - 60 секунд. @@ -25,7 +25,8 @@ Вот как я построил `feedback_loop`: - написал тесты для проверки корректности работы рейк таски/контроллера -- измерил метрику: для рейк таски с example.json (10К записей) - 200 мс +- измерил метрику: для рейк таски с example.json (10К записей) - 200 мс, + для контроллера с записями из large.json - 25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms) - написал performance тесты для защиты от деградации производительности - далее использовал эти же входные данные для дальнейшего профилирования и сравнения метрик @@ -43,4 +44,12 @@ - Переписал рейк таск с использованием библиотеки activerecord-import, каждая таблица заполняется одним запросом, в том числе и join-таблица buses_services. - Метрика уменьшилась с 200 до 25 мс для small.json -- Количество sql запросов в логе сильно уменьшилось. Прогон large.txt занимает 47 секунд - уложились в бюджет. \ No newline at end of file +- Количество sql запросов в логе сильно уменьшилось. Прогон large.txt занимает 47 секунд - уложились в бюджет. + +Ваша находка №2 +- Отчет bullet показал что в контроллере не хватает eager loading +- Добавил .includes(bus: [:services]) +- Время ответа контроллера уменьшилось с 25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms), + до 22393ms (Views: 22124.9ms | ActiveRecord: 240.2ms) +- Bullet больше не показывает ворнингов + diff --git a/config/environments/development.rb b/config/environments/development.rb index 1311e3e4..90792c10 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -1,4 +1,14 @@ Rails.application.configure do + config.after_initialize do + Bullet.enable = true + Bullet.alert = true + Bullet.bullet_logger = true + Bullet.console = true + # Bullet.growl = true + Bullet.rails_logger = true + Bullet.add_footer = true + end + # Settings specified here will take precedence over those in config/application.rb. # In the development environment your application's code is reloaded on diff --git a/config/environments/test.rb b/config/environments/test.rb index 29bf7373..d9df7ed1 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -1,4 +1,10 @@ Rails.application.configure do + config.after_initialize do + Bullet.enable = true + Bullet.bullet_logger = true + Bullet.raise = true # raise an error if n+1 query occurs + end + # Settings specified here will take precedence over those in config/application.rb. # The test environment is used exclusively to run your application's From 16f035b1e3df912717880e354a9aa06962c4bc4d Mon Sep 17 00:00:00 2001 From: "d.bukholtsev" Date: Tue, 4 May 2021 21:56:30 +1000 Subject: [PATCH 4/6] =?UTF-8?q?=D0=9E=D0=B1=D1=8A=D0=B5=D0=B4=D0=B8=D0=BD?= =?UTF-8?q?=D0=B5=D0=BD=D0=B8=D0=B5=20=D0=B2=D1=8C=D1=8E=D1=85=20=D0=B2=20?= =?UTF-8?q?=D0=BE=D0=B4=D0=BD=D1=83?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Gemfile | 1 + Gemfile.lock | 6 ++++++ app/views/trips/_delimiter.html.erb | 1 - app/views/trips/_service.html.erb | 1 - app/views/trips/_services.html.erb | 6 ------ app/views/trips/_trip.html.erb | 5 ----- app/views/trips/index.html.erb | 17 +++++++++++++---- case-study.md | 8 +++++++- config/environments/development.rb | 2 +- 9 files changed, 28 insertions(+), 19 deletions(-) delete mode 100644 app/views/trips/_delimiter.html.erb delete mode 100644 app/views/trips/_service.html.erb delete mode 100644 app/views/trips/_services.html.erb delete mode 100644 app/views/trips/_trip.html.erb diff --git a/Gemfile b/Gemfile index 63a38fff..a25ad5ca 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,7 @@ group :development do # Access an interactive console on exception pages or by calling 'console' anywhere in the code. gem 'web-console', '>= 3.3.0' gem 'listen', '>= 3.0.5', '< 3.2' + gem 'meta_request' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 728eb4b6..3dfb5212 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -81,6 +81,9 @@ GEM marcel (0.3.3) mimemagic (~> 0.3.2) memoist (0.11.0) + meta_request (0.7.2) + rack-contrib (>= 1.1, < 3) + railties (>= 3.0.0, < 7) method_source (0.9.2) mimemagic (0.3.10) nokogiri (~> 1) @@ -95,6 +98,8 @@ GEM pg (1.1.4) puma (3.12.1) rack (2.0.6) + rack-contrib (2.3.0) + rack (~> 2.0) rack-test (1.1.0) rack (>= 1.0, < 3) rails (5.2.3) @@ -189,6 +194,7 @@ DEPENDENCIES byebug kalibera listen (>= 3.0.5, < 3.2) + meta_request pg (>= 0.18, < 2.0) puma (~> 3.11) rails (~> 5.2.3) diff --git a/app/views/trips/_delimiter.html.erb b/app/views/trips/_delimiter.html.erb deleted file mode 100644 index 3f845ad0..00000000 --- a/app/views/trips/_delimiter.html.erb +++ /dev/null @@ -1 +0,0 @@ -==================================================== diff --git a/app/views/trips/_service.html.erb b/app/views/trips/_service.html.erb deleted file mode 100644 index 178ea8c0..00000000 --- a/app/views/trips/_service.html.erb +++ /dev/null @@ -1 +0,0 @@ -
  • <%= "#{service.name}" %>
    • diff --git a/app/views/trips/_services.html.erb b/app/views/trips/_services.html.erb deleted file mode 100644 index 2de639fc..00000000 --- a/app/views/trips/_services.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -
  • Сервисы в автобусе:
    • -
      - <% services.each do |service| %> - <%= render "service", service: service %> - <% end %> -
    diff --git a/app/views/trips/_trip.html.erb b/app/views/trips/_trip.html.erb deleted file mode 100644 index fa1de9aa..00000000 --- a/app/views/trips/_trip.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -
  • <%= "Отправление: #{trip.start_time}" %>
    • -
  • <%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %>
    • -
  • <%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %>
    • -
  • <%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %>
    • -
  • <%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %>
    • diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce41..d9d293e0 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -7,10 +7,19 @@ <% @trips.each do |trip| %>
      - <%= render "trip", trip: trip %> +
    • <%= "Отправление: #{trip.start_time}" %>
      • +
    • <%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %>
      • +
    • <%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %>
      • +
    • <%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %>
      • +
    • <%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %>
      • <% if trip.bus.services.present? %> - <%= render "services", services: trip.bus.services %> - <% end %> +
    • Сервисы в автобусе:
      • +
        + <% trip.bus.services.each do |service| %> +
      • <%= "#{service.name}" %>
        • + <% end %> +
      + <% end %>
    - <%= render "delimiter" %> + ==================================================== <% end %> diff --git a/case-study.md b/case-study.md index af4b6047..e9831aca 100644 --- a/case-study.md +++ b/case-study.md @@ -49,7 +49,13 @@ Ваша находка №2 - Отчет bullet показал что в контроллере не хватает eager loading - Добавил .includes(bus: [:services]) -- Время ответа контроллера уменьшилось с 25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms), +- Время ответа сервера уменьшилось с 25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms), до 22393ms (Views: 22124.9ms | ActiveRecord: 240.2ms) - Bullet больше не показывает ворнингов +Ваша находка №3 +- Rails panel показал,что на рендеринг уходит около 20 секунд +- Перенес все паршиалы в одну вьюху +- Время ответа сервера уменьшилось до 1445ms (Views: 1245.1ms | ActiveRecord: 188.4ms) +- Время рендера вьюхи сократилось до 1,2 с + diff --git a/config/environments/development.rb b/config/environments/development.rb index 90792c10..72843164 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -1,6 +1,6 @@ Rails.application.configure do config.after_initialize do - Bullet.enable = true + Bullet.enable = false Bullet.alert = true Bullet.bullet_logger = true Bullet.console = true From e7dc88ad1cdf6f358dea5d6ec335909f5823ab34 Mon Sep 17 00:00:00 2001 From: "d.bukholtsev" Date: Tue, 4 May 2021 22:20:15 +1000 Subject: [PATCH 5/6] =?UTF-8?q?=D0=98=D0=BD=D0=B4=D0=B5=D0=BA=D1=81=D1=8B?= =?UTF-8?q?=20=D0=BE=D1=82=20PgHero?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Gemfile | 2 ++ Gemfile.lock | 7 +++++++ case-study.md | 5 +++++ config/routes.rb | 2 ++ db/migrate/20210504121026_add_indices_to_trips.rb | 8 ++++++++ db/schema.rb | 4 +++- 6 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20210504121026_add_indices_to_trips.rb diff --git a/Gemfile b/Gemfile index a25ad5ca..be3008bd 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,8 @@ gem 'benchmark-ips' gem 'kalibera' gem 'activerecord-import' gem 'bullet' +gem 'pghero' +gem 'pg_query' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index 3dfb5212..7f60e8b0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -64,6 +64,7 @@ GEM ffi (1.10.0) globalid (0.4.2) activesupport (>= 4.2.0) + google-protobuf (3.15.8) i18n (1.6.0) concurrent-ruby (~> 1.0) kalibera (0.1.1) @@ -96,6 +97,10 @@ GEM nokogiri (1.10.2) mini_portile2 (~> 2.4.0) pg (1.1.4) + pg_query (2.0.3) + google-protobuf (~> 3.15.5) + pghero (2.8.1) + activerecord (>= 5) puma (3.12.1) rack (2.0.6) rack-contrib (2.3.0) @@ -196,6 +201,8 @@ DEPENDENCIES listen (>= 3.0.5, < 3.2) meta_request pg (>= 0.18, < 2.0) + pg_query + pghero puma (~> 3.11) rails (~> 5.2.3) rails-controller-testing diff --git a/case-study.md b/case-study.md index e9831aca..f74cda00 100644 --- a/case-study.md +++ b/case-study.md @@ -59,3 +59,8 @@ - Время ответа сервера уменьшилось до 1445ms (Views: 1245.1ms | ActiveRecord: 188.4ms) - Время рендера вьюхи сократилось до 1,2 с +Ваша находка №4 +- PGHero предлагает построить индексы для 3 самых долгих запросов +- Построил эти индексы +- Время ответа сервера изменилось: 1524ms (Views: 1473.3ms | ActiveRecord: 36.5ms) +- PGHero больше не предлагает строить индексы diff --git a/config/routes.rb b/config/routes.rb index a2da6a7b..92916b51 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,4 +2,6 @@ # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html get "/" => "statistics#index" get "автобусы/:from/:to" => "trips#index" + + mount PgHero::Engine, at: "pghero" end diff --git a/db/migrate/20210504121026_add_indices_to_trips.rb b/db/migrate/20210504121026_add_indices_to_trips.rb new file mode 100644 index 00000000..bef41e8b --- /dev/null +++ b/db/migrate/20210504121026_add_indices_to_trips.rb @@ -0,0 +1,8 @@ +class AddIndicesToTrips < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :trips, %i[from_id to_id], algorithm: :concurrently + add_index :buses_services, :bus_id, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index f6921e45..a8e8f1b4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_03_30_193044) do +ActiveRecord::Schema.define(version: 2021_05_04_121026) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -23,6 +23,7 @@ create_table "buses_services", force: :cascade do |t| t.integer "bus_id" t.integer "service_id" + t.index ["bus_id"], name: "index_buses_services_on_bus_id" end create_table "cities", force: :cascade do |t| @@ -40,6 +41,7 @@ t.integer "duration_minutes" t.integer "price_cents" t.integer "bus_id" + t.index ["from_id", "to_id"], name: "index_trips_on_from_id_and_to_id" end end From 091c44747b1494541870f18d961515777a00ddb1 Mon Sep 17 00:00:00 2001 From: "d.bukholtsev" Date: Tue, 4 May 2021 23:03:10 +1000 Subject: [PATCH 6/6] =?UTF-8?q?=D0=A1=D1=87=D0=B5=D1=82=D1=87=D0=B8=D0=BA?= =?UTF-8?q?=20=D1=80=D0=B5=D0=B9=D1=81=D0=BE=D0=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Gemfile | 1 + Gemfile.lock | 3 +++ app/views/trips/index.html.erb | 2 +- case-study.md | 35 +++++++++++++++++++++++++--------- 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/Gemfile b/Gemfile index be3008bd..f456c3ea 100644 --- a/Gemfile +++ b/Gemfile @@ -24,6 +24,7 @@ group :development do gem 'web-console', '>= 3.3.0' gem 'listen', '>= 3.0.5', '< 3.2' gem 'meta_request' + gem 'rack-mini-profiler' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 7f60e8b0..741ac042 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -105,6 +105,8 @@ GEM rack (2.0.6) rack-contrib (2.3.0) rack (~> 2.0) + rack-mini-profiler (2.3.2) + rack (>= 1.2.0) rack-test (1.1.0) rack (>= 1.0, < 3) rails (5.2.3) @@ -204,6 +206,7 @@ DEPENDENCIES pg_query pghero puma (~> 3.11) + rack-mini-profiler rails (~> 5.2.3) rails-controller-testing rspec diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index d9d293e0..9cccbae2 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -2,7 +2,7 @@ <%= "Автобусы #{@from.name} – #{@to.name}" %>

    - <%= "В расписании #{@trips.count} рейсов" %> + <%= "В расписании #{@trips.load.size} рейсов" %>

    <% @trips.each do |trip| %> diff --git a/case-study.md b/case-study.md index f74cda00..6f8f6164 100644 --- a/case-study.md +++ b/case-study.md @@ -3,7 +3,7 @@ ## Актуальная проблема В нашем проекте возникли 2 проблемы. 1) Необходимо было импортировать в базу файл с данными, чуть больше 32 мегабайт (100К записей). - Рейк таск успешно работал на файлах размером пару мегабайт, но c большими файлами он работала слишком долго. + Рейк таск успешно работал на файлах размером пару мегабайт, но c большими файлами он работал слишком долго. Я решил исправить эту проблему, оптимизировав этот скрипт. 2) Страницы расписаний тоже формируются не эффективно и при росте количества записей начинают сильно тормозить. @@ -25,8 +25,8 @@ Вот как я построил `feedback_loop`: - написал тесты для проверки корректности работы рейк таски/контроллера -- измерил метрику: для рейк таски с example.json (10К записей) - 200 мс, - для контроллера с записями из large.json - 25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms) +- измерил метрику: время выполнения рейк таски с example.json (10 записей) - 200 мс, + время ответа для контроллера с записями из large.json (1000 рейсов) - `25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms)` - написал performance тесты для защиты от деградации производительности - далее использовал эти же входные данные для дальнейшего профилирования и сравнения метрик @@ -35,7 +35,7 @@ - rack-mini-profiler - rails panel - bullet -- explain запросов +- PgHero Вот какие проблемы удалось найти и решить: @@ -48,19 +48,36 @@ Ваша находка №2 - Отчет bullet показал что в контроллере не хватает eager loading -- Добавил .includes(bus: [:services]) -- Время ответа сервера уменьшилось с 25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms), - до 22393ms (Views: 22124.9ms | ActiveRecord: 240.2ms) +- Добавил `.includes(bus: [:services])` +- Время ответа сервера уменьшилось с + + `25548ms (Views: 23577.9ms | ActiveRecord: 1962.8ms)`, до + + `22393ms (Views: 22124.9ms | ActiveRecord: 240.2ms)` - Bullet больше не показывает ворнингов Ваша находка №3 - Rails panel показал,что на рендеринг уходит около 20 секунд - Перенес все паршиалы в одну вьюху -- Время ответа сервера уменьшилось до 1445ms (Views: 1245.1ms | ActiveRecord: 188.4ms) +- Время ответа сервера уменьшилось до `1445ms (Views: 1245.1ms | ActiveRecord: 188.4ms)` - Время рендера вьюхи сократилось до 1,2 с Ваша находка №4 - PGHero предлагает построить индексы для 3 самых долгих запросов - Построил эти индексы -- Время ответа сервера изменилось: 1524ms (Views: 1473.3ms | ActiveRecord: 36.5ms) +- Время ответа сервера почти не изменилось: `1524ms (Views: 1473.3ms | ActiveRecord: 36.5ms)` - PGHero больше не предлагает строить индексы + +Ваша находка №5 +- rake-mini-profiler показал, что вызывается лишний sql запрос для количества рейсов +- Заменил `@trips.count` на `@trips.load.size` во вьюхе +- Время ответа сервера уменьшилось до `1329ms (Views: 1232.9ms | ActiveRecord: 58.5ms)` +- В отчете rake-mini-profiler на один sql запрос меньше + +## Результаты +В результате проделанной оптимизации удалось улучшить метрику импорта 100К записей до 47 секунд и +уложиться в заданный бюджет. +Также удалось уменьшить время ответа сервера для 1000 рейсов с 25 секунд до 1,3 секунды. + +## Защита от регрессии производительности +Для защиты от потери достигнутого прогресса при дальнейших изменениях были написаны performance-тесты на время выполнения.