-
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
performance optimization #1 #159
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,5 @@ | ||
source "https://rubygems.org" | ||
gem "ruby-prof" | ||
gem "rspec-benchmark" | ||
gem "ruby-progressbar" | ||
gem "minitest" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
benchmark-malloc (0.2.0) | ||
benchmark-perf (0.6.0) | ||
benchmark-trend (0.4.0) | ||
coderay (1.1.3) | ||
diff-lcs (1.5.1) | ||
method_source (1.1.0) | ||
minitest (5.25.1) | ||
pry (0.15.2) | ||
coderay (~> 1.1) | ||
method_source (~> 1.0) | ||
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.1) | ||
ruby-progressbar (1.13.0) | ||
|
||
PLATFORMS | ||
arm64-darwin-23 | ||
ruby | ||
|
||
DEPENDENCIES | ||
minitest | ||
pry | ||
rspec-benchmark | ||
ruby-prof | ||
ruby-progressbar | ||
|
||
BUNDLED WITH | ||
2.5.16 |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
# Case-study оптимизации | ||
|
||
## Актуальная проблема | ||
В нашем проекте возникла серьёзная проблема. | ||
|
||
Необходимо было обработать файл с данными, чуть больше ста мегабайт. | ||
|
||
У нас уже была программа на `ruby`, которая умела делать нужную обработку. | ||
|
||
Она успешно работала на файлах размером пару мегабайт, но для большого файла она работала слишком долго, и не было понятно, закончит ли она вообще работу за какое-то разумное время. | ||
|
||
Я решил исправить эту проблему, оптимизировав эту программу. | ||
|
||
## Формирование метрики | ||
Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: | ||
Время выполнения программы. | ||
|
||
## Гарантия корректности работы оптимизированной программы | ||
Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации. | ||
|
||
## 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.
|
||
|
||
Вот как я построил `feedback_loop`: создал файлы с различным количеством строк, чтобы программа могла выполняться за 10–20 секунд. | ||
После каждого изменения я запускал программу на файлах с разным количеством строк и смотрел на результаты. | ||
|
||
## Вникаем в детали системы, чтобы найти главные точки роста | ||
Для того, чтобы найти "точки роста" для оптимизации я воспользовался: | ||
- gem ruby-prof и отчеты callstack & qcachegrind | ||
|
||
Вот какие проблемы удалось найти и решить: | ||
|
||
### находка №1 Многократная итерация объекта sessions для создания users_objects | ||
- callstack из ruby-prof | ||
- Одной итерацией собрать необходимые данные в объекте sessions_by_user | ||
- Время выполнения приложения на 15т строк сократилась 7.5 секунд до 0.9 секунд | ||
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) |
||
- Исправленная проблема перестала быть главной точкой роста | ||
|
||
### находка №2 Неэффективный алгоритм с многократными проверками для сбора unique_browsers. | ||
- callstack из ruby-prof | ||
- Было принято решение заменить неэффективный алгоритм с многократными проверками на более оптимизированное решение, использующее встроенные методы Ruby | ||
- Время выполнения приложения на 15т строк сократилась 0.9 секунд до 0.6 секунд | ||
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. лучше при профилировании в подобных задачах, которые очень долгие давать программе хотя бы несколько секунд покрутиться. Так profile будет более приближен к реальности |
||
- Исправленная проблема перестала быть главной точкой роста | ||
|
||
### Находка №3: Неэффективное добавление элементов в массивы с использованием оператора конкатенации | ||
- callstack из ruby-prof | ||
- Было принято решение заменить неэффективное добавление элементов в массивы с помощью оператора + на использование метода << (shovel operator). Дополнительно была применена конструкция case для улучшения читаемости и производительности. | ||
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(1), тогда как + создает новый массив при каждой итерации, что имеет сложность O(n). | ||
- Время выполнения приложения на 15т строк сократилась 0.6 секунд до 0.5 секунд | ||
- Исправленная проблема перестала быть главной точкой роста | ||
|
||
### Находка №4: Избыточный парсинг дат и преобразование в формат iso8601 | ||
- callstack из ruby-prof | ||
- Так как мы уже имеем данные в нужном формате, то было принято решение не тратить время на преобразование даты в формат iso8601 | ||
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: Избыточное использование хешей для представления сессий | ||
- callstack из ruby-prof | ||
- Анализ профилировщика показал, что создание и обработка хешей в методе parse_session занимают значительное время, особенно при большом количестве сессий. Это связано с накладными расходами на создание объектов хешей и доступ к их ключам. | ||
- Было принято решение заменить использование хешей на Struct для представления сессий. Struct предоставляет более легковесную и быструю структуру для хранения данных с фиксированными ключами, что уменьшает время создания объектов и ускоряет доступ к их атрибутам. | ||
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. ого, кажется впервые вижу такую оптимизацию в этом задании; интересно! |
||
- Исправленная проблема перестала быть главной точкой роста. Данные начали обрабатываться быстрее и укладываться в бюджет времени. | ||
|
||
## Результаты | ||
В результате проделанной оптимизации наконец удалось обработать файл с данными. | ||
Удалось улучшить метрику системы с более чем 15 минут обработки файла до 30 секунд и уложиться в заданный бюджет. | ||
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. там ближе к неделе, чем к 15 минутам |
||
|
||
## Защита от регрессии производительности | ||
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы я написал performance_test.rb. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
require 'rspec-benchmark' | ||
require_relative 'task-1' | ||
|
||
RSpec.configure do |config| | ||
config.include RSpec::Benchmark::Matchers | ||
end | ||
|
||
describe 'Task-1 Performance' do | ||
describe '#work method' do | ||
it 'completes the operation within 30 seconds' do | ||
expect { work(file_name: "data_large.txt", progress_bar: false) }.to perform_under(30).sec | ||
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) можно было бы сделать тест побыстрее. Взять например 1% данных и 1% времени |
||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
require_relative 'task-1.rb' | ||
require 'ruby-prof' | ||
|
||
RubyProf.measure_mode = RubyProf::WALL_TIME | ||
|
||
result = RubyProf.profile { work(file_name: '100000.txt', disable_gc: true, progress_bar: false) } | ||
|
||
RubyProf::FlatPrinter.new(result).print(File.open("ruby_prof_reports/flat.txt", "w+")) | ||
RubyProf::GraphHtmlPrinter.new(result).print(File.open("ruby_prof_reports/graph.html", "w+")) | ||
RubyProf::CallStackPrinter.new(result).print(File.open('ruby_prof_reports/callstack.html', 'w+')) | ||
RubyProf::CallTreePrinter.new(result).print(:path => "ruby_prof_reports", :profile => 'callgrind') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,5 @@ | ||
# Deoptimized version of homework task | ||
|
||
require 'json' | ||
require 'pry' | ||
require 'date' | ||
require 'minitest/autorun' | ||
require 'ruby-progressbar' | ||
|
||
class User | ||
attr_reader :attributes, :sessions | ||
|
@@ -24,15 +20,11 @@ def parse_user(user) | |
} | ||
end | ||
|
||
Session = Struct.new(:user_id, :session_id, :browser, :time, :date) | ||
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. 👍 👍 |
||
|
||
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], | ||
} | ||
_, user_id, session_id, browser, time, date = session.split(',', 6) | ||
Session.new(user_id, session_id, browser, time, date) | ||
end | ||
|
||
def collect_stats_from_users(report, users_objects, &block) | ||
|
@@ -43,16 +35,23 @@ def collect_stats_from_users(report, users_objects, &block) | |
end | ||
end | ||
|
||
def work | ||
file_lines = File.read('data.txt').split("\n") | ||
def work(file_name: "data.txt", disable_gc: false, progress_bar: true) | ||
GC.disable if disable_gc | ||
file_lines = File.read(file_name).split("\n") | ||
|
||
users = [] | ||
sessions = [] | ||
|
||
file_progressbar = progress_bar ? ProgressBar.create(title: "Reading File", total: file_lines.count, format: '%t: |%B| %p%% %e') : nil | ||
|
||
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' | ||
case | ||
when line.start_with?('user,') | ||
users << parse_user(line) | ||
when line.start_with?('session,') | ||
sessions << parse_session(line) | ||
end | ||
increment_progressbar(file_progressbar) | ||
end | ||
|
||
# Отчёт в json | ||
|
@@ -75,13 +74,9 @@ def work | |
report[:totalUsers] = users.count | ||
|
||
# Подсчёт количества уникальных браузеров | ||
uniqueBrowsers = [] | ||
sessions.each do |session| | ||
browser = session['browser'] | ||
uniqueBrowsers += [browser] if uniqueBrowsers.all? { |b| b != browser } | ||
end | ||
unique_browsers = sessions.map { |session| session['browser'] }.uniq | ||
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 ещё заюзать |
||
|
||
report['uniqueBrowsersCount'] = uniqueBrowsers.count | ||
report['uniqueBrowsersCount'] = unique_browsers.count | ||
|
||
report['totalSessions'] = sessions.count | ||
|
||
|
@@ -94,13 +89,21 @@ def work | |
.join(',') | ||
|
||
# Статистика по пользователям | ||
users_objects = [] | ||
sessions_by_user = {} | ||
sessions.each do |session| | ||
user_id = session['user_id'] | ||
sessions_by_user[user_id] ||= [] | ||
sessions_by_user[user_id] << session | ||
end | ||
|
||
user_progressbar = progress_bar ? ProgressBar.create(title: "Processing Users", total: users.count, format: '%t: |%B| %p%% %e') : nil | ||
|
||
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] | ||
users_objects = users.map do |user| | ||
user_id = user['id'] | ||
user_sessions = sessions_by_user[user_id] || [] | ||
user_object = User.new(attributes: user, sessions: user_sessions) | ||
increment_progressbar(user_progressbar) | ||
user_object | ||
end | ||
|
||
report['usersStats'] = {} | ||
|
@@ -137,40 +140,12 @@ def work | |
|
||
# Даты сессий через запятую в обратном порядке в формате 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 } } | ||
{ 'dates' => user.sessions.map{|s| s['date']}.sort.reverse } | ||
end | ||
|
||
File.write('result.json', "#{report.to_json}\n") | ||
end | ||
|
||
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 | ||
def increment_progressbar(progressbar) | ||
progressbar&.increment | ||
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.
Да, но есть момент, что время работы программы ещё зависит от объёма данных.
Нам тут получается нужно несколько разных метрик. Каждая из которых служит одной цели: на текущей итерации оценить насколько текущее изменения полезно