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

Valeev dz3 #21

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

Valeev dz3 #21

wants to merge 2 commits into from

Conversation

ruvaleev
Copy link

за 28.38 секунд выполняется импорт файла fixtures/large.json
за 8 секунд рендерится страница автобусы/Самара/Москва

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.

Approve, можно было бы ещё индексов добавить для ускорения выборок

@@ -7,6 +7,15 @@ gem 'rails', '~> 5.2.3'
gem 'pg', '>= 0.18', '< 2.0'
gem 'puma', '~> 3.11'
gem 'bootsnap', '>= 1.1.0', require: false
gem 'newrelic_rpm'
gem 'pghero'
gem 'badger-rails'
Copy link
Collaborator

Choose a reason for hiding this comment

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

ого

@@ -0,0 +1,84 @@
class ReloadJson
Copy link
Collaborator

Choose a reason for hiding this comment

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

Хорошо, что вынесли в отдельный сервис, но название непонятное

@@ -0,0 +1,84 @@
class ReloadJson
def call(file_name, without_db_queries = false)
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 сразу весь в память -> bloat

@@ -2,15 +2,24 @@
<%= "Автобусы #{@from.name} – #{@to.name}" %>
</h1>
<h2>
<%= "В расписании #{@trips.count} рейсов" %>
<%= "В расписании #{@trips.load.size} рейсов" %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 👍 Вы первый, кто сделал load.size, хотя именно так и было в лекции

После первой итерации (только Trips завернули в import)
Finish in 116.12

### Защитимся от внесения функциональных ошибок тестом test/services/reload_json_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.

👍


Вдобавок ко всему, сделаем это, отказавщись от хэшей, используя только массивы (за исключением json-а)

Результат оказался очень впечатляющим.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

### Седьмая гипотеза
#### Убрать все вычисления во вьюхе trips/index.html.erb

Никакого профита, первый раз страница загрузилась даже за 9+ секунд. Так что геморрой с костылями и колбэками себя не оправдывает.
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