diff --git a/.gitignore b/.gitignore index 59c74047..e5501c85 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /tmp /log /public +.byebug_history \ No newline at end of file diff --git a/.ruby-version b/.ruby-version index ec1cf33c..49cdd668 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.6.3 +2.7.6 diff --git a/Gemfile b/Gemfile index e20b1260..04b96d28 100644 --- a/Gemfile +++ b/Gemfile @@ -1,26 +1,49 @@ +# frozen_string_literal: true + source 'https://rubygems.org' git_source(:github) { |repo| "https://github.com/#{repo}.git" } -ruby '2.6.3' +ruby '2.7.6' gem 'rails', '~> 5.2.3' + gem 'pg', '>= 0.18', '< 2.0' gem 'puma', '~> 3.11' + gem 'bootsnap', '>= 1.1.0', require: false +gem 'mimemagic', '~> 0.3.10' + +gem 'activerecord-import' + +gem 'fast_jsonparser' +gem 'oj' + +gem 'strong_migrations' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console - gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] + gem 'benchmark', '~> 0.2.1' + gem 'byebug', platforms: %i[mri mingw x64_mingw] + gem 'rspec-benchmark', '~> 0.6.0' + # gem 'rspec-rails', '~> 5.0.0' + + gem 'bullet' + gem 'pghero' + gem 'pg_query', '>= 2' + + gem 'memory_profiler' + gem 'rack-mini-profiler', require: false + gem 'stackprof' end 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 'web-console', '>= 3.3.0' end group :test do end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem -gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] +gem 'tzinfo-data', platforms: %i[mingw mswin x64_mingw jruby] diff --git a/Gemfile.lock b/Gemfile.lock index fccf6f5f..f1abd26e 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.4.1) + activerecord (>= 4.2) activestorage (5.2.3) actionpack (= 5.2.3) activerecord (= 5.2.3) @@ -43,17 +45,27 @@ GEM minitest (~> 5.1) tzinfo (~> 1.1) arel (9.0.0) + benchmark (0.2.1) + 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) + bootsnap (1.16.0) + msgpack (~> 1.2) builder (3.2.3) + bullet (7.0.7) + activesupport (>= 3.0.0) + uniform_notifier (~> 1.11) byebug (11.0.1) concurrent-ruby (1.1.5) crass (1.0.4) + diff-lcs (1.5.0) erubi (1.8.0) - ffi (1.10.0) + fast_jsonparser (0.6.0) + ffi (1.15.5) globalid (0.4.2) activesupport (>= 4.2.0) + google-protobuf (3.22.2) i18n (1.6.0) concurrent-ruby (~> 1.0) listen (3.1.5) @@ -67,18 +79,28 @@ GEM mini_mime (>= 0.1.1) marcel (0.3.3) mimemagic (~> 0.3.2) + memory_profiler (1.0.1) 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) - msgpack (1.2.9) + msgpack (1.6.1) nio4r (2.3.1) nokogiri (1.10.2) mini_portile2 (~> 2.4.0) + oj (3.14.2) pg (1.1.4) + pg_query (4.2.0) + google-protobuf (>= 3.19.2) + pghero (2.8.3) + activerecord (>= 5) puma (3.12.1) rack (2.0.6) + rack-mini-profiler (3.0.0) + rack (>= 1.2.0) rack-test (1.1.0) rack (>= 1.0, < 3) rails (5.2.3) @@ -109,6 +131,24 @@ GEM rb-fsevent (0.10.3) rb-inotify (0.10.0) ffi (~> 1.0) + rspec (3.12.0) + rspec-core (~> 3.12.0) + rspec-expectations (~> 3.12.0) + rspec-mocks (~> 3.12.0) + rspec-benchmark (0.6.0) + benchmark-malloc (~> 0.2) + benchmark-perf (~> 0.6) + benchmark-trend (~> 0.4) + rspec (>= 3.0) + rspec-core (3.12.1) + rspec-support (~> 3.12.0) + rspec-expectations (3.12.2) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.12.0) + rspec-mocks (3.12.4) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.12.0) + rspec-support (3.12.0) ruby_dep (1.5.0) sprockets (3.7.2) concurrent-ruby (~> 1.0) @@ -117,10 +157,14 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) + stackprof (0.2.24) + strong_migrations (1.4.4) + activerecord (>= 5.2) thor (0.20.3) thread_safe (0.3.6) tzinfo (1.2.5) thread_safe (~> 0.1) + uniform_notifier (1.16.0) web-console (3.7.0) actionview (>= 5.0) activemodel (>= 5.0) @@ -134,17 +178,30 @@ PLATFORMS ruby DEPENDENCIES + activerecord-import + benchmark (~> 0.2.1) bootsnap (>= 1.1.0) + bullet byebug + fast_jsonparser listen (>= 3.0.5, < 3.2) + memory_profiler + mimemagic (~> 0.3.10) + oj pg (>= 0.18, < 2.0) + pg_query (>= 2) + pghero puma (~> 3.11) + rack-mini-profiler rails (~> 5.2.3) + rspec-benchmark (~> 0.6.0) + stackprof + strong_migrations tzinfo-data web-console (>= 3.3.0) RUBY VERSION - ruby 2.6.3p62 + ruby 2.7.6p219 BUNDLED WITH - 2.0.2 + 2.3.24 diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..f85f8cac --- /dev/null +++ b/Makefile @@ -0,0 +1,7 @@ +task: + bundle exec rake reload_json[fixtures/large.json] + +test: + bundle exec rails test + +.PHONY: test diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be2..09a9811e 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -2,6 +2,8 @@ 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.preload(bus: :services) + .where(from: @from, to: @to) + .order(:start_time) end end diff --git a/app/models/bus.rb b/app/models/bus.rb index 1dcc54cb..d97de31f 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_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..6219d44e --- /dev/null +++ b/app/models/buses_service.rb @@ -0,0 +1,4 @@ +class BusesService < ApplicationRecord + belongs_to :bus + belongs_to :service +end diff --git a/app/models/service.rb b/app/models/service.rb index 9cbb2a32..1781543c 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_many :buses, through: :buses_services validates :name, presence: true validates :name, inclusion: { in: SERVICES } diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce41..ed64df61 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -2,15 +2,24 @@ <%= "Автобусы #{@from.name} – #{@to.name}" %>

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

