From fd302e712dd2b1f40c2152d4357ee962ccdf5d23 Mon Sep 17 00:00:00 2001 From: Araslanov Evgeny Date: Thu, 13 Feb 2025 22:30:42 +0500 Subject: [PATCH 1/2] Step 1 --- .gitignore | 1 + .rspec | 2 + .tool-versions | 1 + Gemfile | 6 ++ Gemfile.lock | 38 +++++++++++ case-study.md | 54 ++++++++++++++++ lib/tasks/utils.rake | 46 ++++++++++---- spec/rails_helper.rb | 81 ++++++++++++++++++++++++ spec/spec_helper.rb | 94 ++++++++++++++++++++++++++++ spec/tasks/utils_spec.rb | 36 +++++++++++ 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, 347 insertions(+), 27 deletions(-) create mode 100644 .rspec create mode 100644 .tool-versions create mode 100644 case-study.md create mode 100644 spec/rails_helper.rb create mode 100644 spec/spec_helper.rb create mode 100644 spec/tasks/utils_spec.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/.gitignore b/.gitignore index 59c74047..66132f96 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /tmp /log /public +.DS_Store \ No newline at end of file diff --git a/.rspec b/.rspec new file mode 100644 index 00000000..c584b5c7 --- /dev/null +++ b/.rspec @@ -0,0 +1,2 @@ +--require rails_helper +--color \ No newline at end of file diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 00000000..041df9aa --- /dev/null +++ b/.tool-versions @@ -0,0 +1 @@ +ruby 3.4.1 diff --git a/Gemfile b/Gemfile index 34074dfd..341aa1a3 100644 --- a/Gemfile +++ b/Gemfile @@ -12,3 +12,9 @@ gem 'rack-mini-profiler' # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] + +group :development, :test do + gem 'rspec-rails', '~> 7.0.0' + gem 'rspec-benchmark' + gem "database_cleaner-active_record" +end \ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock index a9ddd818..fdc1ed7f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -74,6 +74,9 @@ GEM uri (>= 0.13.1) base64 (0.2.0) benchmark (0.4.0) + benchmark-malloc (0.2.0) + benchmark-perf (0.6.0) + benchmark-trend (0.4.0) bigdecimal (3.1.9) bootsnap (1.18.4) msgpack (~> 1.2) @@ -81,7 +84,12 @@ GEM concurrent-ruby (1.3.5) connection_pool (2.5.0) crass (1.0.6) + database_cleaner-active_record (2.2.0) + activerecord (>= 5.a) + database_cleaner-core (~> 2.0.0) + database_cleaner-core (2.0.1) date (3.4.1) + diff-lcs (1.6.0) drb (2.2.1) erubi (1.13.1) ffi (1.17.1-arm64-darwin) @@ -179,6 +187,32 @@ GEM psych (>= 4.0.0) reline (0.6.0) io-console (~> 0.5) + rspec (3.13.0) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-benchmark (0.6.0) + benchmark-malloc (~> 0.2) + benchmark-perf (~> 0.6) + benchmark-trend (~> 0.4) + rspec (>= 3.0) + rspec-core (3.13.3) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.3) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.2) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-rails (7.0.2) + actionpack (>= 7.0) + activesupport (>= 7.0) + railties (>= 7.0) + rspec-core (~> 3.13) + rspec-expectations (~> 3.13) + rspec-mocks (~> 3.13) + rspec-support (~> 3.13) + rspec-support (3.13.2) securerandom (0.4.1) stringio (3.1.2) thor (1.3.2) @@ -194,15 +228,19 @@ GEM zeitwerk (2.7.1) PLATFORMS + arm64-darwin-23 arm64-darwin-24 DEPENDENCIES bootsnap + database_cleaner-active_record listen pg puma rack-mini-profiler rails (~> 8.0.1) + rspec-benchmark + rspec-rails (~> 7.0.0) tzinfo-data RUBY VERSION diff --git a/case-study.md b/case-study.md new file mode 100644 index 00000000..7cbffc84 --- /dev/null +++ b/case-study.md @@ -0,0 +1,54 @@ +# Case-study оптимизации + +## Актуальные проблемы +В проекте возникли несколько серьёзных проблем. +- Долгий импорт данных, при объеме данных более 30 мегабайт +- При большом объеме данных начинает тормозить страница расписаний. + +### Импорт данных +В приложении есть rake таска, которая удаляет все ранее загруженные данные, и добавляет новые из предоставленного файла. +``` +bin/rake reload_json[file] +``` +Она успешно работала на файлах размером пару мегабайт, но для большого файла она работала слишком долго. +Я решил исправить эту проблему, оптимизировав эту программу. + +#### Формирование метрики +Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я добавил вывод времени выполнения программы (определяю временную метку в начале выполнения и в конце, и смотрю разницу). +При первом запуске программы с medium.json файлом она отработала за ~ 1 минуту. + +#### Гарантия корректности работы оптимизированной программы +Для гарантии был добавлен тест при который в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. +``` + bundle exec rspec spec/tasks/utils_spec.rb +``` + +#### Профилирование +Чтобы понять какие проблемы с программой, я решил использовать логи, которые записываются в `log/development.log`. + +#### Поиск проблем +После первого запуска программы, логи показали очень большое количество SELECT запросов для таблиц cities, services и buses, это связано было с тем, что перед созданием записи мы проверяли наличие существующей записи. +Было решено добавить HASH переменные, в которые можно было бы по ключу добавлять записи, чтобы при необходимости получать нужную запись по ключу. +После этого программа для medium.json файла стала отрабатывать за ~ 12 секунд. + +С large.json программа отрабатывала за ~ 1 минуту. +Логи показали на большое количество INSERT trips, потому что записи создавались по отдельности. Было решено использовать метод upsert_all, который добавляет записи одним запросом. +После этого программа для large.json файла стала отрабатывать за ~ 7 секунд. + +Логи показали что осталисось много INSERT INTO "buses_services", но их пока не понятно как загружать +``` + Bulk insert or upsert is currently not supported for has_many through association +``` + +#### Результаты +В результате проделанной оптимизации наконец удалось обработать файл с данными. +Удалось улучшить метрику с более 1 минуты, до примерно 7 секунд и уложиться в заданный бюджет. + +#### Защита от регрессии производительности +Для защиты от потери достигнутого прогресса при дальнейших изменениях программы был добавлен тест. +``` + bundle exec rspec spec/tasks/utils_spec.rb +``` + +### Отображение расписаний +... в процессе \ No newline at end of file diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe871..f9799d4e 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,7 +1,10 @@ # Наивная загрузка данных из json-файла в БД # rake reload_json[fixtures/small.json] task :reload_json, [:file_name] => :environment do |_task, args| + start_time = Time.current + puts "Start at #{start_time}" json = JSON.parse(File.read(args.file_name)) + puts "Parsed at #{Time.current}" ActiveRecord::Base.transaction do City.delete_all @@ -10,25 +13,44 @@ task :reload_json, [:file_name] => :environment do |_task, args| Trip.delete_all ActiveRecord::Base.connection.execute('delete from buses_services;') + cities = {} + all_services = {} + buses = {} + trips = [] + json.each do |trip| - from = City.find_or_create_by(name: trip['from']) - to = City.find_or_create_by(name: trip['to']) + # NOTE! можно хранить только id + cities[trip['from']] ||= City.create(name: trip['from']) + from = cities[trip['from']] + + cities[trip['to']] ||= City.create(name: trip['to']) + to = cities[trip['to']] + services = [] trip['bus']['services'].each do |service| - s = Service.find_or_create_by(name: service) - services << s + all_services[service] ||= Service.create(name: service) + services << all_services[service] 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, + buses[trip['bus']['number']] ||= Bus.create(number: trip['bus']['number']) do |b| + b.model = trip['bus']['model'] + b.services = services + end + bus = buses[trip['bus']['number']] + + trips << { + from_id: from.id, + to_id: to.id, + bus_id: bus.id, start_time: trip['start_time'], duration_minutes: trip['duration_minutes'], - price_cents: trip['price_cents'], - ) + price_cents: trip['price_cents'] + } end + + Trip.upsert_all(trips) end + end_time = Time.current + puts "End at #{end_time}" + puts "Таска выполнена за #{end_time - start_time} секунд" end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb new file mode 100644 index 00000000..58ead80e --- /dev/null +++ b/spec/rails_helper.rb @@ -0,0 +1,81 @@ +# This file is copied to spec/ when you run 'rails generate rspec:install' +require 'spec_helper' +ENV['RAILS_ENV'] ||= 'test' +require_relative '../config/environment' +# Prevent database truncation if the environment is production +abort("The Rails environment is running in production mode!") if Rails.env.production? +# Uncomment the line below in case you have `--require rails_helper` in the `.rspec` file +# that will avoid rails generators crashing because migrations haven't been run yet +# return unless Rails.env.test? +require 'rspec/rails' +# 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. +# +# Rails.root.glob('spec/support/**/*.rb').sort_by(&:to_s).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 + abort e.to_s.strip +end +RSpec.configure do |config| + config.include RSpec::Benchmark::Matchers + + # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures + config.fixture_paths = [ + Rails.root.join('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://rspec.info/features/7-0/rspec-rails + 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") + + config.before(:suite) do + DatabaseCleaner.strategy = :transaction + DatabaseCleaner.clean_with(:truncation) + end + + config.around(:each) do |example| + DatabaseCleaner.cleaning do + example.run + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 00000000..327b58ea --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,94 @@ +# 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 https://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: + # https://rspec.info/features/3-12/rspec-core/configuration/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 diff --git a/spec/tasks/utils_spec.rb b/spec/tasks/utils_spec.rb new file mode 100644 index 00000000..517d2068 --- /dev/null +++ b/spec/tasks/utils_spec.rb @@ -0,0 +1,36 @@ +require 'rake' + +RSpec.describe 'reload_json', type: :task do + let(:task_name) { 'reload_json' } + let(:file) { "fixtures/example.json" } + + before do + Rake.application.rake_require('tasks/utils') + Rake::Task.define_task(:environment) + end + + after do + Rake::Task[task_name].reenable + end + + subject(:task) { Rake::Task[task_name] } + + it "creates cities, buses and trips" do + expect { + task.invoke(file) + }.to change { City.count }.by(2).and( + change { Service.count }.by(2).and( + change { Bus.count }.by(1).and( + change { Trip.count }.by(10).and( + change { Trip.includes(:from, :to).where(from: { name: "Москва" }, to: { name: "Самара" }).count }.by(5))))) + end + + context "performance" do + let(:file) { "fixtures/large.json" } + + it 'works with large data under 10 sec' do + expect(File.size(file)).to be > 30 * 1024 * 1024 # 30 МБ + expect { task.invoke(file) }.to perform_under(10).sec + end + 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 834935d0e44334da77f6c95e26a6f815fef04816 Mon Sep 17 00:00:00 2001 From: Araslanov Evgeny Date: Sat, 15 Feb 2025 00:05:49 +0500 Subject: [PATCH 2/2] Step 2 --- Gemfile | 5 +++ Gemfile.lock | 23 +++++++++++++ app/controllers/trips_controller.rb | 2 +- app/views/trips/_service.html.erb | 1 - app/views/trips/_services.html.erb | 6 ---- app/views/trips/index.html.erb | 7 +++- case-study.md | 33 +++++++++++++++++-- config/initializers/assets.rb | 2 +- config/routes.rb | 1 + ...0250213181526_create_pghero_query_stats.rb | 15 +++++++++ db/schema.rb | 28 +++++++++++----- spec/controllers/trips_controller_spec.rb | 32 ++++++++++++++++++ spec/factories/buses.rb | 10 ++++++ spec/factories/cities.rb | 9 +++++ spec/factories/services.rb | 9 +++++ spec/factories/trips.rb | 10 ++++++ spec/rails_helper.rb | 1 + 17 files changed, 173 insertions(+), 21 deletions(-) delete mode 100644 app/views/trips/_service.html.erb delete mode 100644 app/views/trips/_services.html.erb create mode 100644 db/migrate/20250213181526_create_pghero_query_stats.rb create mode 100644 spec/controllers/trips_controller_spec.rb create mode 100644 spec/factories/buses.rb create mode 100644 spec/factories/cities.rb create mode 100644 spec/factories/services.rb create mode 100644 spec/factories/trips.rb diff --git a/Gemfile b/Gemfile index 341aa1a3..743cedcd 100644 --- a/Gemfile +++ b/Gemfile @@ -4,12 +4,16 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } ruby file: '.ruby-version' gem 'rails', '~> 8.0.1' +gem "sprockets-rails" gem 'pg' gem 'puma' gem 'listen' gem 'bootsnap' gem 'rack-mini-profiler' +gem "pghero" +gem "pg_query" + # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] @@ -17,4 +21,5 @@ group :development, :test do gem 'rspec-rails', '~> 7.0.0' gem 'rspec-benchmark' gem "database_cleaner-active_record" + gem 'factory_bot_rails' end \ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock index fdc1ed7f..72b8c84d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,9 +92,17 @@ GEM diff-lcs (1.6.0) drb (2.2.1) erubi (1.13.1) + factory_bot (6.5.1) + activesupport (>= 6.1.0) + factory_bot_rails (6.4.4) + factory_bot (~> 6.5) + railties (>= 5.0.0) ffi (1.17.1-arm64-darwin) globalid (1.2.1) activesupport (>= 6.1) + google-protobuf (4.29.3-arm64-darwin) + bigdecimal + rake (>= 13) i18n (1.14.7) concurrent-ruby (~> 1.0) io-console (0.8.0) @@ -131,6 +139,10 @@ GEM nokogiri (1.18.2-arm64-darwin) racc (~> 1.4) pg (1.5.9) + pg_query (6.0.0) + google-protobuf (>= 3.25.3) + pghero (3.6.1) + activerecord (>= 6.1) pp (0.6.2) prettyprint prettyprint (0.2.0) @@ -214,6 +226,13 @@ GEM rspec-support (~> 3.13) rspec-support (3.13.2) securerandom (0.4.1) + sprockets (4.2.1) + concurrent-ruby (~> 1.0) + rack (>= 2.2.4, < 4) + sprockets-rails (3.5.2) + actionpack (>= 6.1) + activesupport (>= 6.1) + sprockets (>= 3.0.0) stringio (3.1.2) thor (1.3.2) timeout (0.4.3) @@ -234,13 +253,17 @@ PLATFORMS DEPENDENCIES bootsnap database_cleaner-active_record + factory_bot_rails listen pg + pg_query + pghero puma rack-mini-profiler rails (~> 8.0.1) rspec-benchmark rspec-rails (~> 7.0.0) + sprockets-rails tzinfo-data RUBY VERSION diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be2..1431925c 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.includes(bus: :services).where(from: @from, to: @to).order(:start_time) end end 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 @@ -
  • Сервисы в автобусе:
    • - diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce41..5bd8e51b 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -9,7 +9,12 @@ <%= render "delimiter" %> diff --git a/case-study.md b/case-study.md index 7cbffc84..35ff699d 100644 --- a/case-study.md +++ b/case-study.md @@ -18,7 +18,7 @@ bin/rake reload_json[file] При первом запуске программы с medium.json файлом она отработала за ~ 1 минуту. #### Гарантия корректности работы оптимизированной программы -Для гарантии был добавлен тест при который в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. +Для гарантии был добавлен тест, который в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. ``` bundle exec rspec spec/tasks/utils_spec.rb ``` @@ -51,4 +51,33 @@ bin/rake reload_json[file] ``` ### Отображение расписаний -... в процессе \ No newline at end of file +Веб приложение отображает страницу расписания автобусов по направлениям. Она быстро отвечает при небольшом количестве данных, но при большом количестве данных начинаеат долго загружаться. Я решил исправить эту проблему оптимизировав загрузку страницы. +Для того, чтобы иметь возможность быстро проверять гипотезы я решил загрузить большое количество данных, найти самый популярный маршрут и посмотеть за сколько страница загружается. + +#### Формирование метрики +Стандартный набор в браузере - панель разработчиков, в разделе Network, определять за сколько загружается страница. Дополнительно я добавил гем "mini-profiler-resources" который так же показывает время загрузки страницы. +Я загрузил данные из large.json файла и открыл страницу самого популярного маршрута Волгоград – Рыбинск, 1095 рейсов, страница грузилась ~ 8 секунд. Считается что лучшем временем для загрузки страницы является менее 2 секунд. + +#### Гарантия корректности работы оптимизированной страницы +Для гарантии был добавлен тест, который в фидбек-лупе позволяет не допустить ошибок при оптимизации. +``` + bundle exec rspec spec/controllers/trips_controller_spec.rb +``` + +#### Профилирование +Чтобы понимать какие возможны проблемы на странице я добавил "mini-profiler-resources", который показывает что происходит при загрузке страницы. Дополнительно я установил pghero, чтобы анализировать sql запросы. + +#### Поиск проблем +Используя mini-profiler-resources я обнаруж что на странице делается 1975 sql запросов. Pghero так же показал что есть 2 запроса, которые делаюся более 1900 раз - Автобусы и Сервисы автобусов. Предполагаю что проблема на странице N+1 проблема. Для исправления добавил includes к Trip, для того чтобы формировались несколько запросов на все данные. После этого страница стала грузится за ~2.5 секунды, а количество sql запросов стало 5. + +После mini-profiler-resources показал что на странице очень много рендрерится шаблонов - списка услуг и сами услуги. Я решил избавится от шаблонов и перенести всё основной шаблон. После чего страница стала грузится за 0,6 секунд + +#### Результаты +В результате проделанной оптимизации страница стала грузиться быстрее. +Удалось улучшить метрику с более 8 секунд, до менее 1 секунды и уложиться в заданный бюджет. + +#### Защита от регрессии производительности +Для защиты от потери достигнутого прогресса при дальнейших изменениях был добавлен тест. +``` + bundle exec rspec spec/controllers/trips_controller_spec.rb +``` \ No newline at end of file diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb index 49a8c192..6f4b4599 100644 --- a/config/initializers/assets.rb +++ b/config/initializers/assets.rb @@ -1,7 +1,7 @@ # Be sure to restart your server when you modify this file. # Version of your assets, change this if you want to expire all your assets. -# Rails.application.config.assets.version = '1.0' +Rails.application.config.assets.version = '1.0' # Add additional assets to the asset load path. # Rails.application.config.assets.paths << Emoji.images_path diff --git a/config/routes.rb b/config/routes.rb index 0bbefa7a..d678b101 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,5 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html + mount PgHero::Engine, at: "pghero" get "автобусы/:from/:to" => "trips#index" end diff --git a/db/migrate/20250213181526_create_pghero_query_stats.rb b/db/migrate/20250213181526_create_pghero_query_stats.rb new file mode 100644 index 00000000..74aaaa9a --- /dev/null +++ b/db/migrate/20250213181526_create_pghero_query_stats.rb @@ -0,0 +1,15 @@ +class CreatePgheroQueryStats < ActiveRecord::Migration[8.0] + def change + create_table :pghero_query_stats do |t| + t.text :database + t.text :user + t.text :query + t.integer :query_hash, limit: 8 + t.float :total_time + t.integer :calls, limit: 8 + t.timestamp :captured_at + end + + add_index :pghero_query_stats, [:database, :captured_at] + end +end diff --git a/db/schema.rb b/db/schema.rb index f6921e45..0486033f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2,18 +2,18 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # 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[8.0].define(version: 2025_02_13_181526) do # These are extensions that must be enabled in order to support this database - enable_extension "plpgsql" + enable_extension "pg_catalog.plpgsql" + enable_extension "pg_stat_statements" create_table "buses", force: :cascade do |t| t.string "number" @@ -29,6 +29,17 @@ t.string "name" end + create_table "pghero_query_stats", force: :cascade do |t| + t.text "database" + t.text "user" + t.text "query" + t.bigint "query_hash" + t.float "total_time" + t.bigint "calls" + t.datetime "captured_at", precision: nil + t.index ["database", "captured_at"], name: "index_pghero_query_stats_on_database_and_captured_at" + end + create_table "services", force: :cascade do |t| t.string "name" end @@ -41,5 +52,4 @@ t.integer "price_cents" t.integer "bus_id" end - end diff --git a/spec/controllers/trips_controller_spec.rb b/spec/controllers/trips_controller_spec.rb new file mode 100644 index 00000000..8ca22e05 --- /dev/null +++ b/spec/controllers/trips_controller_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +RSpec.describe TripsController do + render_views + + describe "GET #index" do + let!(:from) { create :city } + let!(:to) { create :city } + + before do + create_list(:trip, 10, from:, to:) + end + + subject(:perform) { get :index, params: { from: from.name, to: to.name } } + + it { is_expected.to be_successful } + + context "performance" do + before do + create_list(:trip, 2000, from:, to:) + end + + it "works with large data under 1 sec" do + time = Benchmark.realtime do + perform + end + + expect(time).to be < 1 + end + end + end +end \ No newline at end of file diff --git a/spec/factories/buses.rb b/spec/factories/buses.rb new file mode 100644 index 00000000..5ec63bb5 --- /dev/null +++ b/spec/factories/buses.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :bus do + initialize_with do + Bus.find_or_create_by(number:) + end + + number { rand(10000).to_s } + model { Bus::MODELS.sample } + end +end \ No newline at end of file diff --git a/spec/factories/cities.rb b/spec/factories/cities.rb new file mode 100644 index 00000000..4315dd04 --- /dev/null +++ b/spec/factories/cities.rb @@ -0,0 +1,9 @@ +FactoryBot.define do + factory :city, aliases: [:from, :to] do + initialize_with do + City.find_or_create_by(name:) + end + + name { ['Тюмень', 'Тобольск', 'Екатеринбург'].sample } + end +end \ No newline at end of file diff --git a/spec/factories/services.rb b/spec/factories/services.rb new file mode 100644 index 00000000..bd19b76a --- /dev/null +++ b/spec/factories/services.rb @@ -0,0 +1,9 @@ +FactoryBot.define do + factory :service do + initialize_with do + Service.find_or_create_by(name:) + end + + name { Service::SERVICES.sample } + end +end \ No newline at end of file diff --git a/spec/factories/trips.rb b/spec/factories/trips.rb new file mode 100644 index 00000000..5047133a --- /dev/null +++ b/spec/factories/trips.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :trip do + from + to + bus + start_time { "09:00" } + duration_minutes { rand(1000) + 1 } + price_cents { rand(100000) } + end +end \ No newline at end of file diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 58ead80e..5e1495f7 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -34,6 +34,7 @@ end RSpec.configure do |config| config.include RSpec::Benchmark::Matchers + config.include FactoryBot::Syntax::Methods # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures config.fixture_paths = [