Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/trips_controller.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion app/models/bus.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
6 changes: 6 additions & 0 deletions app/models/bus_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class BusService < ApplicationRecord
self.table_name = "buses_services"

belongs_to :bus
belongs_to :service
end
3 changes: 2 additions & 1 deletion app/models/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
6 changes: 3 additions & 3 deletions app/models/trip.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
84 changes: 84 additions & 0 deletions app/services/importer.rb
Original file line number Diff line number Diff line change
@@ -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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Интересная идея с Set!
По-моему первый раз встречается среди домашних работ этого задания.

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
15 changes: 10 additions & 5 deletions app/views/trips/_trip.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
<li><%= "Отправление: #{trip.start_time}" %></li>
<li><%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %></li>
<li><%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %></li>
<li><%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %></li>
<li><%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %></li>
<ul>
<li><%= "Отправление: #{trip.start_time}" %></li>
<li><%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %></li>
<li><%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %></li>
<li><%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %></li>
<li><%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %></li>
<% if trip.bus.services.any? %>
<%= render "services", services: trip.bus.services %>
<% end %>
</ul>
15 changes: 3 additions & 12 deletions app/views/trips/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,16 +1,7 @@
<h1>
<%= "Автобусы #{@from.name} – #{@to.name}" %>
<%= "Автобусы #{params[:from]} – #{params[:to]}" %>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ловко 👍

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

<% @trips.each do |trip| %>
<ul>
<%= render "trip", trip: trip %>
<% if trip.bus.services.present? %>
<%= render "services", services: trip.bus.services %>
<% end %>
</ul>
<%= render "delimiter" %>
<% end %>
<%= render partial: 'trip', collection: @trips, spacer_template: "delimiter", cached: true %>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Плюсик за spacer_template!

47 changes: 47 additions & 0 deletions case-study.md
Original file line number Diff line number Diff line change
@@ -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```, для массовой вставки данных
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


## Находка №2
Используется устаревший метод ```has_and_belongs_to_many```. Использовав связную модель через ```has many through:```, появляется возможность более эффективно использовать ```gem activerecord import```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


## Находка №3
Используется не самая эффективная библиотека для работы с json. Было принято решение использовать ```gem oj```, для более эффективной обработки
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Тут бы хорошо добавить инф-ю про эффект замены


### Загрузка стр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сек
1 change: 1 addition & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 8 additions & 0 deletions config/initializers/rack_profiler.rb
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions db/migrate/20190503072744_add_index_to_trips.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class AddIndexToTrips < ActiveRecord::Migration[5.2]
disable_ddl_transaction!

def change
add_index :trips, [:from_id, :to_id], algorithm: :concurrently
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 плюсик за :concurrently

end
end
7 changes: 7 additions & 0 deletions db/migrate/20190503074123_add_index_to_buses_services.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 4 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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|
Expand All @@ -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
Binary file added images/db-load.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added images/miniprofiler-after.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added images/miniprofiler-before.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added images/table1-after.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added images/table1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added images/table2-after.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added images/table2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
38 changes: 9 additions & 29 deletions lib/tasks/utils.rake
Original file line number Diff line number Diff line change
@@ -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