-
Notifications
You must be signed in to change notification settings - Fork 195
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
Домашка первой недели #161
base: master
Are you sure you want to change the base?
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,79 @@ | ||
# Case-study оптимизации | ||
|
||
## Актуальная проблема | ||
В нашем проекте возникла серьёзная проблема. | ||
|
||
Необходимо было обработать файл с данными, чуть больше ста мегабайт. | ||
|
||
У нас уже была программа на `ruby`, которая умела делать нужную обработку. | ||
|
||
Она успешно работала на файлах размером пару мегабайт, но для большого файла она работала слишком долго, и не было понятно, закончит ли она вообще работу за какое-то разумное время. | ||
|
||
Я решил исправить эту проблему, оптимизировав эту программу. | ||
|
||
## Формирование метрики | ||
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: Стремится к тому, чтобы метод должен выполняться быстрее 30 секунд. | ||
|
||
## Гарантия корректности работы оптимизированной программы | ||
Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. | ||
|
||
## Feedback-Loop | ||
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *время, которое у вас получилось* | ||
|
||
Вот как я построил `feedback_loop`: *как вы построили feedback_loop* | ||
|
||
## Вникаем в детали системы, чтобы найти главные точки роста | ||
Для того, чтобы найти "точки роста" для оптимизации я воспользовался *инструментами, которыми вы воспользовались*: | ||
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 и на основе мини файла проверил, как распределяется нагрузка у программы | ||
|
||
Вот какие проблемы удалось найти и решить | ||
|
||
### Ваша находка №1 | ||
- parser_progressbar, показал что огромную часть времени занимает парсинг строк (более 10 минут, хотя должно быть менее 30 секунд) | ||
- вместо сложения массивов использовал Array#push элемента в существующий массив | ||
- уменьшил с >10 мин до 70с | ||
- перестала быть главной точкой роста ибо у users.each (прогрессбар показывает более 4 дней выполнения) | ||
|
||
### Ваша находка №2 | ||
- Подсчёт количества уникальных браузеров занимает времени больше чем 30с, значит в любом случае нужно оптимизировать | ||
- сгруппировать по ключу "браузер" и взять только ключи | ||
- >30c изменилось на моментальную | ||
- перестала быть главной точкой роста | ||
|
||
### Ваша находка №3 | ||
- Сбор браузеров занимает около 1.5 минуты | ||
- использовать результат полученный ранее и вывести в строку | ||
- >1.5мин изменилось на моментальную | ||
- перестала быть главной точкой роста | ||
|
||
### Ваша находка №4 | ||
- users.each (прогрессбар показывает более 4 дней выполнения) | ||
- сгруппировал сессии по user_id вне массива, чтоб не заниматься этим каждый раз, по мимо этого изменил | ||
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. вообще, именно это самая обычно первая у всех находка, так как это то место, где зарыта сложность O(N^2). этой группировкой она превращается в O(N) |
||
формат добавления в user_objects использую оператор "<<" вместо сложения сумм массивов | ||
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. лучше не смешивать два изменения в один шаг, потому что тогда сразу же становится непонятно что как сработало |
||
|
||
- изменилась с >4 дней до 52c (Группировка сессий - 43c, Сбор cтатистики по пользователям - 9c) | ||
- перестала быть точкой роста | ||
|
||
### Ваша находка №5 | ||
- парсинг строк (file_lines.each) показал что работа составляет около 1 минуты и 4с что больше чем 30с | ||
- вместо сложения массивов использовал Array#push элемента в существующий массив | ||
- уменьшил с >10 мин до 7c | ||
- перестала быть главной точкой роста | ||
|
||
### Ваша находка №6 | ||
- метод collect_stats_from_users на данный момент занимает около 33с работы от 41с вообщем | ||
- добавил предварительные вычисления, теперь все данные для пользователя собираются в одном хеше и сразу обновляют соответствующую запись | ||
- уменьшил с 33с до 15.30c | ||
- перестала быть главной точкой роста | ||
|
||
|
||
## Результаты | ||
В результате проделанной оптимизации наконец удалось обработать файл с данными. | ||
Удалось улучшить метрику системы с так называемой бесконечности(не смог дождаться завершения) до 17-18 секунд и уложиться в заданный бюджет. | ||
|
||
*Какими ещё результами можете поделиться* | ||
|
||
## Защита от регрессии производительности | ||
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы добавлен тест о том, что он должен выполняться быстрее 30с |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,9 @@ | |
require 'pry' | ||
require 'date' | ||
require 'minitest/autorun' | ||
require 'minitest/benchmark' | ||
require 'benchmark' | ||
require 'byebug' | ||
|
||
class User | ||
attr_reader :attributes, :sessions | ||
|
@@ -14,45 +17,53 @@ def initialize(attributes:, sessions:) | |
end | ||
end | ||
|
||
def parse_user(user) | ||
fields = user.split(',') | ||
parsed_result = { | ||
'id' => fields[1], | ||
'first_name' => fields[2], | ||
'last_name' => fields[3], | ||
'age' => fields[4], | ||
} | ||
end | ||
|
||
def parse_session(session) | ||
fields = session.split(',') | ||
parsed_result = { | ||
'user_id' => fields[1], | ||
'session_id' => fields[2], | ||
'browser' => fields[3], | ||
'time' => fields[4], | ||
'date' => fields[5], | ||
} | ||
end | ||
|
||
def collect_stats_from_users(report, users_objects, &block) | ||
users_objects.each do |user| | ||
user_key = "#{user.attributes['first_name']}" + ' ' + "#{user.attributes['last_name']}" | ||
user_key = "#{user.attributes['first_name']} #{user.attributes['last_name']}" | ||
report['usersStats'][user_key] ||= {} | ||
report['usersStats'][user_key] = report['usersStats'][user_key].merge(block.call(user)) | ||
|
||
sessions = user.sessions | ||
times = [] | ||
browsers = [] | ||
dates = [] | ||
|
||
sessions.each do |session| | ||
times << session['time'].to_i | ||
browsers << session['browser'] | ||
dates << Date.strptime(session['date'], '%Y-%m-%d') | ||
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 | ||
|
||
user_stats = block.call(user, times, browsers, dates) | ||
report['usersStats'][user_key].merge!(user_stats) | ||
end | ||
end | ||
|
||
def work | ||
file_lines = File.read('data.txt').split("\n") | ||
|
||
def work(file_name = 'data.txt') | ||
users = [] | ||
sessions = [] | ||
|
||
file_lines.each do |line| | ||
File.foreach(file_name).each do |line| | ||
cols = line.split(',') | ||
users = users + [parse_user(line)] if cols[0] == 'user' | ||
sessions = sessions + [parse_session(line)] if cols[0] == 'session' | ||
|
||
case cols[0] | ||
when 'user' | ||
users << { | ||
'id' => cols[1], | ||
'first_name' => cols[2], | ||
'last_name' => cols[3], | ||
'age' => cols[4], | ||
} | ||
when 'session' | ||
sessions << { | ||
'user_id' => cols[1], | ||
'session_id' => cols[2], | ||
'browser' => cols[3].upcase, | ||
'time' => cols[4], | ||
'date' => cols[5], | ||
} | ||
else | ||
next | ||
end | ||
end | ||
|
||
# Отчёт в json | ||
|
@@ -75,69 +86,40 @@ def work | |
report[:totalUsers] = users.count | ||
|
||
# Подсчёт количества уникальных браузеров | ||
uniqueBrowsers = [] | ||
sessions.each do |session| | ||
browser = session['browser'] | ||
uniqueBrowsers += [browser] if uniqueBrowsers.all? { |b| b != browser } | ||
end | ||
uniqueBrowsers = sessions.group_by { |session| session["browser"] }.keys | ||
|
||
report['uniqueBrowsersCount'] = uniqueBrowsers.count | ||
|
||
report['totalSessions'] = sessions.count | ||
|
||
report['allBrowsers'] = | ||
sessions | ||
.map { |s| s['browser'] } | ||
.map { |b| b.upcase } | ||
.sort | ||
.uniq | ||
.join(',') | ||
report['allBrowsers'] = uniqueBrowsers.sort.join(',') | ||
|
||
|
||
# Статистика по пользователям | ||
users_objects = [] | ||
sessions_by_user = {} | ||
|
||
users.each do |user| | ||
attributes = user | ||
user_sessions = sessions.select { |session| session['user_id'] == user['id'] } | ||
user_object = User.new(attributes: attributes, sessions: user_sessions) | ||
users_objects = users_objects + [user_object] | ||
end | ||
|
||
report['usersStats'] = {} | ||
|
||
# Собираем количество сессий по пользователям | ||
collect_stats_from_users(report, users_objects) do |user| | ||
{ 'sessionsCount' => user.sessions.count } | ||
end | ||
|
||
# Собираем количество времени по пользователям | ||
collect_stats_from_users(report, users_objects) do |user| | ||
{ 'totalTime' => user.sessions.map {|s| s['time']}.map {|t| t.to_i}.sum.to_s + ' min.' } | ||
end | ||
|
||
# Выбираем самую длинную сессию пользователя | ||
collect_stats_from_users(report, users_objects) do |user| | ||
{ 'longestSession' => user.sessions.map {|s| s['time']}.map {|t| t.to_i}.max.to_s + ' min.' } | ||
end | ||
sessions_by_user = sessions.group_by { |session| session['user_id'] } | ||
|
||
# Браузеры пользователя через запятую | ||
collect_stats_from_users(report, users_objects) do |user| | ||
{ 'browsers' => user.sessions.map {|s| s['browser']}.map {|b| b.upcase}.sort.join(', ') } | ||
end | ||
users.each do |user| | ||
user_sessions = sessions_by_user[user['id']] || [] | ||
user_object = User.new(attributes: user, sessions: user_sessions) | ||
|
||
# Хоть раз использовал IE? | ||
collect_stats_from_users(report, users_objects) do |user| | ||
{ 'usedIE' => user.sessions.map{|s| s['browser']}.any? { |b| b.upcase =~ /INTERNET EXPLORER/ } } | ||
users_objects << user_object | ||
end | ||
|
||
# Всегда использовал только Chrome? | ||
collect_stats_from_users(report, users_objects) do |user| | ||
{ 'alwaysUsedChrome' => user.sessions.map{|s| s['browser']}.all? { |b| b.upcase =~ /CHROME/ } } | ||
end | ||
report['usersStats'] = {} | ||
|
||
# Даты сессий через запятую в обратном порядке в формате iso8601 | ||
collect_stats_from_users(report, users_objects) do |user| | ||
{ 'dates' => user.sessions.map{|s| s['date']}.map {|d| Date.parse(d)}.sort.reverse.map { |d| d.iso8601 } } | ||
collect_stats_from_users(report, users_objects) do |user, times, browsers, dates| | ||
{ | ||
'sessionsCount' => user.sessions.count, | ||
'totalTime' => "#{times.sum} min.", | ||
'longestSession' => "#{times.max} min.", | ||
'browsers' => browsers.sort.join(', '), | ||
'usedIE' => browsers.any? { |b| b =~ /INTERNET EXPLORER/ }, | ||
'alwaysUsedChrome' => browsers.all? { |b| b =~ /CHROME/ }, | ||
'dates' => dates.sort.reverse.map(&:iso8601) | ||
} | ||
end | ||
|
||
File.write('result.json', "#{report.to_json}\n") | ||
|
@@ -170,7 +152,14 @@ def setup | |
|
||
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') | ||
|
||
time_2 = Benchmark.realtime do | ||
work('data_large.txt') | ||
end | ||
|
||
assert time_2 < 30, "Производительность ниже ожидаемой: #{time_2} секунд" | ||
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.
Это финальная цель. А в процессе оптимизации нам тут получается нужны промежуточные метрики (время на файлах меньшего размера), которые мы реально можем измерить