-
Notifications
You must be signed in to change notification settings - Fork 115
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
Task 3 #31
base: master
Are you sure you want to change the base?
Task 3 #31
Conversation
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.
Очень хорошая работа, проверил с удовольствием, спасибо! 👍
.rubocop.yml | ||
/docker-valgrind-massif/ | ||
/stackprof_reports/ | ||
1M.json |
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.
👍
@@ -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.eager_load(bus: [:services]).where(from: @from, to: @to).order(:start_time).load |
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.
Плюсик за eager_load
и load
вместо includes
@@ -0,0 +1,36 @@ | |||
class TripsLoad |
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.
Плюсик за вынос в сервис
Кстати я бы его предложил назвать LoadTrips
class TripsLoad | ||
|
||
def self.perform(file_name) | ||
json = Oj.load(File.read(file_name)) |
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.
Тут разбухание памяти при загрузке большого файла
Сначала весь файл грузится в память, и потом ещё весь json целиком формируется в памяти
Для анализа влияния изменений на скорость работы возьмем время обработки файла small.json - 9 секунд | ||
|
||
## Гарантия корректности работы оптимизированной программы | ||
Для проверки корректности работы обновленной программы был написан тест, который заполнял БД данными из файла example.json, потом выгружал БД в json и сравнивал с исходным. |
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.
👍 👍
В результате проделанной оптимизации удалось получить страницу c 1000 рейсами менее чем за 0,6 секунды, что считааю достаточным показателем. | ||
|
||
## Защита от регрессии производительности | ||
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы был написан тест с проверкой загрузки индексной страницы на 100 поездках (файле medium.json) менее чем за 0.3 секунд. Ограничение пришлось огрубить,так как загрузка большего объема сильно замедлит тестирование. |
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.
👍
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы был написан тест с проверкой загрузки индексной страницы на 100 поездках (файле medium.json) менее чем за 0.3 секунд. Ограничение пришлось огрубить,так как загрузка большего объема сильно замедлит тестирование. | ||
|
||
## Чем еще поделиться | ||
1. Все ускорилось и без индекса по trips, хотя он очень просился. Эксперимент показал, что время контретно этого запроса уменьшается с 20 до 9 ms. Что для единичного запроса незначительно. |
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.
Но всё равно в реальном проекте стоило бы его добавить, чтобы базе облегчить жизнь.
Плюсик за обоснованный подход!
@@ -0,0 +1,5 @@ | |||
class AddIndexToBusServices < ActiveRecord::Migration[5.2] | |||
def change | |||
add_index :buses_services, [:service_id, :bus_id] |
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.
Лучше с concurrently. Тут не важно, но на проде лучше не забывать.
test "Index for 100 trips in 0.3 second" do | ||
TripsLoad.perform('fixtures/medium.json') | ||
total = Benchmark.measure{ get URI.escape('/автобусы/Самара/Москва') }.total | ||
assert(total < 0.3) |
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.
👍
require 'test_helper' | ||
require 'json' | ||
|
||
class TestLoad < ActiveSupport::TestCase |
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.
👍
No description provided.