Skip to content
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 #29

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Task 3 #29

wants to merge 3 commits into from

Conversation

shved
Copy link

@shved shved commented Mar 6, 2020

No description provided.

Copy link
Collaborator

@spajic spajic left a comment

Choose a reason for hiding this comment

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

Хорошая, осмысленная работа, спасибо!

2. Устранить освновные проблемы при рендере индекса рейсов, загруженных из `large.json`.

## Гарантия корректности работы оптимизированной программы
Перед тем, как дербанить программу, напишем простой тест, чтобы убедиться, что выдача ручки с индексом рейсов
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

## Вникаем в детали системы, чтобы найти главные точки роста
### Итерация 1
Посмотрим на код таски импорта. Почти вся работа происходит в итерации по трипам.
Попробуем включить ActiveRecord логгер и записать вывод в файл.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍


### Итерация 1
Сразу же видим подсказки от буллета - добавим к трипам `includes(bus: :services)`
Время пребывания в базе сразу сократилось с примерно полутора секунд до 60мс.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Теперь основное время занимает рендеринг паршиалов.

### Итерация 2
Попробуем избавиться от них полностью, перенеся все их содержимое прямо в `index.html.erb`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Можно и так, но чище было бы сделать render collection
https://guides.rubyonrails.org/layouts_and_rendering.html#rendering-collections

Completed 200 OK in 3959ms (Views: 3890.6ms | ActiveRecord: 52.4ms)
```

Улучшать рендеринг далее, без вмешательства в работу рендера в самой рельсе скорее всего не удастся,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Плюс за то, что не имеет смысла уменьшать 50мс на фоне 4000 👍

@@ -1,34 +1,61 @@
# Наивная загрузка данных из json-файла в БД
# rake reload_json[fixtures/small.json]
task :reload_json, [:file_name] => :environment do |_task, args|
# ActiveRecord::Base.logger = Logger.new(STDOUT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Лучше бы из рейк-таски вытащить код в отдельный класс


fails += Trip.import(trips.to_a).failed_instances
fails += BusesServices.import(buses_services.to_a).failed_instances
if fails.any?
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants