Find hardcode-dev#1: unefficient user sessions selection

Author: smaximov

.gitignore

@@ -1,2 +1,4 @@
 /samples/*
-!/samples/.keep
+!/samples/.keep
+/tmp/*
+!/tmp/.keep

bin/ruby-prof

@@ -0,0 +1,56 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+require_relative '../boot'
+
+require 'optparse'
+require 'ruby-prof'
+require 'tempfile'
+
+require_relative '../samples'
+
+RubyProf.measure_mode = RubyProf::WALL_TIME
+
+REPORTS_ROOT = Pathname(__dir__).dirname / 'tmp'
+PRINTERS = %i(flat graph callstack callgrind).freeze
+
+opts = {
+  printer: :flat
+}
+
+options_parser = OptionParser.new do |parser|
+  parser.banner = "Usage: bin/ruby-prof [options] SAMPLE_SIZE"
+
+  parser.on('--printer [PRINTER]', PRINTERS,
+            "Printer used to present collected data (#{PRINTERS.join(', ')}) ",
+            "[default: #{opts[:printer]}]")
+  parser.on('-h', '--help', 'Display this message') do
+    puts parser.help
+    exit
+  end
+end
+
+options_parser.parse!(into: opts)
+
+abort(options_parser.help) if ARGV.size != 1
+
+GC.disable
+
+sample = Sample.new(ARGV.first)
+
+report = RubyProf.profile do
+  Tempfile.create do |result|
+    work(src: sample.path, dest: result.path)
+  end
+end
+
+case opts[:printer]
+when :flat
+  RubyProf::FlatPrinter.new(report).print(File.open(REPORTS_ROOT / 'flat.txt', 'w'))
+when :graph
+  RubyProf::GraphHtmlPrinter.new(report).print(File.open(REPORTS_ROOT / 'graph.html', 'w'))
+when :callstack
+  RubyProf::CallStackPrinter.new(report).print(File.open(REPORTS_ROOT / 'callstack.html', 'w'))
+when :callgrind
+  RubyProf::CallTreePrinter.new(report).print(path: REPORTS_ROOT, profile: 'callgrind')
+end

case-study.md

@@ -115,15 +115,49 @@ irb(main):002:0> (factor * factor * 177.3695).round
 Пункты 3-4 реализованы в скрипте `bin/feedback`.
 
 ## Вникаем в детали системы, чтобы найти главные точки роста
-Для того, чтобы найти "точки роста" для оптимизации я воспользовался *инструментами, которыми вы воспользовались*
+Для того, чтобы найти "точки роста" для оптимизации, я воспользовался следующими инструментами:
 
-Вот какие проблемы удалось найти и решить
+- `ruby-prof` (вызывается через скрипт `bin/ruby-prof`).
+- `stack-prof`
 
-### Ваша находка №1
-- какой отчёт показал главную точку роста
-- как вы решили её оптимизировать
-- как изменилась метрика
-- как изменился отчёт профилировщика - исправленная проблема перестала быть главной точкой роста?
+Я также воспользовался `rbspy` на `samples/data_large.txt`, однако на данном скрипте какой-либо полезной информации
+он не показал:
+
+```
+% self  % total  name
+ 99.58    99.97  block in work - /home/nameless/edu/ruby-opt/rails-optimization-task1/task-1.rb
+  0.50   100.00  <c function> - unknown
+ [... snip ...]
+```
+
+Кроме того, я запускал `rubocop` с метриками `performance`, однако он тоже ничего полезного не предложил.
+
+Вот какие проблемы удалось найти и решить.
+
+### Находка №1: очень неэффективная выборка сессий пользователя
+
+- Я построил `callgrind`-отчёт с помощью `ruby-prof` и попытался проанализировать его с помощью `kcachegrind`.
+
+   Во разделе "Flat & Profile" сразу бросается в глаза метод `Array#select`, у в котором программа провела 96%
+   времени (Self) и у которого большое число вызовов (~3000). Дальше я посмотрел вкладку "All Callers", по
+   по которой видно, что большее число раз этот метод вызывался в методе `Array#each`, который в свою очередь
+   больше всего вызывался из `#work`.
+
+   В качестве точки роста я выбрал `Array#select` (я также смотрел другие отчёты `ruby-prof`, они показали примерно
+   ту же информацию).
+
+- В исходном коде только один раз встречается метод `Array#select`, а именно при выборе сессий пользователя из
+   общего массива сессий. Очевидно, что время выполнения этого кода будет расти с увеличением размера данных. Но
+   если бы мы заранее сформировали индекс пользовательских сессий в виде хэш-таблицы с ID пользователя в качестве
+   ключа, то выбор сессий для конкретного пользователя обладало бы характеристиками получения значения в хэше
+   по ключу: 0(1).
+
+   Поэтому я решил во время хэш `user_id => user_sessions`, а заодно избавиться от массивов `users` и и `sessions`,
+   т.к. они используется в основном для формирования массива `users_objects`, и сразу формировать `users_objects`.
+   Значения `totalUsers`, `uniqueBrowsersCount`, `totalSessions`, `allBrowsers` же, которые тоже использовали эти
+   массивы, можно посчитать и без них.
+- Значение метрики для 20000 строк снизилось с 4.5404 секунд до 0.1924 секунд.
+- В отчёте профилировщика `Array#select` перестал быть главной точкой роста.
 
 ### Ваша находка №2
 - какой отчёт показал главную точку роста

task-1.rb

@@ -3,19 +3,25 @@
 require 'json'
 require 'pry'
 require 'date'
+require 'set'
 
 class User
-  attr_reader :attributes, :sessions
+  attr_reader :attributes
 
   def initialize(attributes:, sessions:)
     @attributes = attributes
-    @sessions = sessions
+    @all_sessions = sessions
+  end
+
+  def sessions
+    @sessions ||= @all_sessions[attributes['id']]
   end
 end
 
 def parse_user(user)
   fields = user.split(',')
-  parsed_result = {
+
+  {
     'id' => fields[1],
     'first_name' => fields[2],
     'last_name' => fields[3],
@@ -25,7 +31,8 @@ def parse_user(user)
 
 def parse_session(session)
   fields = session.split(',')
-  parsed_result = {
+
+  {
     'user_id' => fields[1],
     'session_id' => fields[2],
     'browser' => fields[3],
@@ -45,13 +52,31 @@ def collect_stats_from_users(report, users_objects, &block)
 def work(src:, dest:)
   file_lines = File.read(src).split("\n")
 
-  users = []
-  sessions = []
+  report = {}
+
+  totalUsers = 0
+  totalSessions = 0
+  uniqueBrowsers = Set.new
+
+  users_objects = []
+  sessions = Hash.new { |hash, user_id| hash[user_id] = [] }
 
   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 cols[0]
+    when 'user'
+      user = parse_user(line)
+      users_objects << User.new(attributes: user, sessions: sessions)
+
+      totalUsers += 1
+    when 'session'
+      session = parse_session(line)
+      sessions[session['user_id']] << session
+
+      totalSessions += 1
+      uniqueBrowsers << session['browser'].upcase
+    end
   end
 
   # Отчёт в json
@@ -69,39 +94,10 @@ def work(src:, dest:)
   #     - Всегда использовал только Хром? +
   #     - даты сессий в порядке убывания через запятую +
 
-  report = {}
-
-  report[:totalUsers] = users.count
-
-  # Подсчёт количества уникальных браузеров
-  uniqueBrowsers = []
-  sessions.each do |session|
-    browser = session['browser']
-    uniqueBrowsers += [browser] if uniqueBrowsers.all? { |b| b != browser }
-  end
-
+  report['totalUsers'] = totalUsers
   report['uniqueBrowsersCount'] = uniqueBrowsers.count
-
-  report['totalSessions'] = sessions.count
-
-  report['allBrowsers'] =
-    sessions
-      .map { |s| s['browser'] }
-      .map { |b| b.upcase }
-      .sort
-      .uniq
-      .join(',')
-
-  # Статистика по пользователям
-  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]
-  end
-
+  report['totalSessions'] = totalSessions
+  report['allBrowsers'] = uniqueBrowsers.to_a.sort.join(',')
   report['usersStats'] = {}
 
   # Собираем количество сессий по пользователям