-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perform optimization #110
base: master
Are you sure you want to change the base?
Perform optimization #110
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# frozen_string_literal: true | ||
|
||
class BusesService < ApplicationRecord | ||
belongs_to :bus | ||
belongs_to :service | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
# frozen_string_literal: true | ||
|
||
module Tasks | ||
class ImportJsonData | ||
BATCH_SIZE = 1000 | ||
|
||
attr_accessor :cities, :trips, :services, :buses_hash, :buses_array, :buses_services, :cities_hash | ||
attr_reader :filepath | ||
|
||
class << self | ||
def call(filepath:) | ||
new(filepath).call | ||
end | ||
end | ||
|
||
def initialize(filepath) | ||
@filepath = filepath | ||
@cities = [] | ||
@cities_hash = {} | ||
@trips = [] | ||
@services = {} | ||
@buses_hash = {} | ||
@buses_array = [] | ||
@buses_services = [] | ||
end | ||
|
||
def call | ||
ActiveRecord::Base.transaction do | ||
create_services | ||
|
||
File.open(filepath) do |ff| | ||
nesting = 0 | ||
str = +'' | ||
|
||
until ff.eof? | ||
ch = ff.read(1) # читаем по одному символу | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 лайк за стриминг, мало кто делает |
||
if ch == '{' # начинается объект, повышается вложенность | ||
nesting += 1 | ||
str << ch | ||
elsif ch == '}' # заканчивается объект, понижается вложенность | ||
nesting -= 1 | ||
str << ch | ||
if nesting.zero? # если закончился объкет уровня trip, парсим и импортируем его | ||
trip = Oj.load(str) | ||
process_trip(trip) | ||
str = +'' | ||
end | ||
elsif nesting >= 1 | ||
str << ch | ||
end | ||
end | ||
end | ||
end | ||
end | ||
|
||
private | ||
|
||
def create_services | ||
Service::SERVICES.each do |service| | ||
services[service] = Service.create!(name: service).id | ||
end | ||
end | ||
|
||
def process_trip(trip) | ||
bus_key = "#{trip['bus']['number']}-#{trip['bus']['model']}" | ||
|
||
trip['bus']['services'].each do |service| | ||
buses_services << [bus_key, services[service]] | ||
end | ||
|
||
buses_array << [trip['bus']['number'], trip['bus']['model']] | ||
|
||
cities << [trip['from']] | ||
cities << [trip['to']] | ||
|
||
trips << [trip['from'], trip['to'], trip['start_time'], trip['duration_minutes'], trip['price_cents'], bus_key] | ||
|
||
if trips.size == BATCH_SIZE | ||
City.import %i[name], cities.uniq!, on_duplicate_key_ignore: true | ||
update_cities_hash(cities) | ||
|
||
Bus.import %i[number model], buses_array, on_duplicate_key_ignore: true | ||
update_bus_hash(buses_array) | ||
|
||
trips.map! do |trip| | ||
[cities_hash[trip[0]], cities_hash[trip[1]], trip[2], trip[3], trip[4], buses_hash[trip[5]]] | ||
end | ||
Trip.import %i[from_id to_id start_time duration_minutes price_cents bus_id], trips | ||
|
||
buses_services.map! { |buses_service| [buses_hash[buses_service.first], buses_service.last] } | ||
BusesService.import %i[bus_id service_id], buses_services, on_duplicate_key_ignore: true | ||
|
||
@buses_array = [] | ||
@trips = [] | ||
@cities = [] | ||
@buses_services = [] | ||
end | ||
end | ||
|
||
def update_bus_hash(buses_array) | ||
buses_array.uniq.each do |bus| | ||
key = "#{bus.first}-#{bus.last}" | ||
buses_hash[*key] = Bus.find_by(number: bus.first, model: bus.last).id unless buses_hash[*key] | ||
end | ||
end | ||
|
||
def update_cities_hash(cities) | ||
cities.each do |city| | ||
cities_hash[*city] = City.find_by(name: city).id unless cities_hash[*city] | ||
end | ||
end | ||
end | ||
end |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,16 @@ | ||
<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.present? %> | ||
<li>Сервисы в автобусе:</li> | ||
<ul> | ||
<%= render partial: "service", collection: trip.bus.services %> | ||
</ul> | ||
<% end %> | ||
</ul> | ||
|
||
==================================================== | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Вот этот разделитель можно тоже параметром задать (забавный факт: https://guides.rubyonrails.org/layouts_and_rendering.html#spacer-templates) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,15 +2,7 @@ | |
<%= "Автобусы #{@from.name} – #{@to.name}" %> | ||
</h1> | ||
<h2> | ||
<%= "В расписании #{@trips.count} рейсов" %> | ||
<%= "В расписании #{@trips.length} рейсов" %> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. лично я за 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 %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
## Задача 1: Импорт данных | ||
Нужно оптимизировать механизм перезагрузки расписания из файла так, чтобы он импортировал файл _large.json_ в БД posgresql в пределах минуты | ||
|
||
# Ход выполнения | ||
Сразу было видно, даже без профилировщиков, что под каждую запись в бд делается отдельный INSERT запрос - это занимает основную часть времени. | ||
Нужно применить подход с BATCH INSERT. Для этого использовался гем **activerecord-import**. Основная проблема была импортировать связи, т.к | ||
требуется внешний ключ, т.е знание id непосредственно на уровне бд. Чтобы не делать лишних SELECT запросов, использовал хранение id автобусов, городов и сервисов в хеше на уровне оперативной памяти (хеш обновлялся каждый раз при добавлении новых объектов в базу) | ||
|
||
Также добавлял индексы (в том числе и составные) на уникальность, т.к при BATCH INSERT возможно появления дубликатов и нужна валидация на уровне бд | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. кстати говоря, индексы лучше в таком случае накидывать после импорта может иметь смысл даже такой флоу
зависит от использования конечно; но поинт в том, что вставки в базе идут быстрее когда не приходится индекс обновлять / проверять / поддерживать |
||
|
||
# Результат | ||
Для оценки скорости работы рейк таски использовал библиотеку Benchmark | ||
|
||
``` | ||
bundle exec rake reload_json[fixtures/large.json] | ||
|
||
Total time is: 21.044087463000324 | ||
``` | ||
|
||
-> получилось добиться импорта файла _large.json_ за +- 20 секунд | ||
|
||
|
||
## Задача 2: Отображение расписаний | ||
Сами страницы расписаний тоже формируются не эффективно и при росте объёмов начинают сильно тормозить. | ||
|
||
Нужно найти и устранить проблемы, замедляющие формирование этих страниц | ||
|
||
Предварительно импортирован файл _large.json_ | ||
|
||
# Ход выполнения | ||
Для оценки скорости работы загрузки страницы использовал гем **rack-mini-profiler** | ||
|
||
### Находка №1 | ||
- видна проблема n+1 при прогрузке ассоциаций в index.html.erb `trip.bus.services` | ||
- Добавил `preload` в контроллер | ||
- рендер страницы ускорился с 5 секунд до 2.7 секунд | ||
|
||
### Находка №2 | ||
- видна долгая загрузка partial _trip.html.erb и _service.html.erb | ||
- удалил конструкцию с `@trips.each` и переписал на более быстрый подход рендера с конструкцией `render partial: "trip", collection: @trips` | ||
- аналогично для сервисов: `<%= render partial: "service", collection: trip.bus.services %>` | ||
- рендер страницы ускорился с 2.7 секунд до 500мс | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
### Находка №3 | ||
- Делался лишний запрос `SELECT COUNT(*)` - исправил вызов `.count` на `.length` на @trips | ||
|
||
### Находка №4 | ||
- Рендер вьюх всё ещё занимал основное время | ||
- Дропнул ненужную вьюху `_delimiter.html.erb` и просто вставил текст из неё в родительскую вьюху `_trip.html.erb` | ||
- рендер страницы ускорился с 500мс до 260мс | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
# Результат | ||
Для оценки скорости смотрел девелопмент лог и вывод гема rack-mini-profiler | ||
|
||
``` | ||
Completed 200 OK in 263ms (Views: 235.8ms | ActiveRecord: 25.0ms) | ||
|
||
``` | ||
|
||
-> получилось добиться открытия страницы за 263мс | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. там ещё индексы на таблицы можно создать было бы; с точки зрения времени загрузки это не топ проблема, но с точки зрения нагрузки на базу - да; и это идёт как use-case pg-hero, так как он прям пишет какие индексы надо создать |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class AddUniquenessConstraintToBus < ActiveRecord::Migration[5.2] | ||
def change | ||
add_index :buses, [:number, :model], unique: true | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class AddUniquenessConstraintToServices < ActiveRecord::Migration[5.2] | ||
def change | ||
add_index :services, :name, unique: true | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class AddUniquenessConstraintToCities < ActiveRecord::Migration[5.2] | ||
def change | ||
add_index :cities, :name, unique: true | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class AddUniquenessConstraintToBusesService < ActiveRecord::Migration[5.2] | ||
def change | ||
add_index :buses_services, [:bus_id, :service_id], unique: true | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лайк за сервис