Skip to content

ДЗ 3 #20

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
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
11 changes: 5 additions & 6 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,19 @@ 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', require: false

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]
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 'bullet'
gem 'listen', '>= 3.0.5', '< 3.2'
gem 'pghero'
gem 'strong_migrations'
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]
17 changes: 15 additions & 2 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.4)
activerecord (>= 3.2)
activestorage (5.2.3)
actionpack (= 5.2.3)
activerecord (= 5.2.3)
Expand All @@ -47,6 +49,9 @@ GEM
bootsnap (1.4.2)
msgpack (~> 1.0)
builder (3.2.3)
bullet (6.1.0)
activesupport (>= 3.0.0)
uniform_notifier (~> 1.11)
byebug (11.0.1)
concurrent-ruby (1.1.5)
crass (1.0.4)
Expand Down Expand Up @@ -77,6 +82,8 @@ GEM
nokogiri (1.10.2)
mini_portile2 (~> 2.4.0)
pg (1.1.4)
pghero (2.4.1)
activerecord (>= 5)
puma (3.12.1)
rack (2.0.6)
rack-test (1.1.0)
Expand Down Expand Up @@ -117,10 +124,13 @@ GEM
actionpack (>= 4.0)
activesupport (>= 4.0)
sprockets (>= 3.0.0)
strong_migrations (0.6.2)
activerecord (>= 5)
thor (0.20.3)
thread_safe (0.3.6)
tzinfo (1.2.5)
thread_safe (~> 0.1)
uniform_notifier (1.13.0)
web-console (3.7.0)
actionview (>= 5.0)
activemodel (>= 5.0)
Expand All @@ -134,17 +144,20 @@ PLATFORMS
ruby

DEPENDENCIES
activerecord-import
bootsnap (>= 1.1.0)
bullet
byebug
listen (>= 3.0.5, < 3.2)
pg (>= 0.18, < 2.0)
pghero
puma (~> 3.11)
rails (~> 5.2.3)
tzinfo-data
strong_migrations
web-console (>= 3.3.0)

RUBY VERSION
ruby 2.6.3p62

BUNDLED WITH
2.0.2
2.1.4
2 changes: 1 addition & 1 deletion app/controllers/trips_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.where(from: @from, to: @to).preload(:bus).order(:start_time)
end
end
2 changes: 2 additions & 0 deletions app/models/buses_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class BusesService < ApplicationRecord
end
2 changes: 1 addition & 1 deletion app/models/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ class Service < ApplicationRecord

has_and_belongs_to_many :buses, join_table: :buses_services

validates :name, presence: true
validates :name, presence: true, uniqueness: true
validates :name, inclusion: { in: SERVICES }
end
58 changes: 58 additions & 0 deletions app/services/upload_data.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# frozen_string_literal: true

class UploadData
def self.call(file_name)
json = JSON.parse(File.read(file_name))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Грузим в память весь json, который может быть большим
Плюс можно выиграть за счёт Oj


services = []
Service::SERVICES.each do |service|
services << { name: service }
end
Service.import services
uploaded_services = Service.all.inject({}) { |acc, elem| acc.merge!(elem.name => elem.id) }
# uploaded_services == {"Ремни безопасности"=> Service, ...}

cities = Set.new
Copy link
Collaborator

Choose a reason for hiding this comment

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

Кажется не очень хорошая идея здесь использовать Set
Так как Set позволяет хранить только уникальные объекты, ему надо постоянно сравнивать cities, которые сами по себе являются хэшами.

buses = []
bus_numbers = {}
json.each do |trip|
# handle cities
cities << { name: trip['from'] }
cities << { name: trip['to'] }
# handle buses
next if bus_numbers[trip['bus']['number']].present?
buses << Bus.new(
number: trip['bus']['number'],
model: trip['bus']['model'],
service_names: trip['bus']['services']
)
bus_numbers[trip['bus']['number']] = 1
end
# save to DB
result = City.import(cities.to_a, returning: [:id, :name])
uploaded_cities = result.results.inject({}) { |acc, elem| acc.merge!(elem[1] => elem[0]) }
# uploaded_cities == {"Сочи"=>1, "Тула"=>2, "Самара"=>3, "Красноярск"=>4, "Волгоград"=>5, "Рыбинск"=>6, "Саратов"=>7, "Москва"=>8, "Ярославль"=>9, "Ростов"=>10}
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Плюс за коммент, без него было бы сложновато понять что получается

result = Bus.import(buses, recursive: true, returning: [:id, :number])
uploaded_buses = result.results.inject({}) { |acc, elem| acc.merge!(elem[1] => elem[0]) }

trips = []
buses_services = Set.new
until json.empty?
trip = json.shift

