-
Notifications
You must be signed in to change notification settings - Fork 195
Optimized code for task 1 #154
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
base: master
Are you sure you want to change the base?
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.
nice work!
@@ -0,0 +1 @@ | |||
3.3.6 |
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.
Извиняюсь за хаотичность коммитов, не удалось полностью придерживаться изначального плана, переделать и догнать уже не будет возможности.
вообще не проблема, никогда не смотрю на них ))
тут самое основное это case-study, потом код
|
||
Она успешно работала на файлах размером пару мегабайт, но для большого файла она работала слишком долго, и не было понятно, закончит ли она вообще работу за какое-то разумное время. | ||
|
||
Я решил исправить эту проблему, оптимизировав эту программу. |
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.
решила*
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: | ||
|
||
замер времени между началом и концом работы на семпле в 50000 строк, далее на `data_large.txt` | ||
Замеряла просто вычитая время конца из времени начала. Но опять же, большую часть времени смотрела прогресс в отчёте `ruby-prof` |
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.
Этот вопрос в данном случае tricky. По факту нет простого одного ответа на всю работу. У нас на каждую итерацию оптимизации новая метрика - время работы на файлах разного размера. Когда мы не можем посчитать общую метрику на всю систему / исходную проблему, то мы можем воспользоваться промежуточными метриками. Их функция получается в том, чтобы помочь нам понять, была ли оптимизация успешна на данной итерации.
## Формирование метрики | ||
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: | ||
|
||
замер времени между началом и концом работы на семпле в 50000 строк, далее на `data_large.txt` |
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.
ну да, так замерять время вполне нормально
А то и сразу собрать массив объектов `user` и его `sessions`. | ||
Также сразу можем собрать `uniqueBrowsers` и общее кол-во сессий, чтобы потом не считать. | ||
|
||
Также, `report['allBrowsers']` - это и есть `uniqueBrowsers` , только нужно отстортировать и заджойнить. |
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.22433002200000374 | ||
|
||
Результат профилировщика изменился )) |
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.
главное, что квадратичную сложность убрали и сделали линейную
|
||
### Находка 4 | ||
|
||
Вижу, что много времени тратится на парсинг даты, закэширую даты в Hash, вдруг повезёт. |
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.
с датами можно вообще ничего не делать, они сразу нормальные - это такая пасхалочка
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.
С одной стороны - да, но с другой стороны оптимизация получилась приличная (на этих данных по крайней мере), поэтому оставила.
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.
Почитала другие pr и поняла, что они можно было прям всё убрать )
Для меня было не очевидно, что они уже отсортированы, поэтому предположила, что сортировка всё-таки нужна, и соотв. парсинг.
it 'works in 30 seconds' do | ||
expect do | ||
work('data_large.txt') | ||
end.to perform_under(30).sec |
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.
👍
на sample (10000) | ||
5.62 => 4.8 | ||
|
||
Заменила `uniqueBrowsers` на `Set` (но основная оптимизация кмк за счёт убирания прохода по всем браузерам, можно было и `Array.include`) |
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.
по Set поиск O(1), а по массиву O(N)
Извиняюсь за хаотичность коммитов, не удалось полностью придерживаться изначального плана, переделать и догнать уже не будет возможности.