diff --git a/Gemfile b/Gemfile index 33017fd..722dc64 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 'activerecord-import' +gem 'oj' +gem 'rack-mini-profiler', require: false 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 eb22e16..80af6c1 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.1) + activerecord (>= 3.2) activestorage (5.2.3) actionpack (= 5.2.3) activerecord (= 5.2.3) @@ -76,9 +78,12 @@ GEM nio4r (2.3.1) nokogiri (1.10.2) mini_portile2 (~> 2.4.0) + oj (3.7.12) pg (1.1.4) puma (3.12.1) rack (2.0.6) + rack-mini-profiler (1.0.2) + rack (>= 1.2.0) rack-test (1.1.0) rack (>= 1.0, < 3) rails (5.2.3) @@ -134,11 +139,14 @@ PLATFORMS ruby DEPENDENCIES + activerecord-import bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) + oj pg (>= 0.18, < 2.0) puma (~> 3.11) + rack-mini-profiler rails (~> 5.2.3) tzinfo-data web-console (>= 3.3.0) diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be..bdd6344 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -1,7 +1,7 @@ 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) + from = City.find_by_name!(params[:from]) + to = City.find_by_name!(params[:to]) + @trips = Trip.includes(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 1dcc54c..ae56e99 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 :bus_services + has_many :services, through: :bus_services validates :number, presence: true, uniqueness: true validates :model, inclusion: { in: MODELS } diff --git a/app/models/bus_service.rb b/app/models/bus_service.rb new file mode 100644 index 0000000..4e81476 --- /dev/null +++ b/app/models/bus_service.rb @@ -0,0 +1,6 @@ +class BusService < ApplicationRecord + self.table_name = "buses_services" + + belongs_to :bus + belongs_to :service +end diff --git a/app/models/service.rb b/app/models/service.rb index 9cbb2a3..c991963 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 :bus_services + has_many :buses, through: :bus_services validates :name, presence: true validates :name, inclusion: { in: SERVICES } diff --git a/app/models/trip.rb b/app/models/trip.rb index 9d63dff..f7ba9f6 100644 --- a/app/models/trip.rb +++ b/app/models/trip.rb @@ -1,9 +1,9 @@ class Trip < ApplicationRecord HHMM_REGEXP = /([0-1][0-9]|[2][0-3]):[0-5][0-9]/ - belongs_to :from, class_name: 'City' - belongs_to :to, class_name: 'City' - belongs_to :bus + belongs_to :from, class_name: 'City', touch: true + belongs_to :to, class_name: 'City', touch: true + belongs_to :bus, touch: true validates :from, presence: true validates :to, presence: true diff --git a/app/services/importer.rb b/app/services/importer.rb new file mode 100644 index 0000000..611765e --- /dev/null +++ b/app/services/importer.rb @@ -0,0 +1,84 @@ +class Importer + attr_reader :json + + def initialize(json) + @json = json + end + + def call + ActiveRecord::Base.transaction do + reset_db! + import_cities + import_services + import_busses + + buses = Bus.pluck(:number, :id).to_h + cities = City.pluck(:name, :id).to_h + link_services(buses) + import_trips(buses, cities) + end + end + + private + + def reset_db! + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + BusService.delete_all + end + + def import_cities + cities = Set.new + json.each do |trip| + cities << [trip['from']] << [trip['to']] + end + City.import! [:name], cities.to_a + end + + def import_services + services = Set.new + json.each do |trip| + services.merge trip['bus']['services'].map { |el| [el] } + end + Service.import! [:name], services.to_a + end + + def import_busses + buses = Set.new + json.each do |trip| + buses << [trip['bus']['number'], trip['bus']['model']] + end + Bus.import! [:number, :model], buses.to_a + end + + def link_services(buses) + services = Service.pluck(:name, :id).to_h + joins = Set.new + json.each do |trip| + bid = buses[trip['bus']['number']] + joins.merge(trip['bus']['services'].map { |service| [bid, services[service]] }) + end + BusService.import! [:bus_id, :service_id], joins.to_a + end + + def import_trips(buses, cities) + trips = Set.new + columns = [:from_id, :to_id, :bus_id, :start_time, :duration_minutes, :price_cents] + json.each do |trip| + from_id = cities[trip['from']] + to_id = cities[trip['to']] + bus_id = buses[trip['bus']['number']] + trips.add [ + from_id, + to_id, + bus_id, + trip['start_time'], + trip['duration_minutes'], + trip['price_cents'], + ] + end + Trip.import! columns, trips.to_a + end +end diff --git a/app/views/trips/_trip.html.erb b/app/views/trips/_trip.html.erb index fa1de9a..c6342f8 100644 --- a/app/views/trips/_trip.html.erb +++ b/app/views/trips/_trip.html.erb @@ -1,5 +1,10 @@ -
  • <%= "Отправление: #{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 a60bce4..ed930c3 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -1,16 +1,7 @@

    - <%= "Автобусы #{@from.name} – #{@to.name}" %> + <%= "Автобусы #{params[:from]} – #{params[:to]}" %>

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

    - -<% @trips.each do |trip| %> - - <%= render "delimiter" %> -<% end %> +<%= render partial: 'trip', collection: @trips, spacer_template: "delimiter", cached: true %> diff --git a/case-study.md b/case-study.md new file mode 100644 index 0000000..a8ba7de --- /dev/null +++ b/case-study.md @@ -0,0 +1,47 @@ +# Case-study оптимизации + +## Актуальная проблема +1. Медленная загрузка данных в базу +2. Медленная загрузка страницы с расписанием + +## Формирование метрики +Для расчета времени загрузки данных в базу использовался ```Benchmark.realtime``` +Для измерения скорости загрузки страницы использовался ```gem rack-mini-profiler``` + +## Feedback-Loop +В качестве референсного файла использовался fixtures/small.json, по результатам обработки которого уже принималось решение относительно эффективности того или иного метода оптимизации. +Эффективность оптимизаций по скорости загрузки страницы оценивалась на основе метрик ```gem rack-mini-profiler``` + +### Загрузка данных в базу +## Находка №1 +Для обработки каждой json записи инициализируется несколько объектов и каждый из них в отдельной транзакции вставляется в базу данных, что крайне неэффективно на больших объемах. Было принято решение использовать ```gem activerecord-import```, для массовой вставки данных + +## Находка №2 +Используется устаревший метод ```has_and_belongs_to_many```. Использовав связную модель через ```has many through:```, появляется возможность более эффективно использовать ```gem activerecord import``` + +## Находка №3 +Используется не самая эффективная библиотека для работы с json. Было принято решение использовать ```gem oj```, для более эффективной обработки + +### Загрузка стрaницы +Замеры проводились после загрузки файла ```fixtures/large.json``` +Исходя из результатов ```gem rack-mini-profiler``` ![miniprofiler-before](images/miniprofiler-before.png) можно сделать следующие выводы: + +## Находка №1 +Огромное количество запросов к БД и большое количество N+1 запросов. Принято решение использовать includes для подгрузки ассоциаций и ликвидации лишних запросов. + +## Находка №2 +Большое количество времени занимает отрисовка partial. Было принято решение кешировать результаты + +## Находка №3 +Проанализировав sql, стало понятно, что не используются индексы на некоторых таблицах +![table-1](images/table1.png) +![table-2](images/table2.png) +Добавив соответствующие индексы удалось существенно ускорить выполнение запросов +![table-1 after](images/table1-after.png) +![table-2 after](images/table2-after.png) + +## Результаты +![miniprofiler after](images/miniprofiler-after.png) +![db load](images/db-load.png) + +Благодаря оптимизациям удалось снизить скорость загрузки страницы до 420ms и загрузить требуемый файл за 17сек diff --git a/config/application.rb b/config/application.rb index 9c33109..e167ca5 100644 --- a/config/application.rb +++ b/config/application.rb @@ -15,5 +15,6 @@ class Application < Rails::Application # Application configuration can go into files in config/initializers # -- all .rb files in that directory are automatically loaded after loading # the framework and any gems in your application. + config.autoload_paths += %w(services) end end diff --git a/config/initializers/rack_profiler.rb b/config/initializers/rack_profiler.rb new file mode 100644 index 0000000..96b6cc9 --- /dev/null +++ b/config/initializers/rack_profiler.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +if Rails.env.development? + require "rack-mini-profiler" + + # initialization is skipped so trigger it + Rack::MiniProfilerRails.initialize!(Rails.application) +end diff --git a/db/migrate/20190503072744_add_index_to_trips.rb b/db/migrate/20190503072744_add_index_to_trips.rb new file mode 100644 index 0000000..de78ca3 --- /dev/null +++ b/db/migrate/20190503072744_add_index_to_trips.rb @@ -0,0 +1,7 @@ +class AddIndexToTrips < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :trips, [:from_id, :to_id], algorithm: :concurrently + end +end diff --git a/db/migrate/20190503074123_add_index_to_buses_services.rb b/db/migrate/20190503074123_add_index_to_buses_services.rb new file mode 100644 index 0000000..1b64e3d --- /dev/null +++ b/db/migrate/20190503074123_add_index_to_buses_services.rb @@ -0,0 +1,7 @@ +class AddIndexToBusesServices < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :buses_services, :bus_id, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index f6921e4..932d998 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: 2019_05_03_074123) 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,6 +24,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 +42,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/images/db-load.png b/images/db-load.png new file mode 100644 index 0000000..fda5c9e Binary files /dev/null and b/images/db-load.png differ diff --git a/images/miniprofiler-after.png b/images/miniprofiler-after.png new file mode 100644 index 0000000..2e7c6f2 Binary files /dev/null and b/images/miniprofiler-after.png differ diff --git a/images/miniprofiler-before.png b/images/miniprofiler-before.png new file mode 100644 index 0000000..cc5189e Binary files /dev/null and b/images/miniprofiler-before.png differ diff --git a/images/table1-after.png b/images/table1-after.png new file mode 100644 index 0000000..c9c73c7 Binary files /dev/null and b/images/table1-after.png differ diff --git a/images/table1.png b/images/table1.png new file mode 100644 index 0000000..2418008 Binary files /dev/null and b/images/table1.png differ diff --git a/images/table2-after.png b/images/table2-after.png new file mode 100644 index 0000000..94e0aac Binary files /dev/null and b/images/table2-after.png differ diff --git a/images/table2.png b/images/table2.png new file mode 100644 index 0000000..77ef48e Binary files /dev/null and b/images/table2.png differ diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe87..4b837ac 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,34 +1,14 @@ # Наивная загрузка данных из 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;') +require 'benchmark' +require 'oj' - 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 +task :reload_json, [:file_name] => :environment do |_task, args| + json = Oj.load(File.read(args.file_name)) + p 'Start' + time = Benchmark.realtime do + Importer.new(json).call end + p 'Done' + p time end