-
Notifications
You must be signed in to change notification settings - Fork 177
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
Add task 1 #126
base: master
Are you sure you want to change the base?
Add task 1 #126
Changes from all commits
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,11 @@ | ||
data/ | ||
|
||
ruby_prof_reports/ | ||
stackprof_reports/ | ||
|
||
ruby_prof_call_stack.rb | ||
ruby_prof_flat.rb | ||
ruby_prof_graph.rb | ||
ruby_prof_grind.rb | ||
stackprof.rb | ||
stackprof_speedscope.rb |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
bm: | ||
ruby benchmarking.rb | ||
|
||
test: | ||
ruby test_me.rb | ||
|
||
check: | ||
rspec performance.rb | ||
|
||
prof-flat: | ||
ruby ruby_prof_flat.rb | ||
|
||
prof-flat_read: | ||
cat ruby_prof_reports/flat.txt | ||
|
||
prof-graph: | ||
ruby ruby_prof_graph.rb | ||
|
||
prof-graph_read: | ||
open ruby_prof_reports/graph.html | ||
|
||
prof-call_stack: | ||
ruby ruby_prof_call_stack.rb | ||
|
||
prof-call_stack_read: | ||
open ruby_prof_reports/call_stack.html | ||
|
||
prof-call_grind: | ||
ruby ruby_prof_grind.rb | ||
|
||
prof-call_grind_read: | ||
qcachegrind ruby_prof_reports/callgrind.out.15532 | ||
|
||
stackprof: | ||
ruby stackprof.rb | ||
|
||
# stackprof stackprof.dump --method Object#work | ||
stackprof_read: | ||
cd stackprof_reports && stackprof stackprof.dump | ||
|
||
stackprof_speedscope: | ||
ruby stackprof_speedscope.rb | ||
|
||
# sudo rbspy record --pid 58587 # подключение к работающему процессу | ||
|
||
.PHONY: test |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
require 'benchmark' | ||
require 'benchmark/ips' | ||
require_relative 'task-1' | ||
|
||
# 10_000 - 1s | ||
# 20_000 - 3.65s | ||
# 40_000 - 15.48s | ||
# 80_000 - 69.78s | ||
|
||
time = Benchmark.realtime do |x| | ||
work('data/data_40k.txt') | ||
end | ||
puts "Finish in #{time.round(2)}" | ||
|
||
# Benchmark.ips do |x| | ||
# x.config(stats: :bootstrap, confidence: 95) | ||
# x.report('Working time') { work('data/data_large.txt') } | ||
# end | ||
|
||
# 10_000 - 0.02s | ||
# 20_000 - 0.05s | ||
# 40_000 - 0.1s | ||
# 80_000 - 0.19s | ||
|
||
# all data - 17.31s | ||
# all data - 0.076 (± 0.0%) i/s - 1.000 in 13.222855s |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,45 +12,88 @@ | |
Я решил исправить эту проблему, оптимизировав эту программу. | ||
|
||
## Формирование метрики | ||
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: *тут ваша метрика* | ||
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: *время выполнения программы в секундах, расчитываемое на файле 40 000 строк. Также использовались файлы 10 0000, 20 000 и 80 000 строк* | ||
|
||
## Гарантия корректности работы оптимизированной программы | ||
Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. | ||
|
||
## Feedback-Loop | ||
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *время, которое у вас получилось* | ||
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *до 30 секунд, за вычетом времени поиска решения по оптимизации проблемы* | ||
|
||
Вот как я построил `feedback_loop`: *как вы построили feedback_loop* | ||
Вот как я построил `feedback_loop`: | ||
0. Запусается benchmark: `benchmarking.rb` | ||
1. Запускается основной профилировщик `ruby-prof-graph`, eсли проблема не ясна (очень редко) - дополнительный: `stackprof-speedscope` или `ruby_prof_grind` | ||
2. Рефакторится код | ||
3. Выполняется тест `test_me.rb` | ||
4. Запусается benchmark: `benchmarking.rb` | ||
5. Запусается `ruby-prof-graph` | ||
|
||
## Вникаем в детали системы, чтобы найти главные точки роста | ||
Для того, чтобы найти "точки роста" для оптимизации я воспользовался *инструментами, которыми вы воспользовались* | ||
Для того, чтобы найти "точки роста" для оптимизации я воспользовался: | ||
На первом этапе запускал все профилировщик, далее - ruby-prof-flat для общей картины и затем ruby-prof-graph для анализа. Если оставались вопросы - `stackprof-speedscope` или `ruby_prof_grind`, потом - обходился без ruby-prof-flat | ||
|
||
Вот какие проблемы удалось найти и решить | ||
|
||
Стартовый benchmark: | ||
# 40_000 строк - 15.48s | ||
# 80_000 строк - 69.78s | ||
*40_000: 0.059 (± 0.0%) i/s - 1.000 in 16.210859s* | ||
*80_000: 0.014 (± 0.0%) i/s - 1.000 in 73.680666s* | ||
|
||
### Ваша находка №1 | ||
- какой отчёт показал главную точку роста | ||
- как вы решили её оптимизировать | ||
- как изменилась метрика | ||
- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста? | ||
Неоптимальное формирование Статистики по пользователям (user_objects) | ||
- главную точку роста показал отчет ruby-prof-flat: `76.55% Array#select` | ||
- сократил количество проходов по массивам users и sessions до одного для каждого | ||
- новая метрика: | ||
# 40_000 - 0.68s | ||
# 80_000 - 2.45s | ||
*40_000: 1.440 (± 4.2%) i/s - 8.000 in 5.585867s* | ||
*80_000: 0.383 (± 2.6%) i/s - 2.000 in 5.223301s* | ||
- новый отчёт профилировщика - исправленная проблема перестала быть главной точкой роста и возникло несколько проблемных мест с примерно одинаковой долей, асимптотическая сложность значимо снизилась | ||
|
||
### Ваша находка №2 | ||
- какой отчёт показал главную точку роста | ||
- как вы решили её оптимизировать | ||
- как изменилась метрика | ||
- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста? | ||
|
||
### Ваша находка №X | ||
- какой отчёт показал главную точку роста | ||
- как вы решили её оптимизировать | ||
- как изменилась метрика | ||
- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста? | ||
Неоптимальный подсчёт количества уникальных браузеров, а также медленный метод пополнения массива | ||
- отчет ruby-prof-graph показал три точки роста по 20% | ||
`20.24% Array#all?` | ||
`21.02% BasicObject#!=` | ||
`21.65% Array#+` | ||
- использовал Set и поменял во всем коде Array#+ на Array#<< | ||
- новая метрика | ||
# 40_000 - 0.25s | ||
- новый отчёт профилировщика: обе проблемы решены | ||
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. Лучше не смешивать несколько оптимизаций в один шаг |
||
|
||
### Ваша находка №3 | ||
Неоптимальный метод collect_stats_from_users вызывается неоптимальное количество раз | ||
- отчет ruby-prof-graph показал точку роста | ||
`23.69% Array#each в Object#collect_stats_from_users` | ||
- Убрал лишние итерации юзеров, оптимизировал применение map при использовании этого метода | ||
- новая метрика | ||
# 40_000 - 0.18s | ||
- как изменился отчёт профилировщика - по сути не сильно но это понятно, потому что этот метод объективно самый тяжелый в плане доли | ||
|
||
С этого момента какой-то одной большой точки роста не обнаруживалось, поэтому смотрел по группам | ||
### Ваша находка №4 | ||
Плохая работа со строками - #split, #upcase, RegExp | ||
- Оптимизировал работу cо строкаи, а также методы сбора статистики браузеров, убрав лишние проходы по коллекциям | ||
- новая метрика | ||
# 40_000 - 0.15s | ||
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. Файл уже маловат, для профилирования реально долгого процесса хорошо брать пример, который покрутится хотя бы несколько секунд |
||
- как изменился отчёт профилировщика - не слишком, и я понял, что пора в целом глянуть код и заканчивать | ||
|
||
### Ваша находка №5 | ||
- как изменился отчёт профилировщика - не слишком, и я понял, что пора в целом глянуть код и заканчивать: | ||
1. Заменил строки на символы в ключах | ||
2. Убрал объект User. | ||
3. Убрал парсинг даты | ||
- новая метрика | ||
# 40_000 - 0.1s | ||
- как изменился отчёт профилировщика - радикально повлиял убранный парсинг даты, без него - примерно то же самое | ||
|
||
## Результаты | ||
В результате проделанной оптимизации наконец удалось обработать файл с данными. | ||
Удалось улучшить метрику системы с *того, что у вас было в начале, до того, что получилось в конце* и уложиться в заданный бюджет. | ||
Удалось улучшить метрику системы с *надоело-ждать секунд до 16-18 секунд* и уложиться в заданный бюджет 30 секунд. | ||
|
||
*Какими ещё результами можете поделиться* | ||
Сознательно отказался от попытки посторочного или глобального анализа кода и шел тупо по результатам профилировщика, что было интересным опытом. Быстрое и грубое решение об изменении схемы перебора Array#select + замена Array#+ на Array#<< радикально поменяла всею картину. Потом код дорабатывался (в итоге - долго) - убирал лишние переборы и проч., но эффект был незначительный (кроме парсинга даты). Принцип Парето как он есть:) Можно дальше дорабатывать, посмотреть сериалайзер для JSON и проч., но задача выполнена - бюджет достигнут. Также поразмышлял над диллемой - добавить коду читаемости и удобства поддержки или тупо выгадывать каждую миллисекунду, учитывая что по сравнению со временем запроса к БД это - ничто. | ||
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. 👍 супер |
||
|
||
## Защита от регрессии производительности | ||
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы *о performance-тестах, которые вы написали* | ||
|
||
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы написал тест performance.rb: 40_000 строк - в пределах 120ms |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
require 'rspec/core' | ||
require 'rspec-benchmark' | ||
require_relative 'task-1' | ||
|
||
RSpec.configure do |config| | ||
config.include RSpec::Benchmark::Matchers | ||
end | ||
|
||
describe 'basic work' do | ||
let(:filepath) { 'data/data_40k.txt' } | ||
let(:sample) { 10 } | ||
|
||
# start point: 15.48s | ||
# end point: 0.1s | ||
it 'works under 0.12 s' do | ||
expect { work(filepath) } | ||
.to perform_under(110).ms.warmup(2).times.sample(sample).times | ||
end | ||
end |
Large diffs are not rendered by default.
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.
О, Мейкфайл кайф