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

[HW1] CPU optimization #128

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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.idea
data_*.txt
result.json
1 change: 1 addition & 0 deletions .ruby-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3.1.2
52 changes: 32 additions & 20 deletions case-study-template.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,45 +12,57 @@
Я решил исправить эту проблему, оптимизировав эту программу.

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

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

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

Вот как я построил `feedback_loop`: *как вы построили feedback_loop*
Вот как я построил `feedback_loop`:
- Профилирование и поиск наиболее затратных мест в программе
- Оптимизация этих мест
- Зеленые тесты
- Итоговый результат

## Вникаем в детали системы, чтобы найти главные точки роста
Для того, чтобы найти "точки роста" для оптимизации я воспользовался *инструментами, которыми вы воспользовались*
Для того, чтобы найти "точки роста" для оптимизации я воспользовался RubyProf. Попробовал разные форматы, но самым удобным показался callgrind.

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

### Ваша находка №1
- какой отчёт показал главную точку роста
- как вы решили её оптимизировать
- как изменилась метрика
- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста?
- Array#select
- Было решено на этапе чтения файла сразу готовить хэш с массивом сессий с ключем user_id, чтобы уйти от сложности O(n^2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

- Для файла с 30k строк:
Было: 66.977 s
Стало: 1.076115 s
- Проблема перестала быть точкой роста

### Ваша находка №2
- какой отчёт показал главную точку роста
- как вы решили её оптимизировать
- как изменилась метрика
- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста?

### Ваша находка №X
- какой отчёт показал главную точку роста
- как вы решили её оптимизировать
- как изменилась метрика
- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста?
### Ваша находка №2
- Array#each в месте где происходит подсчёт количества уникальных браузеров
- Использовать map и uniq
- Для файла с 30 к строк:
Было: 1.076115 s
Стало: 0.781335 s
- Проблема перестала быть точкой роста

### Ваша находка №3
- Следующая точка роста Object#collect_stats_from_users
- Решил сократить количество вызовов метода, убрал лишние вызовы map внутри, также убрал merge с вызовом блока.
- Для файла с 30 к строк:
Было: 1.076115 s
Стало: 0.349385 s
- Проблема перестала быть точкой роста

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

Choose a reason for hiding this comment

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

Не совсем понятно, надеюсь имеется в виду что data_large обработался за <= 30 секунд

Copy link
Author

Choose a reason for hiding this comment

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

Упс, не для large файла цифры. data_large обрабатывается за ~25 сек по итогу


*Какими ещё результами можете поделиться*

## Защита от регрессии производительности
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы *о performance-тестах, которые вы написали*
Для защиты от потери достигнутого прогресса при дальнейших изменениях программы был написан perfomance тест для файла с 30к строк.

Binary file removed data_large.txt.gz
Binary file not shown.
16 changes: 16 additions & 0 deletions perfomance_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
require 'rspec'
require 'rspec-benchmark'

require_relative 'task-1.rb'

RSpec.configure do |config|
config.include RSpec::Benchmark::Matchers
end

describe 'data_30_000.txt' do
it 'performs less than 100 ms' do
expect do
work('data30_000.txt')
end.to perform_under(100).ms.warmup(2).times.sample(10).times
end
end
18 changes: 18 additions & 0 deletions prof.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
require 'ruby-prof'
require_relative 'task-1.rb'

RubyProf.measure_mode = RubyProf::WALL_TIME

result = RubyProf.profile do
work('data30_000.txt', disable_gc: false)
end

printer = RubyProf::FlatPrinter.new(result)
printer.print(File.open('ruby-prof-flat_final.txt', 'w+'))

# printer = RubyProf::GraphHtmlPrinter.new(result)
# printer.print(File.open("ruby-prof-graph.html", "w+"))

# printer4 = RubyProf::CallTreePrinter.new(result)
# printer4.print(:profile => 'callgrind')

60 changes: 60 additions & 0 deletions ruby-prof-flat_final.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
Measure Mode: wall_time
Thread ID: 80
Fiber ID: 60
Total: 0.349385
Sort by: self_time

%self total self wait child calls name location
19.68 0.276 0.069 0.000 0.207 3 Array#each
17.93 0.104 0.063 0.000 0.042 13780 Array#map
13.74 0.048 0.048 0.000 0.000 30001 String#split
9.10 0.046 0.032 0.000 0.015 25408 Object#parse_session /Users/evgeniyshumilin/projects/rails-optimization-task1/task-1.rb:25
8.26 0.029 0.029 0.000 0.000 175408 Hash#[]
6.02 0.021 0.021 0.000 0.000 175408 Array#[]
3.27 0.011 0.011 0.000 0.000 50816 String#upcase
3.22 0.011 0.011 0.000 0.000 9185 Array#sort
2.70 0.009 0.009 0.000 0.000 19175 String#=~
2.42 0.008 0.008 0.000 0.000 4597 Hash#[]=
1.93 0.007 0.007 0.000 0.000 1 JSON::Ext::Generator::GeneratorMethods::Hash#to_json
1.51 0.012 0.005 0.000 0.007 4592 Array#any?
1.28 0.007 0.004 0.000 0.002 4592 Object#parse_user /Users/evgeniyshumilin/projects/rails-optimization-task1/task-1.rb:16
1.27 0.004 0.004 0.000 0.000 30000 Array#<<
1.12 0.004 0.004 0.000 0.000 4593 Array#join
1.08 0.004 0.004 0.000 0.000 25408 String#to_i
0.83 0.003 0.003 0.000 0.000 2 Array#uniq
0.79 0.006 0.003 0.000 0.003 4592 Array#all?
0.77 0.003 0.003 0.000 0.000 18368 User#sessions
0.65 0.004 0.002 0.000 0.002 4592 Class#new
0.50 0.002 0.002 0.000 0.000 4592 User#initialize /Users/evgeniyshumilin/projects/rails-optimization-task1/task-1.rb:10
0.39 0.001 0.001 0.000 0.000 9184 User#attributes
0.32 0.001 0.001 0.000 0.000 4592 Array#max
0.30 0.001 0.001 0.000 0.000 4592 Array#reverse
0.24 0.001 0.001 0.000 0.000 4592 Array#sum
0.23 0.001 0.001 0.000 0.000 1 <Class::IO>#write
0.21 0.001 0.001 0.000 0.000 4595 Array#count
0.20 0.001 0.001 0.000 0.000 1 <Class::IO>#read
0.04 0.349 0.000 0.000 0.349 1 Object#work /Users/evgeniyshumilin/projects/rails-optimization-task1/task-1.rb:42
0.01 0.349 0.000 0.000 0.349 1 [global]# prof.rb:7
0.00 0.145 0.000 0.000 0.145 1 Object#parse_data /Users/evgeniyshumilin/projects/rails-optimization-task1/task-1.rb:108
0.00 0.016 0.000 0.000 0.016 1 Enumerable#group_by
0.00 0.000 0.000 0.000 0.000 1 JSON::Ext::Generator::State#initialize
0.00 0.125 0.000 0.000 0.125 1 Object#collect_stats_from_users /Users/evgeniyshumilin/projects/rails-optimization-task1/task-1.rb:35

* recursively called methods

Columns are:

%self - The percentage of time spent in this method, derived from self_time/total_time.
total - The time spent in this method and its children.
self - The time spent in this method.
wait - The amount of time this method waited for other threads.
child - The time spent in this method's children.
calls - The number of times this method was called.
name - The name of the method.
location - The location of the method.

The interpretation of method names is:

* MyObject#test - An instance method "test" of the class "MyObject"
* <Object:MyObject>#test - The <> characters indicate a method on a singleton class.

133 changes: 40 additions & 93 deletions task-1.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
# Deoptimized version of homework task
# frozen_string_literal: true

require 'json'
require 'pry'
require 'date'
require 'minitest/autorun'

class User
attr_reader :attributes, :sessions
Expand All @@ -14,46 +13,36 @@ def initialize(attributes:, sessions:)
end
end

def parse_user(user)
fields = user.split(',')
parsed_result = {
def parse_user(fields)
{
'id' => fields[1],
'first_name' => fields[2],
'last_name' => fields[3],
'age' => fields[4],
'age' => fields[4]
}
end

def parse_session(session)
fields = session.split(',')
parsed_result = {
def parse_session(fields)
{
'user_id' => fields[1],
'session_id' => fields[2],
'browser' => fields[3],
'time' => fields[4],
'date' => fields[5],
'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']}"
report['usersStats'][user_key] ||= {}
report['usersStats'][user_key] = report['usersStats'][user_key].merge(block.call(user))
user_key = "#{user.attributes['first_name']} #{user.attributes['last_name']}"
report['usersStats'][user_key] = block.call(user)
end
end

def work
file_lines = File.read('data.txt').split("\n")
def work(filename = 'data_large.txt', disable_gc: false)
GC.disable if disable_gc

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
users, sessions = parse_data(filename)

# Отчёт в json
# - Сколько всего юзеров +
Expand All @@ -75,11 +64,7 @@ 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.map { |s| s['browser'] }.uniq
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


report['uniqueBrowsersCount'] = uniqueBrowsers.count

Expand All @@ -93,84 +78,46 @@ def work
.uniq
.join(',')

grouped_sessions = sessions.group_by { |session| session['user_id'] }
# Статистика по пользователям
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]
users_objects = users.map do |user|
User.new(attributes: user, sessions: grouped_sessions[user['id']] || [])
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

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

# Хоть раз использовал 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

# Даты сессий через запятую в обратном порядке в формате 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 } }
browsers_upcased = user.sessions.map { |s| s['browser'].upcase }
sessions_time = user.sessions.map { |s| s['time'].to_i }
{
'sessionsCount' => user.sessions.count,
'totalTime' => "#{sessions_time.sum} min.",
'longestSession' => "#{sessions_time.max} min.",
'browsers' => browsers_upcased.sort.join(', '),
'usedIE' => browsers_upcased.any? { |b| b =~ /INTERNET EXPLORER/ },
'alwaysUsedChrome' => browsers_upcased.all? { |b| b =~ /CHROME/ },
'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 parse_data(filename)
users = []
sessions = []

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')
File.read(filename).split("\n").each do |line|
cols = line.split(',')
case cols[0]
when 'user'
users << parse_user(cols)
when 'session'
sessions << parse_session(cols)
end
end

[users, sessions]
end
Loading