trip['bus']['services'].each do |service|
buses_services << { bus_id: uploaded_buses[trip['bus']['number']], service_id: uploaded_services[service] }
end
trips << {
bus_id: uploaded_buses[trip['bus']['number']],
from_id: uploaded_cities[trip['from']],
to_id: uploaded_cities[trip['to']],
start_time: trip['start_time'],
duration_minutes: trip['duration_minutes'],
price_cents: trip['price_cents']
}
end
BusesService.import [:bus_id, :service_id], buses_services.to_a
Trip.import trips
end
end
1 change: 0 additions & 1 deletion app/views/trips/_delimiter.html.erb

This file was deleted.

1 change: 0 additions & 1 deletion app/views/trips/_service.html.erb

This file was deleted.

6 changes: 0 additions & 6 deletions app/views/trips/_services.html.erb

This file was deleted.

5 changes: 0 additions & 5 deletions app/views/trips/_trip.html.erb

This file was deleted.

19 changes: 14 additions & 5 deletions app/views/trips/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,24 @@
<%= "Автобусы #{@from.name} – #{@to.name}" %>
</h1>
<h2>
<%= "В расписании #{@trips.count} рейсов" %>
<%= "В расписании #{@trips.size} рейсов" %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

</h2>

<% @trips.each do |trip| %>
<ul>
<%= render "trip", trip: trip %>
<% if trip.bus.services.present? %>
<%= render "services", services: trip.bus.services %>
<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>
<% unless trip.bus.service_names.empty? %>
<li>Сервисы в автобусе:</li>
<ul>
<% trip.bus.service_names.each do |service| %>
<li><%= "#{service}" %></li>
<% end %>
</ul>
<% end %>
</ul>
<%= render "delimiter" %>
====================================================
<% end %>
4 changes: 2 additions & 2 deletions bin/setup
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ chdir APP_ROOT do
puts "\n== Preparing database =="
system! 'bin/rails db:setup'

puts "\n== Loading data from fixtures/small.json =="
system! 'bin/rake reload_json[fixtures/small.json]'
puts "\n== Loading data from fixtures/large.json =="
system! 'bin/rake reload_json[fixtures/large.json]'

puts "\n== Removing old logs and tempfiles =="
system! 'bin/rails log:clear tmp:clear'
Expand Down
158 changes: 158 additions & 0 deletions case-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
# Case-study оптимизации

## Актуальная проблема
Оптимизация rails приложения

1. Медленная загрузка изначальных данных
2. Не оптимальная работа приложения по выводу информации

## Формирование метрики
Для первой части задания метрикой является время загрузка в БД изначальных данных
Бюджет метрики - large файл с данными должен загружаться быстрее минуты

Для второй части необходимо провести оптимизацию вывод информации
После загрузки данных был произведен просмотр страницы

результаты времени выполнение для large данных
Rendered trips/index.html.erb within layouts/application (3082.9ms)
Completed 200 OK in 3703ms (Views: 3358.3ms | ActiveRecord: 302.6ms)

## Feedback-Loop
Вот как я построил `feedback_loop` для проверки загрузки данных:
т.к. идея на данном этапе понятна - это использовать activerecord-import и делать как можно меньше запросов, то:
- проверить время загрузки
- внести изменения в алгоритм
- повторить

`feedback_loop` для оптимизации вывода информации:
- предварительная проверка через bullet
- цикличная проверка через PgHero
- поиск точки роста
- модификация (добавление индексов, оптимизация запросов)
- проверка результата
- повторение

## Находка 1
Раздумия над алгоритмом загрузки показали, что:
- должно быть слишком много запросов на создание/поиск городов, т.к. их всего 10, то проще и быстрее создать их за 1 прозод через файл, сформировать список загруженных и затем использовать его вместо постоянного обращения к БД
Copy link
Collaborator

Choose a reason for hiding this comment

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

Совершенно верно 👍

- с сервисами такая же проблема, к тому же в описании задания сказано, что их всегда 10, можно заранее загрузить
- для загрузки автобусов и маршрутов использовать activerecord-import с подставлением заранее загруженных данных по городам и сервисам
- в БД нет индексов, поэтому загрузка данных будет максимально быстрой
- добавил валидацию на уникальность для сервисов по названию (особо не имеет смысла, если сервисов всегда 10 и они не меняются, но пусть будет)

### Решение 1
- гипотеза: использовать activerecord-import,
- применение: сперва переделал алгоритм загрузка с использованием гема, в итоге время загрузка small файла уменьшилось, но даже medium файл загружается больше минуты

### Решение 2
- гипотеза: проходить через json несколько раз, с загрузкой части данных
- применение:
- во время первого прохода через json формируются массивы для городов и сервисов, затем список городов формируется в хэш, { название => айди }, для загрузки путешествий нужен будет только айди города, сервисы - { название => объект }, т.к. для создания записей через has_and_belongs_to_many для автобусов нужно будет передавать массив объектов
- во время второго прохода формируется список автобусов, для сервисов используются заранее загруженные данные
- для городов и автобусов - данные формируются через результат работы гема, без доп запросов в БД
- при третьем проходе формируются маршруты
- результат
- medium файл стал загружаться за 7 секнуд
- large - 23 секунды
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍


