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 7 #46

Open
wants to merge 1 commit 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
Binary file added after.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added before.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
107 changes: 107 additions & 0 deletions case-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# Case-study оптимизации

## Актуальная проблема
В нашем проекте тесты (6594) проходят за примерно 5,5 минут на CI (локально около 3 минут).

Чуть меньше года назад была итерация по ускорению тестов, результат которой:
30 минут -> 4 минуты (CI)
Copy link
Collaborator

Choose a reason for hiding this comment

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

💪 💪

8,5 минут -> 3 минуты (локально)
Copy link
Collaborator

Choose a reason for hiding this comment

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

💪

В первой итерации основные изменения:
вынесла в сиды 2 справочника, объекты которых создавались в каждом тесте
уменьшила вложенность фабрик
вынесла в фикстуры дефолтные фабрики
привела в порядок использование фабрик в самых долгих тестах (let_it_be, build, before_all)

Локально время не дегрдировало, на CI время поменялось после переезда в Gitlab
Я решила попробовать ускорить тесты еще раз.

## Формирование метрики
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику:
среднее время выполнения тестов за 3 запуска на локальной машине должно укладываться в 2 минуты
Copy link
Collaborator

Choose a reason for hiding this comment

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

сорри за духоту, но это опять не метрика; среднее время за 3 запуска это метрика; а 2 минуты это бюджет;


## Вникаем в детали системы, чтобы найти главные точки роста
Время до оптимизаций: Finished in 3 minutes 7.8 seconds (files took 4.04 seconds to load)

Для того, чтобы найти "точки роста" для оптимизации я воспользовался rspec, ruby-prof, stackprof, test-prof и другими инструментами, которые мы рассмотрели на лекциях.

Вот какие проблемы удалось найти и решить

### Ваша находка №1
воспользовалась test-prof FPROF=flamegraph, нашла вложенность фабрик, которую можно убрать, начала с накладных
пересмотрела еще раз фабрики, нашла избыточную вложенность, которую можно убрать. Экономит не так много времени, зато уменьшается вложенность
было фабрик: Total: 20290
стало: Total: 20093
![before.png](before.png)
![after.png](after.png)

### Ваша находка №2
воспользовалась test-prof FPROF=1, нашла часто используемую фабрику с долгим времен создания, убрала лишние поля
фабрика компаний занимает много времени несмотря на свою простоту, посмотрим что внутри:
много sequence и ненужных полей, попробуем убрать и проверить эффект:

name total top-level total time time per call top-level time
было: company 2075 1268 24.2947s 0.0117s 14.6411s
стало: company 2075 1268 18.5263s 0.0089s 10.9690s
Copy link
Collaborator

Choose a reason for hiding this comment

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

да, можно в этом отчёте вообще посмотреть по time per call, какие фабрики сильно медленнее остальных и обратить на них внимание


Проверка времени на текущий момент: Finished in 2 minutes 57 seconds (files took 3.99 seconds to load)

### Ваша находка №3
По отчету test-prof FPROF=1 FPROF_VARS=1 находим часто создаваемую фабрику, вынесем ее в фикстуру
Вынесем фикстуру админа в before(:suite)

user 1629 1604 20.2135s 0.0124s 20.1218s
Total: 19893
Total top-level: 12747
Total time: 01:36.860 (out of 03:06.752)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ахах, капец, половина от всех тестов юзеры создаются

Total uniq factories: 110
Copy link
Collaborator

Choose a reason for hiding this comment

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

раньше не было про uniq factories, клёвое нововведение


user 1035 1010 13.4071s 0.0130s 13.3243s
Total: 18057
Total top-level: 12143
Copy link
Collaborator

Choose a reason for hiding this comment

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

треть в каскадах получается

Total time: 01:28.752 (out of 02:54.755)
Total uniq factories: 110

[TEST PROF INFO] AnyFixture usage stats:

key build time hit count saved time
platform_admin 00:00.040 593 00:24.047

### Ваша находка №4
Вынесем фикстуру forwarder_operator:

[TEST PROF INFO] AnyFixture usage stats:

key build time hit count saved time
forwarder_operator 00:00.009 126 00:01.237

[TEST PROF INFO] Factories usage

Total: 17554
Total top-level: 12020
Total time: 01:27.353 (out of 02:54.839)
Total uniq factories: 110

user 910 885 11.5259s 0.0127s 11.4501s

Finished in 2 minutes 51.9 seconds (files took 4.2 seconds to load)

### Ваша находка №5
Надо бы еще вынести в фикстуру одну авиакомпанию, которая создается почти во всех тестах и пользователя для авиакомпании, тогда кол-во фабрик сократится еще на 1267 + 235 = 1502
Но так как АК достаточно плотно интегрирована во все тесты и большинство фабрик, то это задача не их простых, поэтому пока оставим как есть

Даже с внедрением этих изменений не думаю, что удастся уложиться в 120 секунд без распараллеливания
Хотя там еще будет затронуто создание компаний, накладных и пр. Будет видно, только если вынести фикстуру

### Ваша находка №N
Ну и распараллелим тесты, чтобы ускорить их выполнение:
gem 'parallel_tests'

rake "parallel:spec[3]"
Took 68 seconds (1:08)
Took 67 seconds (1:07)
Took 63 seconds (1:03)

вполне себе успех, уложились в бюджет))
Copy link
Collaborator

Choose a reason for hiding this comment

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

йеее 💪


## Результаты
В результате проделанной оптимизации наконец удалось уложиться в бюджет в 120 секунд на локальной машине
Copy link
Collaborator

Choose a reason for hiding this comment

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

да ближе даже к одной минуте, отлично!