Task 1 optimization

case-study-template.md

@@ -12,35 +12,76 @@
 Я решил исправить эту проблему, оптимизировав эту программу.
 
 ## Формирование метрики
-Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: *тут ваша метрика*
+Попытался запустить исходный код и тот самый `data_large.txt`, но спустя примерно 20 минут ожиданий я забросил это дело. Выбрал выборку поменьше (весом ~ 700кб), общее время выполнения кода стало ~ 19 сек. Путем нехитрых вычислений прикинем, что общее время для анализа файла весом в 135 мб составит около 61 минуты.
+
+Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы, я задал цель: файл в итоге не должен открываться больше 5 минут (300 секунд).
 
 ## Гарантия корректности работы оптимизированной программы
 Программа поставлялась с тестом. Выполнение этого теста позволяет не допустить изменения логики программы при оптимизации.
 
 ## Feedback-Loop
-Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *время, которое у вас получилось*
+Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за 19-20 секунд.
+
+Вот как я построил `feedback_loop`:
 
-Вот как я построил `feedback_loop`: *как вы построили feedback_loop*
+1) Уменьшил размер файла до 700кб (~3000 юзеров)
+2) Общее время время выполнения кода стало ~ 19 секунд
+3) Каждый раз, внося изменения, проверял их на тесте, который уже был в коде
 
 ## Вникаем в детали системы, чтобы найти 20% точек роста
-Для того, чтобы найти "точки роста" для оптимизации я воспользовался *инструментами, которыми вы воспользовались*
+Для того, чтобы найти "точки роста" для оптимизации я воспользовался инструментами:
+
+1) встроенный модуль `Benchmark`
+2) гем `stackprof`
+3) гем `memory_profiler`
 
 Вот какие проблемы удалось найти и решить
 
-### Ваша находка №1
-О вашей находке №1
+### Оптимизация №1
+При использовании гема `memory_profiler` было обнаружено, что больше всего памяти было съедено в строке исходного кода `task-1.rb:55`, а именно строка кода `sessions = sessions + [parse_session(line)] if cols[0] == 'session'`, где мы в цикле `file_lines.each` обрабатываем каждую строку на наличие юзера или сессии.
+Было решено вместо конкатенации массивов использовать `<<`, т.е. `Array#push`, что существенно снизило память, выделенную на эту операцию (вместо 1119мб стало лишь 37мб).
+На общее время выполнения кода это никак не повлияло.
+
+### Оптимизация №2
+Далее тот же `memory_profiler` и `stackprof` показывали на другую тяжелую операцию в строке `task-1.rb:101`, а именно `user_sessions = sessions.select { |session| session['user_id'] == user['id'] }`.
+Было решено сильно отрефакторить код, вместо создания новых объектов `User` и присваивания им различных параметров, создать хеш `sessions`, где ключом был бы `id` юзера, а значением - строка из файла с текущей сессией.
+Начальная итерация по файлу стала выглядить так:
+
+```ruby
+  users = []
+  unique_browsers = []
+  sessions = {}
+  total_sessions = 0
+
+  File.read(file).split("\n").each do |line|
+    users << parse_user(line) if line.`start_with?`('user')
+    next if !line.start_with?('session')
 
-### Ваша находка №2
-О вашей находке №2
+    current_session = parse_session(line)
+    sessions[current_session[:user_id]] ||= []
+    sessions[current_session[:user_id]] << current_session
 
-### Ваша находка №X
-О вашей находке №X
+    total_sessions += 1
+    browser = current_session[:browser].upcase!
+    unique_browsers << browser if !unique_browsers.include?(browser)
+  end
+```
+Была убраны лишнии проверки `cols = line.split(',')` -> `if cols[0] == 'user'` или `if cols[0] == 'session'`. Сделал сразу проверку строки `start_with?` на наличие юзера или сессии.
+Также сразу считаем общее кол-во сессий и уникальных браузеров.
+
+Были удалены повторяющиеся `collect_stats_from_users`, которые постоянно гоняли `user_objects`, когда можно было по уже готовому массиву `users` пройти один раз, записав все нужные значения для отчета, что и было сделано.
+
+После всех этих манипуляций итоговое время выполнение кода стало ~ 3.15 секунды.
+
+### Оптимизация №3
+Далее самой прожорливой строчкой была `task-1.rb:140`, а именно метод `Date#parse`, на который было затрачено 17мб памяти. Путем гугления `Date#parse ruby benchmark` выяснил, что есть быстрее метод - `Date#strptime`. Замена на него снизило выделение памяти до 6.1мб, а общее время выполнения кода стало ~ 2.45 секунд.
 
 ## Результаты
-В результате проделанной оптимизации наконец удалось обработать файл с данными.
-Удалось улучшить метрику системы с *того, что у вас было в начале, до того, что получилось в конце*
+До всех изменений общее время выполнения кода было ~ **19 секунд**, памяти выделялось на все это ~ **1685мб**.
+После проделанных манипуляций код выполнялся за **2.45 секунды**, а выделение памяти было около **45мб**.
 
