last loop

Author: aleksey.ryabchikov

Gemfile

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+source 'https://rubygems.org'
+
+gem 'ruby-prof'
+gem 'rspec-benchmark'
+gem 'ruby-progressbar'
+gem 'stackprof'
+gem 'pry'
+gem 'minitest'

Gemfile.lock

@@ -0,0 +1,48 @@
+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.4)
+    pry (0.14.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.6.3)
+    ruby-progressbar (1.13.0)
+    stackprof (0.2.27)
+
+PLATFORMS
+  x86_64-linux
+
+DEPENDENCIES
+  minitest
+  pry
+  rspec-benchmark
+  ruby-prof
+  ruby-progressbar
+  stackprof
+
+BUNDLED WITH
+   2.4.13

case-study.md

@@ -29,7 +29,7 @@
 - С помощью профилировщика найти главную точку роста (Профилируем с выключенным GC предварительно прогрев кеши)
 - Внести оптимизационные правки
 - С помощью профилировщика проверить есть ли улучшения
-- Запустить тест, проверить если улучшения есть, если да то закоммитить.
+- Запустить тест, проверить, если улучшения есть тогда закоммитить.
 
 ## Вникаем в детали системы, чтобы найти главные точки роста
 Для того, чтобы найти "точки роста" для оптимизации я воспользовался rbspy (удобно, потому что встроен в rubymine), stackprof и ruby-prof в разных режимах отчетов
@@ -46,6 +46,7 @@
 - Заменил перебор всех сессий на хэш с группированные данных по user_id. В данном конкретном месте алгоритмическая сложность с O(n) изменилась на O(1)
 - Метрика кратно уменьшилась при прогоне теста на перфоманс с средних 5 сек. до 0.4 сек. Это и было самым узким местом программы по всей видимости.
 - Повторный запуск профилировщика показал, что вместо 89,15% теперь это место занимает 0.3%.
+- В отчетах профилировщика эта точка роста перестала быть главной.
 
 ### Ваша находка №2
 - Профилировщики указали на следующую точку роста:
@@ -56,6 +57,7 @@
 - Заменил `sessions = sessions + [parse_session(line)] if cols[0] == 'session'` на `sessions = sessions << parse_session(line) if cols[0] == 'session'`
 Известная проблема в ruby. Оператор << позволяет не создавать новую переменную каждый раз, а писать все в существующую.
 - При прогоне теста, среднее значение метрики упало с 0.4 сек. до 0.25 сек.
+- В отчетах профилировщика эта точка роста перестала быть главной.
 
 ### Ваша находка №3
 - Ради интереса прогнал тесты с разным объемом данных, 20к, 40к, 80к и так далее. В том числе и на полном файле.