<% @trips.each do |trip| %> - <%= render "delimiter" %> + ==================================================== <% end %> diff --git a/case-study-template.md b/case-study-template.md new file mode 100644 index 00000000..500e977e --- /dev/null +++ b/case-study-template.md @@ -0,0 +1,78 @@ +# Case-study оптимизации + +## Актуальная проблема +1. Долгий импорт данных в БД +2. Долгое отображение расписаний + +## Формирование метрики +Mетрика: *время импорта данных в БД в секундах, расчитываемое на файле `large.json`.* +Бюджет: 60 секунд. + +## Гарантия корректности работы оптимизированной программы +Тест соответствия скорости импорта бюджету +Тест корректности импорта (`fixtures/example.json`) + +## Вникаем в детали системы, чтобы найти главные точки роста +Стартовый benchmark: +# small.json - 4.13s +# medium.json - 57.1s + +### Неоптимальная запись в БД +Использование гема activerecord-import: +small.json - 2.26s +medium.json - 18.78s + +Забавно, что использование гема oj для загрузки json дало обратный результат: +small.json - 2.31s +medium.json - 19.25s +large.json - 193.09s +В итоге остановился на FastJsonParse, добавили batch_size +small.json - 2.11s +medium.json - 18.28s +large.json - 182.09s + +Осталось закэшировать bus_services, воспользуемся отсутствием false: id в schema +small.json - 0.8s +medium.json - 1.92s +large.json - 8.34s +Разница драматическая, на этом эту часть заканчиваем + +Поставил pg_hero - eму все нравится, полезной инфы - нет +Rails panel - не заработал +Bullet разумеется помог, но он меня достал уже на рабочем Проекте), здесь все гораздо хуже, ибо он лезет ото всюду - из поискового окна, прямо со страницы... там конечно все настраивается, но он все равно мне не нравится +rack-mini-profiler - великолепен: ненавязчивый и удобный, всегда им пользуюсь + +Render time, start: +5746.0 ms 1886 sql - RMP +Completed 200 OK in 5737ms (Views: 4483.1ms | ActiveRecord: 1247.9ms) + +### Рендеринг страницы +- Добавляем индексы поисковым полям: City#name, Trip#from/to (составной) +- size вместо count для trips +- в trip.html прелоадим bus для trip и services для bus +- #any? вместо #presence? +Completed 200 OK in 2865ms (Views: 2834.6ms | ActiveRecord: 24.0ms) +2874.2 ms 7 sql - запросы выглядят хорошо, но выигрыш по времени небольшой + +Bullet молчит - но по RMP видно что многовато времени в сумме на паршиалы, объединим их в один +Completed 200 OK in 223ms (Views: 194.9ms | ActiveRecord: 22.5ms) +231.5 ms 7 sql - разница драматическая, не все паршиалы одинаково полезны). Заканчиваем. + +Render time, finish +Completed 200 OK in 223ms (Views: 194.9ms | ActiveRecord: 22.5ms) +231.5 ms 7 sql + +## Результаты +- импорт файла `fixtures/large.json`: 8-9 s +- время рендеринга страницы `автобусы/Самара/Москва` на базе файла large.json: 200-300 ms + +*Какими ещё результами можете поделиться* +- Bullet ожидаемо утомителен +- RMP ожидаемо хорош +- паршиалы - красиво и архитектурно, но ни дороговато ли они стоят подчас? Это главный вывод для меня сегодня. +P.S. На работе пользуюсь AppSignal вместо Rollbar - пока доволен + +## Защита от регрессии производительности +Для защиты от потери достигнутого прогресса при дальнейших изменениях программы написал тесты: +Тест соответствия скорости импорта бюджету +Тест корректности импорта (`fixtures/example.json`) diff --git a/config/environments/development.rb b/config/environments/development.rb index 1311e3e4..6dcfbe34 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -1,4 +1,13 @@ Rails.application.configure do + config.after_initialize do + Bullet.enable = false + Bullet.alert = true + Bullet.bullet_logger = true + Bullet.console = 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 0a38fd3c..14557be4 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 = false + Bullet.bullet_logger = true + Bullet.raise = false # 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 diff --git a/config/initializers/rack_profiler.rb b/config/initializers/rack_profiler.rb new file mode 100644 index 00000000..7678f725 --- /dev/null +++ b/config/initializers/rack_profiler.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +if Rails.env.development? + require 'rack-mini-profiler' + + # initialization is skipped so trigger it + Rack::MiniProfiler.config.show_total_sql_count = true + Rack::MiniProfilerRails.initialize!(Rails.application) +end diff --git a/config/initializers/strong_migrations.rb b/config/initializers/strong_migrations.rb new file mode 100644 index 00000000..b3a789a3 --- /dev/null +++ b/config/initializers/strong_migrations.rb @@ -0,0 +1,26 @@ +# Mark existing migrations as safe +StrongMigrations.start_after = 20230326224528 + +# Set timeouts for migrations +# If you use PgBouncer in transaction mode, delete these lines and set timeouts on the database user +StrongMigrations.lock_timeout = 10.seconds +StrongMigrations.statement_timeout = 1.hour + +# Analyze tables after indexes are added +# Outdated statistics can sometimes hurt performance +StrongMigrations.auto_analyze = true + +# Set the version of the production database +# so the right checks are run in development +# StrongMigrations.target_version = 10 + +# Add custom checks +# StrongMigrations.add_check do |method, args| +# if method == :add_index && args[0].to_s == "users" +# stop! "No more indexes on the users table" +# end +# end + +# Make some operations safe by default +# See https://github.com/ankane/strong_migrations#safe-by-default +# StrongMigrations.safe_by_default = true diff --git a/config/routes.rb b/config/routes.rb index a2da6a7b..84053141 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,7 @@ 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 "/" => "statistics#index" get "автобусы/:from/:to" => "trips#index" end diff --git a/db/migrate/20230327215033_add_indexes_to_trips_and_cities.rb b/db/migrate/20230327215033_add_indexes_to_trips_and_cities.rb new file mode 100644 index 00000000..c587ec6e --- /dev/null +++ b/db/migrate/20230327215033_add_indexes_to_trips_and_cities.rb @@ -0,0 +1,9 @@ +class AddIndexesToTripsAndCities < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :cities, :name, algorithm: :concurrently + add_index :trips, [:from_id, :to_id], algorithm: :concurrently + add_index :buses_services, [:bus_id, :service_id], unique: true, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index f6921e45..7929d274 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,9 +10,10 @@ # # 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: 2023_03_27_215033) do # These are extensions that must be enabled in order to support this database + enable_extension "pg_stat_statements" enable_extension "plpgsql" create_table "buses", force: :cascade do |t| @@ -23,10 +24,12 @@ create_table "buses_services", force: :cascade do |t| t.integer "bus_id" t.integer "service_id" + t.index ["bus_id", "service_id"], name: "index_buses_services_on_bus_id_and_service_id", unique: true end create_table "cities", force: :cascade do |t| t.string "name" + t.index ["name"], name: "index_cities_on_name" end create_table "services", force: :cascade do |t| @@ -40,6 +43,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 diff --git a/lib/tasks/initial_seed_service.rb b/lib/tasks/initial_seed_service.rb new file mode 100644 index 00000000..77cdebf4 --- /dev/null +++ b/lib/tasks/initial_seed_service.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'oj' +require 'fast_jsonparser' + +class InitialSeedService + class << self + def call(file_name) + prepare_db + data = parsed_data(file_name) + persist(data) + end + + def prepare_db + Trip.delete_all + BusesService.delete_all + Service.delete_all + Bus.delete_all + City.delete_all + end + + def parsed_data(file_name) + file_content = File.read(file_name) + # JSON.parse(file_content) + # Oj.load(file_content, symbolize_keys: false) + FastJsonparser.parse(file_content, symbolize_keys: false) + end + + def persist(data) + cities = {} + buses = {} + services = {} + trips = [] + buses_services = {} + + ActiveRecord::Base.transaction do + data.each do |trip| + cities[trip['from']] ||= City.create!(name: trip['from']) + cities[trip['to']] ||= City.create!(name: trip['to']) + + bus_number = trip['bus']['number'] + + buses[bus_number] ||= Bus.create!( + number: bus_number, + model: trip['bus']['model'] + ) + + trip['bus']['services'].each do |service| + services[service] ||= Service.create!(name: service) + key = [bus_number, service].join('_') + buses_services[key] ||= BusesService.create!(bus: buses[bus_number], service: services[service]) + end + + trips << { + from_id: cities[trip['from']].id, + to_id: cities[trip['to']].id, + bus_id: buses[bus_number].id, + start_time: trip['start_time'], + duration_minutes: trip['duration_minutes'], + price_cents: trip['price_cents'] + } + end + + Trip.import!(trips) + end + end + end +end diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe871..86dca30d 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,34 +1,9 @@ +# frozen_string_literal: true + +require_relative 'initial_seed_service' + # Наивная загрузка данных из 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'], - ) - end - end + InitialSeedService.call(args.file_name) end diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index d19212ab..652febbd 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -1,4 +1,6 @@ -require "test_helper" +# frozen_string_literal: true + +require 'test_helper' class ApplicationSystemTestCase < ActionDispatch::SystemTestCase driven_by :selenium, using: :chrome, screen_size: [1400, 1400] diff --git a/test/initial_seed_service_test.rb b/test/initial_seed_service_test.rb new file mode 100644 index 00000000..3b3c390d --- /dev/null +++ b/test/initial_seed_service_test.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'test_helper' +require Rails.root.join('lib/tasks/initial_seed_service') + +class ReloadJsonTaskTest < ActiveSupport::TestCase + test 'task on large data is finished less than 10 sec' do + file_name = 'fixtures/large.json' + time = Benchmark.realtime do |_x| + InitialSeedService.call(file_name) + end + + puts "Finish in #{time.round(2)}" + + assert time <= 10 + end + + test 'task updates database correctly' do + file_name = 'fixtures/example.json' + InitialSeedService.call(file_name) + random_trip = + { + from: City.find_by(name: 'Самара'), + to: City.find_by(name: 'Москва'), + start_time: '17:30', + duration_minutes: 37, + price_cents: 173, + bus: Bus.find_by(number: 123) + } + + assert Trip.find_by(random_trip) + assert 10, Trip.count + assert 2, City.count + assert 1, Bus.count + assert 2, Service.count + end +end diff --git a/test/integration/trips_controller_test.rb b/test/integration/trips_controller_test.rb new file mode 100644 index 00000000..508e92b7 --- /dev/null +++ b/test/integration/trips_controller_test.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'test_helper' +require 'rake' + +class TripsControllerTest < ActionDispatch::IntegrationTest + setup do + Task4::Application.load_tasks + file_name = 'fixtures/example.json' + Rake::Task['reload_json'].invoke(file_name) + end + + test 'should get index' do + get URI.parse(URI.escape('/автобусы/Самара/Москва')) + + assert_response :success + assert_select('h1', 'Автобусы Самара – Москва') + assert_select('h2', 'В расписании 5 рейсов') + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 3ab84e3d..96364a67 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,6 +1,9 @@ +# frozen_string_literal: true + ENV['RAILS_ENV'] ||= 'test' require_relative '../config/environment' require 'rails/test_help' +require 'benchmark' class ActiveSupport::TestCase # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.