-
Notifications
You must be signed in to change notification settings - Fork 193
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?
Changes from all commits
2ca3148
cdf1502
e221269
e31cb7c
83dfa3a
6415886
fe2b75d
e4772fb
dbb01dd
4e2f5c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 3.3.6 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| ruby '3.3.6' | ||
| source "https://rubygems.org" | ||
|
|
||
| gem 'minitest' | ||
| gem 'ruby-prof' | ||
| gem 'rspec-benchmark' | ||
| gem 'stackprof' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| GEM | ||
| remote: https://rubygems.org/ | ||
| specs: | ||
| benchmark-malloc (0.2.0) | ||
| benchmark-perf (0.6.0) | ||
| benchmark-trend (0.4.0) | ||
| diff-lcs (1.5.1) | ||
| minitest (5.25.4) | ||
| rspec (3.13.0) | ||
| rspec-core (~> 3.13.0) | ||
| rspec-expectations (~> 3.13.0) | ||
| rspec-mocks (~> 3.13.0) | ||
| rspec-benchmark (0.6.0) | ||
| benchmark-malloc (~> 0.2) | ||
| benchmark-perf (~> 0.6) | ||
| benchmark-trend (~> 0.4) | ||
| rspec (>= 3.0) | ||
| rspec-core (3.13.2) | ||
| rspec-support (~> 3.13.0) | ||
| rspec-expectations (3.13.3) | ||
| diff-lcs (>= 1.2.0, < 2.0) | ||
| rspec-support (~> 3.13.0) | ||
| rspec-mocks (3.13.2) | ||
| diff-lcs (>= 1.2.0, < 2.0) | ||
| rspec-support (~> 3.13.0) | ||
| rspec-support (3.13.2) | ||
| ruby-prof (1.7.0) | ||
| stackprof (0.2.27) | ||
|
|
||
| PLATFORMS | ||
| ruby | ||
| x86_64-linux | ||
|
|
||
| DEPENDENCIES | ||
| minitest | ||
| rspec-benchmark | ||
| ruby-prof | ||
| stackprof | ||
|
|
||
| RUBY VERSION | ||
| ruby 3.3.6p108 | ||
|
|
||
| BUNDLED WITH | ||
| 2.5.16 |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| # Case-study оптимизации | ||
|
|
||
| ## Актуальная проблема | ||
| В нашем проекте возникла серьёзная проблема. | ||
|
|
||
| Необходимо было обработать файл с данными, чуть больше ста мегабайт. | ||
|
|
||
| У нас уже была программа на `ruby`, которая умела делать нужную обработку. | ||
|
|
||
| Она успешно работала на файлах размером пару мегабайт, но для большого файла она работала слишком долго, и не было понятно, закончит ли она вообще работу за какое-то разумное время. | ||
|
|
||
| Я решил исправить эту проблему, оптимизировав эту программу. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. решила* |
||
|
|
||
| ## Формирование метрики | ||
| Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: | ||
|
|
||
| замер времени между началом и концом работы на семпле в 50000 строк, далее на `data_large.txt` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ну да, так замерять время вполне нормально |
||
| Замеряла просто вычитая время конца из времени начала. Но опять же, большую часть времени смотрела прогресс в отчёте `ruby-prof` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Этот вопрос в данном случае tricky. По факту нет простого одного ответа на всю работу. У нас на каждую итерацию оптимизации новая метрика - время работы на файлах разного размера. Когда мы не можем посчитать общую метрику на всю систему / исходную проблему, то мы можем воспользоваться промежуточными метриками. Их функция получается в том, чтобы помочь нам понять, была ли оптимизация успешна на данной итерации. |
||
|
|
||
| ## Гарантия корректности работы оптимизированной программы | ||
| Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. | ||
|
|
||
| ## Feedback-Loop | ||
| Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *время, которое у вас получилось* | ||
|
|
||
| Вот как я построила (мой неидеальный) `feedback_loop`: | ||
|
|
||
| - подобрала sample , на котором программа выполняется за обозримое время (5-50 секунд) | ||
| - после того, как программа стала выполняться за 90 секунд, проверяла на большом объёме данных | ||
|
|
||
| ## Вникаем в детали системы, чтобы найти главные точки роста | ||
| Для того, чтобы найти "точки роста" для оптимизации я воспользовалась `ruby-prof` (как основным инструментом). | ||
| Сначала в режиме flat, потом быстро перешла на `RubyProf::WALL_TIME`, в принципе этого хватило. Также пробовала stackprof. | ||
|
|
||
| Вот какие проблемы удалось найти и решить: | ||
|
|
||
| ### Ваша находка №1 | ||
| - какой отчёт показал главную точку роста | ||
| ruby-prof: call tree | ||
| ``` | ||
| 7.59% (7.80%) Array#all? [8464 calls, 10000 total] | ||
| ``` | ||
| на sample (10000) | ||
| 5.62 => 4.8 | ||
|
|
||
| Заменила `uniqueBrowsers` на `Set` (но основная оптимизация кмк за счёт убирания прохода по всем браузерам, можно было и `Array.include`) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. по Set поиск O(1), а по массиву O(N) |
||
|
|
||
| не сильно изменилось, но это была бесячая вещь... | ||
|
|
||
| ### Ваша находка №2 | ||
|
|
||
| В принципе сразу тоже: | ||
|
|
||
| ``` | ||
| 86.97% (89.02%) Array#select [1536 calls, 1536 total] | ||
| ``` | ||
|
|
||
| Нужно перестать проходиться по всем сессиям при обработке каждого пользователя. | ||
| Например, сделать ассоциативный массив с ключом user и значением - массив сессий. | ||
|
|
||
| А то и сразу собрать массив объектов `user` и его `sessions`. | ||
| Также сразу можем собрать `uniqueBrowsers` и общее кол-во сессий, чтобы потом не считать. | ||
|
|
||
| Также, `report['allBrowsers']` - это и есть `uniqueBrowsers` , только нужно отстортировать и заджойнить. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. очень важно не делать несколько изменений на одной итерации, потому что тогда сразу непонятно что как сработало; |
||
|
|
||
| Теперь результат - 0.22433002200000374 | ||
|
|
||
| Результат профилировщика изменился )) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. главное, что квадратичную сложность убрали и сделали линейную |
||
|
|
||
| Сразу попробуем на больших данных. | ||
|
|
||
| 72.75583059900032 - уже можно жить | ||
|
|
||
| ### Находка 3 | ||
|
|
||
| Теперь много времени тратим на `Object#collect_stats_from_users`, в т.ч. `map`. | ||
| В принципе можно просто разок пройтись по юзерам и сессиям и собрать `report['usersStats'][user_key]` | ||
|
|
||
| 52.531833646999985 сек | ||
|
|
||
| ### Находка 4 | ||
|
|
||
| Вижу, что много времени тратится на парсинг даты, закэширую даты в Hash, вдруг повезёт. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. с датами можно вообще ничего не делать, они сразу нормальные - это такая пасхалочка
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. С одной стороны - да, но с другой стороны оптимизация получилась приличная (на этих данных по крайней мере), поэтому оставила.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Почитала другие pr и поняла, что они можно было прям всё убрать ) |
||
|
|
||
| 39.95 - неплохо! | ||
|
|
||
| ### Находка 5 | ||
|
|
||
| Теперь `parse_sessions` много времени отнимает. | ||
| Сделаем без массива: | ||
|
|
||
| ```ruby | ||
| _, user_id, session_id, browser, time, date = cols.split(',') | ||
| { | ||
| 'user_id' => user_id, | ||
| 'session_id' => session_id, | ||
| 'browser' => browser, | ||
| 'time' => time, | ||
| 'date' => date, | ||
| } | ||
| ``` | ||
|
|
||
| Ещё пыталась парсить с `csv` , но это было супердолго! | ||
|
|
||
| 36.794 - ну окей, пусть будет | ||
|
|
||
| ### Находка 6 | ||
|
|
||
| И всё-таки не нравится мне эти `file_lines`... | ||
| прочитаем построчно `File.readlines` | ||
|
|
||
| 36.31289 - в пределах погрешности? Ну пусть будет | ||
|
|
||
| ### Время проверить без профилировщика и с GC! | ||
|
|
||
| 26.353673987 | ||
|
|
||
| Okay. | ||
|
|
||
| ## Результаты | ||
| В результате проделанной оптимизации наконец удалось обработать файл с данными. | ||
| Удалось улучшить метрику системы с "невозможно дождаться выполнения на большом объёме данных, 5,6 с на 50000 строк" и уложиться в заданный бюджет. | ||
|
|
||
| ## Защита от регрессии производительности | ||
| Для защиты от потери достигнутого прогресса при дальнейших изменениях программы: | ||
|
|
||
| - написан минимальный тест на performance, проверяющий время | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| {"totalUsers":3,"uniqueBrowsersCount":14,"totalSessions":15,"allBrowsers":"CHROME 13,CHROME 20,CHROME 35,CHROME 6,FIREFOX 12,FIREFOX 32,FIREFOX 47,INTERNET EXPLORER 10,INTERNET EXPLORER 28,INTERNET EXPLORER 35,SAFARI 17,SAFARI 29,SAFARI 39,SAFARI 49","usersStats":{"Leida Cira":{"sessionsCount":6,"totalTime":"455 min.","longestSession":"118 min.","browsers":"FIREFOX 12, INTERNET EXPLORER 28, INTERNET EXPLORER 28, INTERNET EXPLORER 35, SAFARI 29, SAFARI 39","usedIE":true,"alwaysUsedChrome":false,"dates":["2017-09-27","2017-03-28","2017-02-27","2016-10-23","2016-09-15","2016-09-01"]},"Palmer Katrina":{"sessionsCount":5,"totalTime":"218 min.","longestSession":"116 min.","browsers":"CHROME 13, CHROME 6, FIREFOX 32, INTERNET EXPLORER 10, SAFARI 17","usedIE":true,"alwaysUsedChrome":false,"dates":["2017-04-29","2016-12-28","2016-12-20","2016-11-11","2016-10-21"]},"Gregory Santos":{"sessionsCount":4,"totalTime":"192 min.","longestSession":"85 min.","browsers":"CHROME 20, CHROME 35, FIREFOX 47, SAFARI 49","usedIE":false,"alwaysUsedChrome":false,"dates":["2018-09-21","2018-02-02","2017-05-22","2016-11-25"]}}} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
|
|
||
|
|
||
| require 'rspec' | ||
| require 'rspec-benchmark' | ||
| require_relative '../work' | ||
|
|
||
| RSpec.configure do |config| | ||
| config.include RSpec::Benchmark::Matchers | ||
| end | ||
|
|
||
| describe 'Performance' do | ||
| describe 'work' do | ||
| it 'works in 30 seconds' do | ||
| expect do | ||
| work('data_large.txt') | ||
| end.to perform_under(30).sec | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| require_relative '../work' | ||
| class TestMe < Minitest::Test | ||
| def setup | ||
| File.write('result.json', '') | ||
| File.write('data.txt', | ||
| 'user,0,Leida,Cira,0 | ||
| session,0,0,Safari 29,87,2016-10-23 | ||
| session,0,1,Firefox 12,118,2017-02-27 | ||
| session,0,2,Internet Explorer 28,31,2017-03-28 | ||
| session,0,3,Internet Explorer 28,109,2016-09-15 | ||
| session,0,4,Safari 39,104,2017-09-27 | ||
| session,0,5,Internet Explorer 35,6,2016-09-01 | ||
| user,1,Palmer,Katrina,65 | ||
| session,1,0,Safari 17,12,2016-10-21 | ||
| session,1,1,Firefox 32,3,2016-12-20 | ||
| session,1,2,Chrome 6,59,2016-11-11 | ||
| session,1,3,Internet Explorer 10,28,2017-04-29 | ||
| session,1,4,Chrome 13,116,2016-12-28 | ||
| user,2,Gregory,Santos,86 | ||
| session,2,0,Chrome 35,6,2018-09-21 | ||
| session,2,1,Safari 49,85,2017-05-22 | ||
| session,2,2,Firefox 47,17,2018-02-02 | ||
| session,2,3,Chrome 20,84,2016-11-25 | ||
| ') | ||
| end | ||
|
|
||
| def test_result | ||
| work | ||
| expected_result = '{"totalUsers":3,"uniqueBrowsersCount":14,"totalSessions":15,"allBrowsers":"CHROME 13,CHROME 20,CHROME 35,CHROME 6,FIREFOX 12,FIREFOX 32,FIREFOX 47,INTERNET EXPLORER 10,INTERNET EXPLORER 28,INTERNET EXPLORER 35,SAFARI 17,SAFARI 29,SAFARI 39,SAFARI 49","usersStats":{"Leida Cira":{"sessionsCount":6,"totalTime":"455 min.","longestSession":"118 min.","browsers":"FIREFOX 12, INTERNET EXPLORER 28, INTERNET EXPLORER 28, INTERNET EXPLORER 35, SAFARI 29, SAFARI 39","usedIE":true,"alwaysUsedChrome":false,"dates":["2017-09-27","2017-03-28","2017-02-27","2016-10-23","2016-09-15","2016-09-01"]},"Palmer Katrina":{"sessionsCount":5,"totalTime":"218 min.","longestSession":"116 min.","browsers":"CHROME 13, CHROME 6, FIREFOX 32, INTERNET EXPLORER 10, SAFARI 17","usedIE":true,"alwaysUsedChrome":false,"dates":["2017-04-29","2016-12-28","2016-12-20","2016-11-11","2016-10-21"]},"Gregory Santos":{"sessionsCount":4,"totalTime":"192 min.","longestSession":"85 min.","browsers":"CHROME 20, CHROME 35, FIREFOX 47, SAFARI 49","usedIE":false,"alwaysUsedChrome":false,"dates":["2018-09-21","2018-02-02","2017-05-22","2016-11-25"]}}}' + "\n" | ||
| assert_equal expected_result, File.read('result.json') | ||
| end | ||
| end |
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, потом код