Perform optimization

Author: maksim.rabau

benchmark.rb

@@ -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("slow string concatenation") do
+    work('data_large.txt', disable_gc: false)
+  end
+end

case-study.md

@@ -0,0 +1,96 @@
+# Case-study оптимизации
+
+## Актуальная проблема
+В нашем проекте возникла серьёзная проблема.
+
+Необходимо было обработать файл с данными, чуть больше ста мегабайт.
+
+У нас уже была программа на `ruby`, которая умела делать нужную обработку.
+
+Она успешно работала на файлах размером пару мегабайт, но для большого файла она работала слишком долго, и не было понятно, закончит ли она вообще работу за какое-то разумное время.
+
+Я решил исправить эту проблему, оптимизировав эту программу.
+
+## Формирование метрики
+Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумал использовать такую метрику: *время выполнения в секундах*
+
+## Гарантия корректности работы оптимизированной программы
+Программа поставлялась с тестом. Выполнение этого теста в фидбек-лупе позволяет не допустить изменения логики программы при оптимизации.
+
+## Feedback-Loop
+Для того, чтобы иметь возможность быстро проверять гипотезы я выстроил эффективный `feedback-loop`, который позволил мне получать обратную связь по эффективности сделанных изменений за *29 секунд*
+
+Вот как я построил `feedback_loop`: 
+1) Использовал профилировщик `RubyProf Flat` с выключенным GC (по началу на объеме в 50к строк)
+2) Находил главную точку роста и начинал править ровно с этого места
+3) Перезапускал профилировщик повторно и смотрел на полученный результат + непосредственно сам скрипт с включенным GC
+4) Увеличивал объем данных и повторно начинал с пункта 1) 
+
+## Вникаем в детали системы, чтобы найти главные точки роста
+Для того, чтобы найти "точки роста" для оптимизации я воспользовался `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%
+
+### Ваша находка №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 вместо хранения в виде массива строк
+- Программа перестала убиваться системой
+
+### Ваша находка №3
+- `RubyProf Flat` показывает большой % использования `Array#each`
+- Видно, что логика для построения статистики по каждому пользователю написана криво с использованием огромного кол-ва переборов. Нет смысла каждый раз итерироваться по всему массиву при построении отдельной статистики (т.е нет смысла начинать итерацию с нуля при построении sessionsCount, totalTime, longestSession, browsers, usedIE, alwaysUsedChrome и dates - можно строить все нужные статистики на текущей итерации)
+- После рефакторинга `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' более чем достаточно
+- После рефакторинга `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
+                   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
+```
+

rubyprof_flat.rb

@@ -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+"))

task-1.rb

@@ -5,12 +5,43 @@
 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/ } },
+  '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, :time
+
+  def initialize(attributes:)
     @attributes = attributes
-    @sessions = sessions
+    @time = attributes['time']
   end
 end
 
@@ -35,26 +66,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
   #   - Сколько всего юзеров +
   #   - Сколько всего уникальных браузеров +
@@ -70,74 +88,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(',')
 
-  # Собираем количество сессий по пользователям
-  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
 
-  # Выбираем самую длинную сессию пользователя
-  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")
@@ -169,7 +163,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