@@ -67,15 +69,47 @@
 ```
 - Оптимизировал блок each, заменил '+' на Set.
 - При прогоне теста, среднее значение метрики упало с 0.25 сек. до 0.18 сек.
+- В отчетах профилировщика эта точка роста перестала быть главной.
 
 ### Ваша находка №4
-- Попробовал увеличить кол-во данных, чтобы проще было увидеть проблематику до 100_000 строк.
-- По отчетам нашел новую точку роста, это метод `collect_stats_from_users`, он аффектит на два проблемных места сразу
+- По отчетам профилировщиков нашел новую точку роста, это метод `def collect_stats_from_users`
 ```
   %Total	%Self	Total	Self	Wait	Child	Calls	Name
   89.28%	23.89%	1.43	0.38	0.00	1.05	10	Array#each
 ```
-- Оптимизировал, а именно избавился от collect_stats_from_users и начал подготавливать данные за один проход.
+- Оптимизировал, а именно избавился от collect_stats_from_users и начал подготавливать данные для отчета за один проход.
 - При прогоне теста, среднее значение метрики упало с 0.18 сек. до 0.14 сек. на 20_000 строк.
+- В отчетах профилировщика эта точка роста перестала быть главной.
 
+### Ваша находка №5
+- stackprof показал на точку роста на строке с парсингом даты `dates = sessions.map { |s| Date.parse(s['date']) }`
 
+```
+  %Total	%Self	Total	Self	Wait	Child	Calls	Name
+  28.57%	16.02%	0.09	0.05	0.00	0.04	16954	<Class::Date>#parse
+```
+- Поправил, Date.parse лишняя обработка.
+- Среднее значение метрики упало с 0.14 до 0.2 сек.
+- В отчетах профилировщика эта точка роста перестала быть главной.
+
+### Ваша находка №6
+- Уже правктически выполняется бюджет прогона файла data_large.txt, но профилировщик подсветил еще одну точку роста:
+```
+%Total	%Self	Total	Self	Wait	Child	Calls	Name
+30.73%	30.73%	0.06	0.06	0.00	0.00	40001	String#split
+```
+- Выпилил лишние String.split 
+- Так как выполнение теста на 20_000 строк очень мало, увеличил файл до 60_000 строк.
+Среднее значение прогода 60_000 строк = 0.3 сек.
+- Среднее значение прогона всего файла с ~30 сек. упало до 27 сек.
+- В отчетах профилировщика эта точка роста перестала быть главной.
+- Вижу еще несколько точек роста, но решил на этом остановится =)
+
+## Результаты
+В результате проделанной оптимизации наконец удалось обработать файл с данными.
+Удалось улучшить метрику системы на 20_000 строк с 5 сек. до 0.1 сек., на 60_000 строк с до 0.3 сек и уложиться в заданный бюджет.
+Так же был переписан тест с minitest на rspec и вынесен в отдельный файл.
+
+## Защита от регрессии производительности
+Для защиты от потери достигнутого прогресса при дальнейших изменениях программы было написано два performance теста, один для проверки на 60_000 строках,
+второй на полном объеме файла data_large.txt

spec/task-1_spec.rb

@@ -3,20 +3,12 @@
 
 describe 'task-1' do
   let(:result) { './spec/fixtures/files/result.json' }
+
   describe 'Result' do
     let(:file) { './spec/fixtures/files/data.txt' }
 
     let(:expected_result) do
-      '{"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"
+      '{"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"
     end
 
     it 'returns equal' do
@@ -30,14 +22,14 @@
 
     describe 'with partial file' do
       it 'works under 0.4 sec' do
-        expect { work(file) }.to perform_under(0.14).sec.warmup(2).times.sample(10).times
+        expect { work(file) }.to perform_under(0.3).sec.warmup(2).times.sample(10).times
       end
     end
 
-    # describe 'with full file' do
-    #   it 'works under 30 sec' do
-    #     expect { work('./data_large.txt') }.to perform_under(30).sec
-    #   end
-    # end
+    describe 'with full file' do
+      it 'works under 30 sec' do
+        expect { work('./data_large.txt') }.to perform_under(27).sec
+      end
+    end
   end
 end

task-1.rb

@@ -15,19 +15,17 @@ 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],
   }
 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],
@@ -52,8 +50,8 @@ def work(file_path, result_path = 'spec/fixtures/files/result.json')
 
   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'
+    users = users << parse_user(cols) if cols[0] == 'user'
+    sessions = sessions << parse_session(cols) if cols[0] == 'session'
   end
 
   # Отчёт в json
@@ -108,7 +106,6 @@ def work(file_path, result_path = 'spec/fixtures/files/result.json')
     sessions = user.sessions
     times = sessions.map { |s| s['time'].to_i }
     browsers = sessions.map { |s| s['browser'].upcase }
-    dates = sessions.map { |s| Date.parse(s['date']) }
 
     report['usersStats'][user_key] = {
       # Количество сессий
@@ -124,7 +121,7 @@ def work(file_path, result_path = 'spec/fixtures/files/result.json')
       # Всегда использовал только Chrome?
       'alwaysUsedChrome' => browsers.all? { |b| b.include?('CHROME') },
       # Даты сессий через запятую в обратном порядке в формате iso8601
-      'dates' => dates.sort.reverse.map(&:iso8601)
+      'dates' => user.sessions.map { |s| s['date'] }.sort.reverse
     }
   end