-*Какими ещё результами можете поделиться*
+В результате проделанной оптимизации наконец удалось обработать файл с данными.
+Код выполнился за **86 секунд**, что считаю более или менее приемлемым результатом.
 
 ## Защита от регресса производительности
-Для защиты от потери достигнутого прогресса при дальнейших изменениях программы сделано *то, что вы для этого сделали*
+Для защиты от потери достигнутого прогресса при дальнейших изменениях программы нужно зафиксировать полученные время (+-1 секунда).

data.txt

@@ -0,0 +1,18 @@
+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

result.json

@@ -0,0 +1 @@
+{"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"]}}}

task-1.rb

@@ -1,58 +1,50 @@
-# Deoptimized version of homework task
-
 require 'json'
 require 'pry'
 require 'date'
 require 'minitest/autorun'
-
-class User
-  attr_reader :attributes, :sessions
-
-  def initialize(attributes:, sessions:)
-    @attributes = attributes
-    @sessions = sessions
-  end
-end
+require 'benchmark'
+require 'stackprof'
+require 'memory_profiler'
 
 def parse_user(user)
   fields = user.split(',')
-  parsed_result = {
-    'id' => fields[1],
-    'first_name' => fields[2],
-    'last_name' => fields[3],
-    'age' => fields[4],
+
+  {
+    id: fields[1],
+    full_name: "#{fields[2]} #{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],
+
+  {
+    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']}"
-    report['usersStats'][user_key] ||= {}
-    report['usersStats'][user_key] = report['usersStats'][user_key].merge(block.call(user))
-  end
-end
+def report(file)
+  users = []
+  unique_browsers = []
+  sessions = {}
+  total_sessions = 0
 
-def work
-  file_lines = File.read('data.txt').split("\n")
+  File.read(file).split("\n").each do |line|
+    users << parse_user(line) if line.start_with?('user')
+    next if !line.start_with?('session')
 
-  users = []
-  sessions = []
+    current_session = parse_session(line)
+    sessions[current_session[:user_id]] ||= []
+    sessions[current_session[:user_id]] << current_session
 
-  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'
+    total_sessions += 1
+    browser = current_session[:browser].upcase!
+    unique_browsers << browser if !unique_browsers.include?(browser)
   end
 
   # Отчёт в json
@@ -72,77 +64,55 @@ def work
 
   report = {}
 
+  # Подсчет всех юзеров
   report[:totalUsers] = users.count
-
   # Подсчёт количества уникальных браузеров
-  uniqueBrowsers = []
-  sessions.each do |session|
-    browser = session['browser']
-    uniqueBrowsers += [browser] if uniqueBrowsers.all? { |b| b != browser }
-  end
-
-  report['uniqueBrowsersCount'] = uniqueBrowsers.count
-
-  report['totalSessions'] = sessions.count
-
-  report['allBrowsers'] =
-    sessions
-      .map { |s| s['browser'] }
-      .map { |b| b.upcase }
-      .sort
-      .uniq
-      .join(',')
-
+  report[:uniqueBrowsersCount] = unique_browsers.count
+  # Подсчёт количества сессий
+  report[:totalSessions] = total_sessions
+  # Собираем все браузеры
+  report[:allBrowsers] = unique_browsers.sort.join(',')
   # Статистика по пользователям
-  users_objects = []
+  report[:usersStats] = {}
 
   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]
+    user_sessions = sessions.delete(user[:id]) || []
+    time_of_sessions = user_sessions.map { |session| session[:time].to_i }
+    user_browsers = user_sessions.map { |session| session[:browser] }
+
+    report[:usersStats][user[:full_name]] = {
+      # Кол-во сессией по пользователю
+      sessionsCount: user_sessions.count,
+      # Кол-во времени по пользователям
+      totalTime: "#{time_of_sessions.sum} min.",
+      # Самая длинная сессия пользователя
+      longestSession: "#{time_of_sessions.max} min.",
+      # Браузеры пользователя через запятую
+      browsers: user_browsers.sort.join(', '),
+      # Хоть раз использовал IE?
+      usedIE: user_browsers.any? { |browser| browser.upcase =~ /INTERNET EXPLORER/ },
+      # Всегда использовал только Chrome?
+      alwaysUsedChrome: user_browsers.all? { |browser| browser.upcase =~ /CHROME/ },
+      # Даты сессий через запятую в обратном порядке в формате iso8601
+      dates: user_sessions.map { |s| Date.strptime(s[:date], '%Y-%m-%d') }.sort.reverse.map(&:iso8601)
+    }
   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
+  File.write('result.json', "#{report.to_json}\n")
+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 } }
+time = Benchmark.realtime do
+  StackProf.run(mode: :object, out: 'stackprof.dump', raw: true) do
+    report('data.txt')
   end
+end
 
-  File.write('result.json', "#{report.to_json}\n")
+data = StackProf.run(mode: :object) do
+  report('data.txt')
 end
 
+puts "Finish in #{time.round(2)}"
+
 class TestMe < Minitest::Test
   def setup
     File.write('result.json', '')
@@ -169,7 +139,7 @@ def setup
   end
 
   def test_result
-    work
+    report('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