### Решение 3
- вспомнил, что сервисы постоянны
- применение:
- сервисы создаются заранее, без прозода по массиву данных
- на первом проходе через данные теперь формируется список городов и автобусов
- на втором проходе - формируются маршруты
результат
- время обработки визуально не изменилось, но должно быть чуточку быстрее, особенно на гигантских данных
- последние данные по времени загрузки - 22 секунды

## Находка 2
- сообщение от bullet
USE eager loading detected
Trip => [:bus]
Add to your query: .includes([:bus])

- добавил .includes(:bus)
- сообщение от bullet
USE eager loading detected
Bus => [:services]
Add to your query: .includes([:services])
- изменил на .includes(bus: :services)
- результат:
- выполняется всего 6 запросов в БД
- много рендеринга паршиалов
- загрузка из БД ускорилась в 10 раз
- рендеринг в 2 раза
Rendered trips/index.html.erb within layouts/application (1825.6ms)
Completed 200 OK in 1862ms (Views: 1802.2ms | ActiveRecord: 38.3ms)

## Находка 3
- теперь пора добавлять индексы для поиска
- для поиска города по имени
- для поиска маршрутов по двум полям, from/to, составной
- для маршрутов отдельные индексы для bus_id и для start_time
- составной индекс для таблицы buses_services

Результаты PgHero (после внесение изменений в конфиг pg, для сбора статистики)
No long running queries
Connections healthy 7
Vacuuming healthy
No columns near integer overflow
No invalid indexes or constraints
No duplicate indexes
No slow queries

Space вкладка
- индексы для bus_id и start_time в trips бесполезные, надо удалить

Queries вкладка
- основные запросы - на создание индексов и какие-то рельсовые
- запросы для улучшения
- 8 мс 4 раза - получение маршрутов
- 8 мс 4 раза - подсчет count для маршрутов (изменить на size, думаю лишний запрос)
- остальное незначительно

результат:
- загрузка из БД ускорилась в 3 раза, общее ускорение БД - 30 раз
Rendered trips/index.html.erb within layouts/application (1771.6ms)
Completed 200 OK in 1784ms (Views: 1769.6ms | ActiveRecord: 12.7ms)

## Находка 4
ПРОБЛЕМА
- обнаружил, что не создались записи для связанной таблицы buses_services
- решение исправил файл загрузки + создал класс BusesService для запуска импорта
- результат: время загрузки уменьшилось до 20 секунд
- видимо автобусы стали быстрее загружаться без дополнительных данных, а загрузка связанной таблицы занимает меньше времени

Рендеринг с сервисами из БД
Rendered trips/index.html.erb within layouts/application (4275.7ms)
Completed 200 OK in 4299ms (Views: 4262.1ms | ActiveRecord: 28.4ms)

Рендеринг без сервисов
Rendered trips/index.html.erb within layouts/application (841.3ms)
Completed 200 OK in 854ms (Views: 836.4ms | ActiveRecord: 15.7ms)

Видно, что значительное время тратится на рендеринг названий сервисов для автобусов
имеет смысл сделать сервисы статичными без использования БД и хранить их прямо в автобусах, а при изменении сервиса вызывать обновление всех связанных автобусов

- проделал миграцию
- результат импорта данных уменьшился до 19 секунд

результаты рендеринга
Rendered trips/index.html.erb within layouts/application (705.2ms)
Completed 200 OK in 717ms (Views: 702.6ms | ActiveRecord: 12.0ms)

## Заметки
- пока таблицы services, buses_services выглядят бесполезными
- но сервис может обновится, соответственно надо будет обновить список сервисов всех автобусов, связанных с ним
- и если вдруг добавится/удалится сервис из автобуса
- если удалить эти 2 таблицы, то ускорится время импорта исходных данных, но пока всё укладывается в метрику

## Выводы
- время импорта large данных - 19 секунд
- рендеринг страницы
Rendered trips/index.html.erb within layouts/application (705.2ms)
Completed 200 OK in 717ms (Views: 702.6ms | ActiveRecord: 12.0ms)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 nice!

- использовалось
- [x] `bullet`
- [x] `pghero`
- здравый смысл и опыт
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

6 changes: 6 additions & 0 deletions config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,10 @@
# Use an evented file watcher to asynchronously detect changes in source code,
# routes, locales, etc. This feature depends on the listen gem.
config.file_watcher = ActiveSupport::EventedFileUpdateChecker

config.after_initialize do
Bullet.enable = true
Bullet.alert = true
Bullet.bullet_logger = true
end
end
Loading