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

Perform optimization #139

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
15 changes: 15 additions & 0 deletions benchmark.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
require 'benchmark/ips'
require_relative 'task-1.rb'

Benchmark.ips do |x|
# The default is :stats => :sd, which doesn't have a configurable confidence
# confidence is 95% by default, so it can be omitted
x.config(
stats: :bootstrap,
confidence: 95,
)

x.report("generating report") do
work('data_large.txt', disable_gc: false)
end
end
96 changes: 96 additions & 0 deletions case-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Case-study оптимизации

## Актуальная проблема
В нашем проекте возникла серьёзная проблема.

Необходимо было обработать файл с данными, чуть больше ста мегабайт.

У нас уже была программа на `ruby`, которая умела делать нужную обработку.

Она успешно работала на файлах размером пару мегабайт, но для большого файла она работала слишком долго, и не было понятно, закончит ли она вообще работу за какое-то разумное время.

Я решил исправить эту проблему, оптимизировав эту программу.

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

## Гарантия корректности работы оптимизированной программы
Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации.

## Feedback-Loop
Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *29 секунд*

Вот как я построил `feedback_loop`:
1) Использовал профилировщик `RubyProf Flat` с выключенным GC (по началу на объеме в 50к строк)
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) Перезапускал профилировщик повторно и смотрел на полученный результат + непосредственно сам скрипт с включенным GC
4) Увеличивал объем данных и повторно начинал с пункта 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Плюсую увеличение объёма данных

Хоршо подбирать объём данных так, чтобы программа успевала покрутиться пару секунд. Если она завершается слишком быстро (“не успевает поработать”) могут возникнуть какие-то перекосы (например, на полном объёме основная часть времени тратится в основном цикле, а если данных мало - то большая часть уходит на инициализацию и финализацию, например на чтение из файла и запись потом в файл)

И плюс когда время уже на миллисекунды - больше влияние погрешностей.


## Вникаем в детали системы, чтобы найти главные точки роста
Для того, чтобы найти "точки роста" для оптимизации я воспользовался `RubyProf Flat`

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

### Ваша находка №1
```
user_sessions = sessions.select { |session| session['user_id'] == user['id'] }
```
- вместо полного перебора через select использовал
```
sessions_grouped = sessions.group_by {|session| session['user_id']}
user_sessions = sessions_grouped[user['id']]
```
- На объеме данных в 50к строк скрипт работал +- 2 секунды
- `Array#select` упал с 80% до 0%
Copy link
Collaborator

Choose a reason for hiding this comment

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

И главное что асимптотика стало качественно лучше и приблизилась к линейной


### Ваша находка №2
```
file_lines.each do |line|
cols = line.split(',')
users = users + [parse_user(line)] if cols[0] == 'user'
sessions = sessions + [parse_session(line)] if cols[0] == 'session'
end
```
- На полном объёме данных выжирает всю оперативу, процесс убивается системой (видно в syslog). Переписал всю логику на использование массива объектов классов User и Session вместо хранения в виде массива строк
Copy link
Collaborator

Choose a reason for hiding this comment

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

Кек, но оно выжирает оперативку и убивается только с выключенным GC

- Программа перестала убиваться системой

### Ваша находка №3
- `RubyProf Flat` показывает большой % использования `Array#each`
- Видно, что логика для построения статистики по каждому пользователю написана криво с использованием огромного кол-ва переборов. Нет смысла каждый раз итерироваться по всему массиву при построении отдельной статистики (т.е нет смысла начинать итерацию с нуля при построении sessionsCount, totalTime, longestSession, browsers, usedIE, alwaysUsedChrome и dates - можно строить все нужные статистики на текущей итерации по каждому пользователю, т.е можно перебрать массив users лишь один раз, а не 7)
- После рефакторинга `Array#each` упал до 13.36%

### Ваша находка №4
- `RubyProf Flat` показывает большой % использования `Array#map`
- Некоторые метрики по пользователям (totalTime + longestSession, browsers + usedIE + alwaysUsedChrome) используют одинаковые куски кода с map. Можно использовать мемоизацию. Также заметно бросается в глаза двойной вызов `map` - в некоторых местах достаточно его вызывать лишь один раз
- После рефакторинга `Array#map` упал до 10.18% соответственно

### Ваша находка №5
- `RubyProf Flat` показывает большой % использования `Date#parse`
- Обратив внимание на структуру данных, видно, что конструкцию `Date#parse` можно опустить вообще - использование `.sort.reverse' более чем достаточно
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 да, такая пасхалочка

- После рефакторинга `Date#parse` упал до 0% соответственно

## Результаты
В результате проделанной оптимизации наконец удалось обработать файл с данными.
Удалось улучшить метрику системы с *того, что у вас было в начале, до того, что получилось в конце* и уложиться в заданный бюджет.

## Защита от регрессии производительности
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы написал скрипт с использованием `benchmark/ips` для замера производительности. Итоговый результат:
```
ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-linux]
Warming up --------------------------------------
slow string concatenation
1.000 i/100ms
Calculating -------------------------------------
slow string concatenation
0.034 (± 0.0%) i/s - 1.000 in 29.189983s
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

with 95.0% confidence
Run options: --seed 39795

# Running:

.

Finished in 0.008757s, 114.2005 runs/s, 114.2005 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
```

11 changes: 11 additions & 0 deletions rubyprof_flat.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
require 'ruby-prof'
require_relative 'task-1.rb'

RubyProf.measure_mode = RubyProf::WALL_TIME

