Skip to content

Homework 3 #120

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 2 commits into
base: master
Choose a base branch
from
Open

Homework 3 #120

wants to merge 2 commits into from

Conversation

alexrails
Copy link

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.

Супер, спасибо за работу!

@@ -1,3 +1,11 @@
# == Schema Information
Copy link
Collaborator

Choose a reason for hiding this comment

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

лайк за аннотации

#
# Indexes
#
# index_trips_on_from_id_and_to_id (from_id,to_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

лайк за составной индекс

@@ -25,7 +41,7 @@ def to_h
bus: {
number: bus.number,
model: bus.model,
services: bus.services.map(&:name),
services: bus.services.pluck(:name),
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

def connection
@connection ||= ActiveRecord::Base.connection.raw_connection
end
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 чётко


Для удобства работы с моделями добавил gem annotate.

Перед началом оптимизации написал тест который позволит контролировать что после изменений скрипт работает валидно(spec/services/reloader_spec.rb).
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

* {"3.4.1":{"gc":"enabled","time":0.56,"gc_count":24,"memory":"14 MB"}} - medium.json
* {"3.4.1":{"gc":"enabled","time":2.6,"gc_count":243,"memory":"15 MB"}} - large.json
* {"3.4.1":{"gc":"enabled","time":24.16,"gc_count":586,"memory":"32 MB"}} - 1M.json
* {"3.4.1":{"gc":"enabled","time":222.63,"gc_count":5441,"memory":"18 MB"}} - 10M.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

respect 💪

* medium.json - 804ms
* large.json - 5800ms

Добавил тесты для проверки конторллера и элементов страницы.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Добавил тесты для проверки конторллера и элементов страницы.

## Поиск точек роста
- `rack-mini-profiler` показывает более 1700 sql запросов при рендере страницы
Copy link
Collaborator

Choose a reason for hiding this comment

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

эхехе

Время загрузки уменьшилось до 1300ms.

- В `rack-mini-profiler` вижу два запроса в таблице cities по name.
Понимаю, что из-за количества запросов добавление индекса не очень скажется на перформансе.
Copy link
Collaborator

Choose a reason for hiding this comment

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

да, тут с точки зрения времени рендеринга страницы по профилировщико понятно что ускорения за счёт индексов не добиться

но если бы мы решали задачу оптимизировать БД, то там бы такие индексы могли бы помочь

<% end %>
<ul>
<%= render partial: "trip", collection: @trips, as: :trip %>
====================================================
Copy link
Collaborator

Choose a reason for hiding this comment

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

Есть забавная возможность задать spacer параметром: https://guides.rubyonrails.org/layouts_and_rendering.html#spacer-templates

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