result = RubyProf.profile do
work('data_large.txt', disable_gc: true)
end

printer = RubyProf::FlatPrinter.new(result)
printer.print(File.open("ruby_prof_reports/flat.txt", "w+"))
141 changes: 67 additions & 74 deletions task-1.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,42 @@
require 'date'
require 'minitest/autorun'

USER_STATS = {
'sessionsCount' => -> (user) { user.sessions.count },
'totalTime' => -> (user) { user.sessions_time.sum.to_s + ' min.' },
'longestSession' => -> (user) { user.sessions_time.max.to_s + ' min.' },
'browsers' => -> (user) { user.sessions_browsers },
'usedIE' => -> (user) { user.sessions_browsers.include? 'INTERNET EXPLORER' },
'alwaysUsedChrome' => -> (user) { user.sessions_browsers.split(',').uniq.all? { |b| b.upcase =~ /CHROME/ } },
Copy link
Collaborator

Choose a reason for hiding this comment

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

split по запятой как-то не очень, получается сначала собрали, потом разобрали

'dates' => -> (user) { user.sessions.map{ |s| s.attributes['date'] }.sort.reverse }
}

class User
attr_reader :attributes, :sessions

def initialize(attributes:, sessions:)
def initialize(attributes:)
@attributes = attributes
@sessions = []
end

def add_session(session)
@sessions << session
end

def sessions_time
@sessions_time ||= sessions.map {|s| s.attributes['time'].to_i}
end

def sessions_browsers
@sessions_browsers ||= sessions.map {|s| s.attributes['browser'].upcase}.sort.join(', ')
end
end

class Session
attr_reader :attributes

def initialize(attributes:)
@attributes = attributes
@sessions = sessions
end
end

Expand All @@ -35,26 +65,13 @@ def parse_session(session)
}
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']}"
report['usersStats'][user_key] ||= {}
report['usersStats'][user_key] = report['usersStats'][user_key].merge(block.call(user))
end
def collect_stats_from_users(report, user, stat, block)
user_key = "#{user.attributes['first_name']}" + ' ' + "#{user.attributes['last_name']}"
report['usersStats'][user_key] ||= {}
report['usersStats'][user_key][stat] = block.call(user)
end

def work
file_lines = File.read('data.txt').split("\n")

users = []
sessions = []

file_lines.each do |line|
cols = line.split(',')
users = users + [parse_user(line)] if cols[0] == 'user'
sessions = sessions + [parse_session(line)] if cols[0] == 'session'
end

def work(filepath, options = {})
# Отчёт в json
# - Сколько всего юзеров +
# - Сколько всего уникальных браузеров +
Expand All @@ -70,74 +87,50 @@ def work
# - Всегда использовал только Хром? +
# - даты сессий в порядке убывания через запятую +

report = {}
GC.disable if options[:disable_gc]

report[:totalUsers] = users.count
file_lines = File.read(filepath).split("\n")

# Подсчёт количества уникальных браузеров
uniqueBrowsers = []
sessions.each do |session|
browser = session['browser']
uniqueBrowsers += [browser] if uniqueBrowsers.all? { |b| b != browser }
end
users_objects = []
sessions_objects = []
report = {}

report['uniqueBrowsersCount'] = uniqueBrowsers.count
file_lines.each_slice(10000) do |batch|
batch.each do |line|
cols = line.split(',')

report['totalSessions'] = sessions.count
if cols[0] == 'user'
user_attributes = parse_user(line)

report['allBrowsers'] =
sessions
.map { |s| s['browser'] }
.map { |b| b.upcase }
.sort
.uniq
.join(',')
users_objects << User.new(attributes: user_attributes)
elsif cols[0] == 'session'
session_attributes = parse_session(line)
session_object = Session.new(attributes: session_attributes)

# Статистика по пользователям
users_objects = []

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]
sessions_objects << session_object
users_objects.last.add_session(session_object)
end
end
end

report['usersStats'] = {}
all_browsers = sessions_objects.map {|session| session.attributes['browser'].upcase}.sort.uniq.join(',')
Copy link
Collaborator

Choose a reason for hiding this comment

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

обратите внимание, что map создаёт массив, sort ещё один


# Собираем количество сессий по пользователям
collect_stats_from_users(report, users_objects) do |user|
{ 'sessionsCount' => user.sessions.count }
end
report[:totalUsers] = users_objects.count

# Собираем количество времени по пользователям
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
report['uniqueBrowsersCount'] = all_browsers.split(',').size
Copy link
Collaborator

Choose a reason for hiding this comment

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

опять собрали, разобрали

уникальные браузеры удобно собирать в Set, или SortedSet


# Выбираем самую длинную сессию пользователя
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
report['totalSessions'] = sessions_objects.count

# Браузеры пользователя через запятую
collect_stats_from_users(report, users_objects) do |user|
{ 'browsers' => user.sessions.map {|s| s['browser']}.map {|b| b.upcase}.sort.join(', ') }
end
report['allBrowsers'] = all_browsers

# Хоть раз использовал IE?
collect_stats_from_users(report, users_objects) do |user|
{ 'usedIE' => user.sessions.map{|s| s['browser']}.any? { |b| b.upcase =~ /INTERNET EXPLORER/ } }
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 } }
users_objects.each do |user|
USER_STATS.each do |stat, block|
collect_stats_from_users(report, user, stat, block)
end
end

File.write('result.json', "#{report.to_json}\n")
Expand Down Expand Up @@ -169,7 +162,7 @@ def setup
end

def test_result
work
work('data.txt')
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
Expand Down