From 208b9ebeda8b23018274aeb86b994cc0090299a7 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 21 Mar 2023 15:42:33 +0300 Subject: [PATCH 01/34] Add tools --- Gemfile | 2 ++ Gemfile.lock | 26 +++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index e20b1260..c37eac38 100644 --- a/Gemfile +++ b/Gemfile @@ -24,3 +24,5 @@ end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] + +gem 'perfolab', github: 'Tab10id/perfolab', branch: :temp_26 diff --git a/Gemfile.lock b/Gemfile.lock index fccf6f5f..7495eec4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,15 @@ +GIT + remote: https://github.com/Tab10id/perfolab.git + revision: b75ed48af9aa69affc28a4034d47221a1690db26 + branch: temp_26 + specs: + perfolab (0.1.0) + memory_profiler (~> 1.0) + ruby-prof (~> 1.4, < 1.4.4) + stackprof (~> 0.2.0) + tabulo (~> 2.0) + zeitwerk (~> 2.6) + GEM remote: https://rubygems.org/ specs: @@ -67,8 +79,11 @@ GEM mini_mime (>= 0.1.1) marcel (0.3.3) mimemagic (~> 0.3.2) + memory_profiler (1.0.1) method_source (0.9.2) - mimemagic (0.3.3) + mimemagic (0.3.10) + nokogiri (~> 1) + rake mini_mime (1.0.1) mini_portile2 (2.4.0) minitest (5.11.3) @@ -109,6 +124,7 @@ GEM rb-fsevent (0.10.3) rb-inotify (0.10.0) ffi (~> 1.0) + ruby-prof (1.4.3) ruby_dep (1.5.0) sprockets (3.7.2) concurrent-ruby (~> 1.0) @@ -117,10 +133,16 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) + stackprof (0.2.24) + tabulo (2.8.2) + tty-screen (= 0.8.1) + unicode-display_width (~> 2.2) thor (0.20.3) thread_safe (0.3.6) + tty-screen (0.8.1) tzinfo (1.2.5) thread_safe (~> 0.1) + unicode-display_width (2.4.2) web-console (3.7.0) actionview (>= 5.0) activemodel (>= 5.0) @@ -129,6 +151,7 @@ GEM websocket-driver (0.7.0) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.3) + zeitwerk (2.6.7) PLATFORMS ruby @@ -137,6 +160,7 @@ DEPENDENCIES bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) + perfolab! pg (>= 0.18, < 2.0) puma (~> 3.11) rails (~> 5.2.3) From bf737ab4ad29207b142b6df529c1f3ff402da02f Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 21 Mar 2023 15:44:22 +0300 Subject: [PATCH 02/34] Extract rake task to separate class for simplify optimisations --- lib/my_app/import.rb | 42 ++++++++++++++++++++++++++++++++++++++++++ lib/tasks/utils.rake | 31 ++----------------------------- 2 files changed, 44 insertions(+), 29 deletions(-) create mode 100644 lib/my_app/import.rb diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb new file mode 100644 index 00000000..b92f0516 --- /dev/null +++ b/lib/my_app/import.rb @@ -0,0 +1,42 @@ +module MyApp + class Import + attr_reader :file_name + + def initialize(file_name) + @file_name = file_name + end + + def call + json = JSON.parse(File.read(file_name)) + + ActiveRecord::Base.transaction do + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + ActiveRecord::Base.connection.execute('delete from buses_services;') + + json.each do |trip| + from = City.find_or_create_by(name: trip['from']) + to = City.find_or_create_by(name: trip['to']) + services = [] + trip['bus']['services'].each do |service| + s = Service.find_or_create_by(name: service) + services << s + end + bus = Bus.find_or_create_by(number: trip['bus']['number']) + bus.update(model: trip['bus']['model'], services: services) + + Trip.create!( + from: from, + to: to, + bus: bus, + start_time: trip['start_time'], + duration_minutes: trip['duration_minutes'], + price_cents: trip['price_cents'], + ) + end + end + end + end +end diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe871..1608ba4c 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,34 +1,7 @@ # Наивная загрузка данных из json-файла в БД # rake reload_json[fixtures/small.json] task :reload_json, [:file_name] => :environment do |_task, args| - json = JSON.parse(File.read(args.file_name)) + require Rails.root.join('lib/my_app/import') - ActiveRecord::Base.transaction do - City.delete_all - Bus.delete_all - Service.delete_all - Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') - - json.each do |trip| - from = City.find_or_create_by(name: trip['from']) - to = City.find_or_create_by(name: trip['to']) - services = [] - trip['bus']['services'].each do |service| - s = Service.find_or_create_by(name: service) - services << s - end - bus = Bus.find_or_create_by(number: trip['bus']['number']) - bus.update(model: trip['bus']['model'], services: services) - - Trip.create!( - from: from, - to: to, - bus: bus, - start_time: trip['start_time'], - duration_minutes: trip['duration_minutes'], - price_cents: trip['price_cents'], - ) - end - end + MyApp::Import.new(args.file_name).call end From 2758b67d5d654223608392cc286b43455e2fd367 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Thu, 30 Mar 2023 14:58:54 +0300 Subject: [PATCH 03/34] Prepare code for analyze --- perfolab/my_app/import_stand.rb | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 perfolab/my_app/import_stand.rb diff --git a/perfolab/my_app/import_stand.rb b/perfolab/my_app/import_stand.rb new file mode 100644 index 00000000..68646aba --- /dev/null +++ b/perfolab/my_app/import_stand.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'my_app/import' +require 'perfolab' + +loop = + PerfoLab::Loop.new do |toolbox| + toolbox.add_tool( + :stackprof, + type: :stackprof, + config: { + raw: true + }, + runner_options: { + arguments: ['small'] + } + ) + toolbox.add_tool( + :benchmark, + type: :benchmark, + runner_options: { + warmup: 1, + arguments: [ + 'small', + # 'medium', + # 'large' + ] + } + ) + end + +loop.analyze do |filename| + MyApp::Import.new(Rails.root.join('fixtures', "#{filename}.json")).call +end From 438b5ce68d652d8a0c220e68b255acd3ae0ab764 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Thu, 30 Mar 2023 16:52:45 +0300 Subject: [PATCH 04/34] Add activerecord-import --- Gemfile | 2 ++ Gemfile.lock | 3 +++ 2 files changed, 5 insertions(+) diff --git a/Gemfile b/Gemfile index c37eac38..e90a9ca9 100644 --- a/Gemfile +++ b/Gemfile @@ -26,3 +26,5 @@ end gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] gem 'perfolab', github: 'Tab10id/perfolab', branch: :temp_26 + +gem "activerecord-import", "~> 1.4" diff --git a/Gemfile.lock b/Gemfile.lock index 7495eec4..2fe4162b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,6 +45,8 @@ GEM activemodel (= 5.2.3) activesupport (= 5.2.3) arel (>= 9.0) + activerecord-import (1.4.1) + activerecord (>= 4.2) activestorage (5.2.3) actionpack (= 5.2.3) activerecord (= 5.2.3) @@ -157,6 +159,7 @@ PLATFORMS ruby DEPENDENCIES + activerecord-import (~> 1.4) bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) From 1bd2f31a4dbd5059b07490bfd9c9b65bd40f9fd2 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Fri, 31 Mar 2023 00:05:18 +0300 Subject: [PATCH 05/34] initial case-study --- case-study.md | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 case-study.md diff --git a/case-study.md b/case-study.md new file mode 100644 index 00000000..21edcaa6 --- /dev/null +++ b/case-study.md @@ -0,0 +1,65 @@ +# Оптимизация взаимодействия с БД + +## User story + +Тут должны быть описаны пльзовательские истории, которые определят целевые +значения метрик, но так как их нет, то я их придумаю. + +### Импорт данных + +Для проведения нагрузочного тестирования на тестовом стенде необходимо регулярно +импортировать различные наборы данных о расписании автобусов из больших +текстовых файлов, файлы получаются из внешних систем поэтому нет возможности +осуществлять накат предварительно сформированного sql-дампа. +Текущее решение не позволяет выполять эту операцию быстро из-за чего подготовка +тестовой среды происходит непозволительно долгое время чем блокирует всю +дальнейшую работу пайплайна. + +### Отображение расписания + +Система веб-аналитики показывает, что при просмотре расписаний на популярных +направлениях пользователи не дожидаются загрузки страницы и уходят с сайта. +Британские учёные доказали, что финансово успешные web-проекты должны отображать +информацию пользователю не позже чем через 3 секунды после запроса. + +## Что сделать + +* Нужно оптимизировать механизм перезагрузки расписания из файла так, + * файл large.json в пределах минуты. +* Необходимо ускорить отображение расписаний + * страница автобусы/Самара/Москва должна открываться за адекватное время + +# Оптимизация импорта + +## Основные инструменты для исследования + +* perfolab (чуть доведенный до ума фреймворк разработанный мной в рамках предыдущего задания) +* stackprof +* speedscope + +## Основные проблемы на начальном этапе + +* Отсутствие тестов. + * Нет возможности проводить оптимизацию без опасения что-либо сломать. +* Вся процедура выполняется в едином блоке кода без разбивки на методы. + * Сложно выяснить что именно вызывает основные проблемы. + +## Первичный анализ + +Замеры времени импорта показывют что оно равстёт приблизительно линейно в +зависимости от размера файла, что несложно увидеть и при обычном чтении +оригинального скрипта. Инструменты показывают что большую часть времени +выполняются запросы на поиск и создание объектов. + +Уже на данном этапе очевидно что необходимо кардинально уменьшать количество +отправляемых в БД запросов. Внимательное изучение скрипт показывает что +от запросов на поиск данных в бд можно полностью избавиться, так как изначально +в БД нет никаких данных. Уменьшения же количества запросов на вставку данных +можно с помощью массового создания объектов. + +Для удобства можно воспользоваться библиотекой activerecord-import. + +## Первые шаги + +* Код импорта был выделен в отдельный класс для удобства анализа и тестрования. +* Был написан минимальный тест на корректнось работы скрипта. \ No newline at end of file From eda88113437b5e18c86c371393bc10767fc1ddd9 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Fri, 31 Mar 2023 00:42:58 +0300 Subject: [PATCH 06/34] Add basic test --- fixtures/example.json | 6 +++--- test/lib/my_app/import_test.rb | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 test/lib/my_app/import_test.rb diff --git a/fixtures/example.json b/fixtures/example.json index 510b4c2d..2ce50934 100644 --- a/fixtures/example.json +++ b/fixtures/example.json @@ -46,10 +46,10 @@ }, { "bus": { - "model": "Икарус", - "number": "123", + "model":"ГАЗ", + "number":"584", "services": [ - "Туалет", + "Кондиционер общий", "WiFi" ] }, diff --git a/test/lib/my_app/import_test.rb b/test/lib/my_app/import_test.rb new file mode 100644 index 00000000..565b05b8 --- /dev/null +++ b/test/lib/my_app/import_test.rb @@ -0,0 +1,29 @@ +require 'test_helper' +require 'my_app/import' + +class TestMe < Minitest::Test + def test_result + MyApp::Import.new(Rails.root.join("fixtures", 'example.json')).call + assert_equal 10, Trip.count + assert_equal 2, Bus.count + assert_equal 3, Service.count + assert_equal 2, City.count + gaz = Bus.find_by!(number: '584', model: 'ГАЗ') + icarus = Bus.find_by!(number: '123', model: 'Икарус') + assert_equal ['Кондиционер общий', 'WiFi'], gaz.services.map(&:name) + assert_equal ['Туалет', 'WiFi'], icarus.services.map(&:name) + assert_equal 1, gaz.trips.size + gaz_trip = gaz.trips.first + gaz_trip_attributes = gaz_trip.attributes + assert_equal( + { + 'duration_minutes' => 315, + 'price_cents' => 969, + 'start_time' => '18:30', + }, + gaz_trip_attributes.slice('duration_minutes', 'price_cents', 'start_time') + ) + assert_equal 'Самара', gaz_trip.from.name + assert_equal 'Москва', gaz_trip.to.name + end +end \ No newline at end of file From f06956466a2eed95699434ce3d00a7d1b7f38a9d Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 17:35:20 +0300 Subject: [PATCH 07/34] Add script for performance analyse only after tests run --- loop.sh | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100755 loop.sh diff --git a/loop.sh b/loop.sh new file mode 100755 index 00000000..a8a02ee9 --- /dev/null +++ b/loop.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash + +set -e + +bundle exec rake test + +export RAILS_ENV=production +bundle exec ruby bin/rails runner perfolab/my_app/import_stand.rb \ No newline at end of file From 60b0a152b959201e2c76df30bdf72affdb57efb0 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 17:40:39 +0300 Subject: [PATCH 08/34] Add first measure results --- case-study.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/case-study.md b/case-study.md index 21edcaa6..5be2af44 100644 --- a/case-study.md +++ b/case-study.md @@ -62,4 +62,10 @@ ## Первые шаги * Код импорта был выделен в отдельный класс для удобства анализа и тестрования. -* Был написан минимальный тест на корректнось работы скрипта. \ No newline at end of file +* Был написан минимальный тест на корректнось работы скрипта. +* Произведен первичный анализ скрипта на файле small.json + +| benchmark | Previous | Current | Diff % | +|--------------------------------|----------|-----------------|-----------------| +| total | | small: 3014ms | | + From a1117aa4948dfa7fee6338c46a3c714e7fdfe36c Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 18:10:44 +0300 Subject: [PATCH 09/34] Extract model creation to separate methods --- case-study.md | 21 ++++++++++++--- lib/my_app/import.rb | 63 +++++++++++++++++++++++++++++++------------- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/case-study.md b/case-study.md index 5be2af44..c78e1ee4 100644 --- a/case-study.md +++ b/case-study.md @@ -65,7 +65,22 @@ * Был написан минимальный тест на корректнось работы скрипта. * Произведен первичный анализ скрипта на файле small.json -| benchmark | Previous | Current | Diff % | -|--------------------------------|----------|-----------------|-----------------| -| total | | small: 3014ms | | +| benchmark | Previous | Current | Diff % | +|--------------------------------|----------|---------|-----------------| +| total | | 3014ms | | + +## Выделение методов + +Хоть у нас и есть инсайдерская информация о частоте появления уникальных записей +различных моделей, всё же будет намного удобнее ориентироваться в результатах +профилировщиков если выделить создание разных объектов в отельные методы. +Как и ожидалось, выделение методов не оказало значительного влияния на время +работы импорта. + +| benchmark | Previous | Current | Diff % | +|--------------------------------|-----------|---------|----------------| +| total | 3014ms | 2976ms | -1% | + +Кроме того сразу видно что 59% времени уходит на создание и обновление записей +об автобусах. diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index b92f0516..6cbafce3 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -17,26 +17,53 @@ def call ActiveRecord::Base.connection.execute('delete from buses_services;') json.each do |trip| - from = City.find_or_create_by(name: trip['from']) - to = City.find_or_create_by(name: trip['to']) - services = [] - trip['bus']['services'].each do |service| - s = Service.find_or_create_by(name: service) - services << s - end - bus = Bus.find_or_create_by(number: trip['bus']['number']) - bus.update(model: trip['bus']['model'], services: services) - - Trip.create!( - from: from, - to: to, - bus: bus, - start_time: trip['start_time'], - duration_minutes: trip['duration_minutes'], - price_cents: trip['price_cents'], - ) + from = find_or_create_city(trip['from']) + to = find_or_create_city(trip['to']) + service_names = trip['bus']['services'] + services = find_or_create_services(service_names) + bus_number = trip['bus']['number'] + bus_model = trip['bus']['model'] + bus = find_or_create_bus(bus_number, bus_model, services) + + start_time = trip['start_time'] + duration = trip['duration_minutes'] + price = trip['price_cents'] + add_trip(bus, duration, from, price, start_time, to) end end end + + private + + def find_or_create_city(city_name) + City.find_or_create_by(name: city_name) + end + + def find_or_create_services(service_names) + service_names.map do |service| + find_or_create_service(service) + end + end + + def find_or_create_service(service) + Service.find_or_create_by(name: service) + end + + def find_or_create_bus(bus_number, model, services) + bus = Bus.find_or_create_by(number: bus_number) + bus.update(model: model, services: services) + bus + end + + def add_trip(bus, duration, from, price, start_time, to) + Trip.create!( + from: from, + to: to, + bus: bus, + start_time: start_time, + duration_minutes: duration, + price_cents: price, + ) + end end end From 4534a50603fa72acbb7bad39a8460fa9113ed31e Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 19:15:02 +0300 Subject: [PATCH 10/34] Remove unnecessary updates for bus creation --- case-study.md | 11 +++++++++++ lib/my_app/import.rb | 9 +++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/case-study.md b/case-study.md index c78e1ee4..b17067ba 100644 --- a/case-study.md +++ b/case-study.md @@ -84,3 +84,14 @@ Кроме того сразу видно что 59% времени уходит на создание и обновление записей об автобусах. +## Оптимизация создания автобусов + +При внимательном изучении механизма создания автобусов становится очевидным +что выполняются множество лишних действий: +* лишнее обновление записи, имя модели выставляется уже после создания; +* обновление модели и услуг происходит каждый раз, даже если автобус уже есть в БД; +* поиск автобусов в БД, хотя их создание происходило в этом же скрипте; + +| benchmark | Previous | Current | Diff % | +|--------------------------------|----------|---------|-----------------| +| total | 2976ms | 2628ms | -11% | diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index 6cbafce3..3446e9f8 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -4,6 +4,7 @@ class Import def initialize(file_name) @file_name = file_name + @imported_buses = {} end def call @@ -50,8 +51,12 @@ def find_or_create_service(service) end def find_or_create_bus(bus_number, model, services) - bus = Bus.find_or_create_by(number: bus_number) - bus.update(model: model, services: services) + bus = @imported_buses[bus_number] + unless bus + bus = Bus.create!(number: bus_number, model: model) + bus.update(services: services) + @imported_buses[bus_number] = bus + end bus end From d86dde783ca90199e18445eceadea5de2bb97c0a Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 19:37:15 +0300 Subject: [PATCH 11/34] Import bus services --- case-study.md | 13 +++++++++++++ lib/my_app/import.rb | 7 ++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/case-study.md b/case-study.md index b17067ba..091d5a5d 100644 --- a/case-study.md +++ b/case-study.md @@ -95,3 +95,16 @@ | benchmark | Previous | Current | Diff % | |--------------------------------|----------|---------|-----------------| | total | 2976ms | 2628ms | -11% | + +## Оптимизация выставления связей между автобусами и услугами + +Результат улучшения оказался не столь существенным для файла small.json, +на обновление данных об услугах в автобусах всё ещё уходит около 45% времени, +это говорит о том что нужно обтимизировать выставление связей между автобусами +и услугами. +Оптимизировать это можно несколькими способами, но самым эффективным будет +массовое выставление связей уже после создания записей автобусов. + +| benchmark | Previous | Current | Diff % | +|--------------------------------|----------|----------|-----------------| +| total | 2628ms | 1450ms | -44% | diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index 3446e9f8..ec926578 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -1,10 +1,12 @@ module MyApp class Import attr_reader :file_name + BusesService = Class.new(ActiveRecord::Base) def initialize(file_name) @file_name = file_name @imported_buses = {} + @buses_services = [] end def call @@ -31,6 +33,7 @@ def call price = trip['price_cents'] add_trip(bus, duration, from, price, start_time, to) end + BusesService.import(@buses_services) end end @@ -54,7 +57,9 @@ def find_or_create_bus(bus_number, model, services) bus = @imported_buses[bus_number] unless bus bus = Bus.create!(number: bus_number, model: model) - bus.update(services: services) + services.each do |service| + @buses_services << {bus_id: bus.id, service_id: service.id} + end @imported_buses[bus_number] = bus end bus From 53660539b06a4b564d153b572459975981618c9a Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 19:53:04 +0300 Subject: [PATCH 12/34] Remove unnecessary service find in db --- case-study.md | 14 ++++++++++++++ lib/my_app/import.rb | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/case-study.md b/case-study.md index 091d5a5d..b1b2b979 100644 --- a/case-study.md +++ b/case-study.md @@ -108,3 +108,17 @@ | benchmark | Previous | Current | Diff % | |--------------------------------|----------|----------|-----------------| | total | 2628ms | 1450ms | -44% | + +## Создание услуг + +На поиск и создание услуг уходит 34% времени. При этом заранее известно что +услуг ровно 10 (валидация на имя в модели и подсказка в Readme.md) +Вместо того чтобы осуществлять поиск и создание можно было бы заранее создать +все 10 записей игнорируя данные из json. Но так как у нас есть требование на +полное отсутствие функциональных изменений, то отказываемся от этой идеи, так +как для файла example.json должно быть создано только 3 услуги. +Так или иначе можно избавиться от необходимости поиска записей в БД. + +| benchmark | Previous | Current | Diff % | +|--------------------------------|----------|----------|-----------------| +| total | 1450ms | 885ms | -38% | \ No newline at end of file diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index ec926578..b69cbd10 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -6,6 +6,7 @@ class Import def initialize(file_name) @file_name = file_name @imported_buses = {} + @imported_services = {} @buses_services = [] end @@ -49,8 +50,8 @@ def find_or_create_services(service_names) end end - def find_or_create_service(service) - Service.find_or_create_by(name: service) + def find_or_create_service(service_name) + @imported_services[service_name] ||= Service.create(name: service_name) end def find_or_create_bus(bus_number, model, services) From 21547a5b66c982b4b255e82028f3bf091c8e5d3f Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 20:11:54 +0300 Subject: [PATCH 13/34] Import trips --- case-study.md | 13 ++++++++++++- lib/my_app/import.rb | 19 +++++++++++-------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/case-study.md b/case-study.md index b1b2b979..ad988197 100644 --- a/case-study.md +++ b/case-study.md @@ -121,4 +121,15 @@ | benchmark | Previous | Current | Diff % | |--------------------------------|----------|----------|-----------------| -| total | 1450ms | 885ms | -38% | \ No newline at end of file +| total | 1450ms | 885ms | -38% | + +## Импорт расписаний + +Наконец дошли до основной модели. Замеры показывают что 39% времени уходит на +создание записей модели Trip. +Так же как и с привязкой услуг к автобусам используем массовый импорт заранее +подготовленных данных. + +| benchmark | Previous | Current | Diff % | +|--------------------------------|----------|----------|-----------------| +| total | 885ms | 626ms | -29% | \ No newline at end of file diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index b69cbd10..5e92cb2c 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -7,6 +7,7 @@ def initialize(file_name) @file_name = file_name @imported_buses = {} @imported_services = {} + @imported_trips = [] @buses_services = [] end @@ -35,6 +36,7 @@ def call add_trip(bus, duration, from, price, start_time, to) end BusesService.import(@buses_services) + Trip.import(@imported_trips) end end @@ -67,14 +69,15 @@ def find_or_create_bus(bus_number, model, services) end def add_trip(bus, duration, from, price, start_time, to) - Trip.create!( - from: from, - to: to, - bus: bus, - start_time: start_time, - duration_minutes: duration, - price_cents: price, - ) + @imported_trips << + Trip.new( + from: from, + to: to, + bus: bus, + start_time: start_time, + duration_minutes: duration, + price_cents: price, + ) end end end From a05f4835e1a58fe42553c3efe26440d8917deba8 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sat, 1 Apr 2023 20:38:52 +0300 Subject: [PATCH 14/34] Remove unnecessary city find in db --- case-study.md | 32 +++++++++++++++++++++++++++++++- lib/my_app/import.rb | 3 ++- perfolab/my_app/import_stand.rb | 8 ++------ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/case-study.md b/case-study.md index ad988197..3d88bc56 100644 --- a/case-study.md +++ b/case-study.md @@ -132,4 +132,34 @@ | benchmark | Previous | Current | Diff % | |--------------------------------|----------|----------|-----------------| -| total | 885ms | 626ms | -29% | \ No newline at end of file +| total | 885ms | 626ms | -29% | + +## Оптимизация создания городов + +Теперь 38% времени уходит на создание записей модели City. Так же как и ранее +убираем поиск сохраненных записей в БД. + +| benchmark | Previous | Current | Diff % | +|--------------------------------|----------|----------|-----------------| +| total | 626ms | 357ms | -43% | + +## Предварительные итоги + +Проверка иморта файла large.json показала что время импорта составляет около +12 секунд, что уже существенно ниже исходных требований, но скрипт всё ещё +можно оптимизировать без особых сложностей. +На файле small.json stackprof показывает что около 50% времени уходит на +создание записей модели автобуса, однако на файлах medium.json и large.json +на данное действие уходит намного меньше времени в процентном отношении (24% +и 3,5% соответственно). + +На больших файла на первый план выходит уже работа массового импорта записей. +Причём много времени уходит на валидации записей. +Можно было бы отключить их, но строго говоря это действие является +функциональным изменением, так как оригинальный скрипт просто бы пропустил +навалидные записи. + +Дальнейший анализ будет производиться на файле medium.json, но уже после +выполнения второй части задания. Вероятно решение второй части зададания +несколько замедлит работу импорта, так как наверняка в рамках изменений +потребуется добавить индексы, что должно несколько замедлить добавление записей. diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index 5e92cb2c..5b99288f 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -7,6 +7,7 @@ def initialize(file_name) @file_name = file_name @imported_buses = {} @imported_services = {} + @imported_cities = {} @imported_trips = [] @buses_services = [] end @@ -43,7 +44,7 @@ def call private def find_or_create_city(city_name) - City.find_or_create_by(name: city_name) + @imported_cities[city_name] ||= City.create(name: city_name) end def find_or_create_services(service_names) diff --git a/perfolab/my_app/import_stand.rb b/perfolab/my_app/import_stand.rb index 68646aba..65002a9e 100644 --- a/perfolab/my_app/import_stand.rb +++ b/perfolab/my_app/import_stand.rb @@ -12,7 +12,7 @@ raw: true }, runner_options: { - arguments: ['small'] + arguments: ['medium'] } ) toolbox.add_tool( @@ -20,11 +20,7 @@ type: :benchmark, runner_options: { warmup: 1, - arguments: [ - 'small', - # 'medium', - # 'large' - ] + arguments: ['medium'] } ) end From add9e9b3d850a4ebf7e153911a7696c485d75eea Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sun, 2 Apr 2023 01:59:36 +0300 Subject: [PATCH 15/34] Add first analyze of TripsController work --- case-study.md | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/case-study.md b/case-study.md index 3d88bc56..e4b431f6 100644 --- a/case-study.md +++ b/case-study.md @@ -19,8 +19,8 @@ Система веб-аналитики показывает, что при просмотре расписаний на популярных направлениях пользователи не дожидаются загрузки страницы и уходят с сайта. -Британские учёные доказали, что финансово успешные web-проекты должны отображать -информацию пользователю не позже чем через 3 секунды после запроса. +Британские учёные доказали, что финансово успешные web-проекты должны +формировать основной документ страницы не больше чем за 300 милисекунд. ## Что сделать @@ -163,3 +163,22 @@ выполнения второй части задания. Вероятно решение второй части зададания несколько замедлит работу импорта, так как наверняка в рамках изменений потребуется добавить индексы, что должно несколько замедлить добавление записей. + +# Ускорение отображения расписаний + +## Основные инструменты для исследования + +* rack-mini-profiler + stackprof +* bullet + +## Первичный анализ + +Страница расписания рейсов из Самары в Москву генерируется около 2 секунд +в dev-окружении и около 1,2 секунды в production окружении. +При этом время на работу с БД относительно небольшое: +Production: Views: 934.6ms | ActiveRecord: 356.4ms +Development: Views: 1714.1ms | ActiveRecord: 242.4ms. +Вызывает вопросы почему время на ActiveRecord в Development-окружении +отображается как меньшее, пока решил отложить этот вопрос. +Очевидно тут стоило бы заняться в первую очередь вопросом генерации вьюх, +но так как тема задания касается оптимизации БД, то пока проигнорируем это. \ No newline at end of file From 1de56a0f7a9bf03e07ce8d13c14e8483a2233c4a Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sun, 2 Apr 2023 02:02:35 +0300 Subject: [PATCH 16/34] Add rack-mini-profiler --- Gemfile | 2 ++ Gemfile.lock | 3 +++ 2 files changed, 5 insertions(+) diff --git a/Gemfile b/Gemfile index e90a9ca9..9f143ee5 100644 --- a/Gemfile +++ b/Gemfile @@ -28,3 +28,5 @@ gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] gem 'perfolab', github: 'Tab10id/perfolab', branch: :temp_26 gem "activerecord-import", "~> 1.4" + +gem "rack-mini-profiler" \ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock index 2fe4162b..502db72e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -96,6 +96,8 @@ GEM pg (1.1.4) puma (3.12.1) rack (2.0.6) + rack-mini-profiler (3.0.0) + rack (>= 1.2.0) rack-test (1.1.0) rack (>= 1.0, < 3) rails (5.2.3) @@ -166,6 +168,7 @@ DEPENDENCIES perfolab! pg (>= 0.18, < 2.0) puma (~> 3.11) + rack-mini-profiler rails (~> 5.2.3) tzinfo-data web-console (>= 3.3.0) From 492c5b66ac31e4af95c15ace7f191c9d3077b8b2 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sun, 2 Apr 2023 02:07:21 +0300 Subject: [PATCH 17/34] Add rack-mini-profiler study info --- case-study.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/case-study.md b/case-study.md index e4b431f6..b8670009 100644 --- a/case-study.md +++ b/case-study.md @@ -181,4 +181,11 @@ Development: Views: 1714.1ms | ActiveRecord: 242.4ms. Вызывает вопросы почему время на ActiveRecord в Development-окружении отображается как меньшее, пока решил отложить этот вопрос. Очевидно тут стоило бы заняться в первую очередь вопросом генерации вьюх, -но так как тема задания касается оптимизации БД, то пока проигнорируем это. \ No newline at end of file +но так как тема задания касается оптимизации БД, то пока проигнорируем это. + +## Анализ с использованием rack-mini-profiler + +Для удобства решил выполнять основную работу по оптимизации в development. +Подключение rack-mini-profiler сразу же ухудшило показатели +Views: 5273.9ms | ActiveRecord: 737.0ms, тем не менее общая картина должна +быть той же. \ No newline at end of file From f80e385c65f83736f41b6e4cf0a6fdceff4d40ef Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sun, 2 Apr 2023 02:47:35 +0300 Subject: [PATCH 18/34] Fix N+1 --- app/controllers/trips_controller.rb | 2 +- case-study.md | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be2..eb2dee12 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -2,6 +2,6 @@ class TripsController < ApplicationController def index @from = City.find_by_name!(params[:from]) @to = City.find_by_name!(params[:to]) - @trips = Trip.where(from: @from, to: @to).order(:start_time) + @trips = Trip.where(from: @from, to: @to).preload(bus: :services).order(:start_time) end end diff --git a/case-study.md b/case-study.md index b8670009..de137c08 100644 --- a/case-study.md +++ b/case-study.md @@ -188,4 +188,17 @@ Development: Views: 1714.1ms | ActiveRecord: 242.4ms. Для удобства решил выполнять основную работу по оптимизации в development. Подключение rack-mini-profiler сразу же ухудшило показатели Views: 5273.9ms | ActiveRecord: 737.0ms, тем не менее общая картина должна -быть той же. \ No newline at end of file +быть той же. + +## Двойная проблема N+1 (автобусы и сервисы) + +Даже без установки bullet в логе видно огромное количество однотипных запросов. +Добавил preload на обе модели. +Время генерации страницы сократилось вдвое, при этом на работу ActiveRecord +теперь уходит всего 22ms (если верить логу rails) +Views: 2928.7ms | ActiveRecord: 22.0ms. +Предполагаю что уменьшение времени на вьюхи обусловлено меньшим временем для +формирования отчёта rack-mini-profiler. +С отключенным rack-mini-profiler: +Production: Views: 345.8ms | ActiveRecord: 22.2ms +Development: Views: 984.0ms | ActiveRecord: 17.3ms From b21f53453e6e55443e84579e14814365524eb459 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sun, 2 Apr 2023 03:16:35 +0300 Subject: [PATCH 19/34] Improve collection render --- app/views/trips/_services.html.erb | 6 ------ app/views/trips/_trip.html.erb | 18 +++++++++++++----- app/views/trips/index.html.erb | 10 +--------- case-study.md | 13 +++++++++++++ 4 files changed, 27 insertions(+), 20 deletions(-) delete mode 100644 app/views/trips/_services.html.erb diff --git a/app/views/trips/_services.html.erb b/app/views/trips/_services.html.erb deleted file mode 100644 index 2de639fc..00000000 --- a/app/views/trips/_services.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -
  • Сервисы в автобусе:
    • -
      - <% services.each do |service| %> - <%= render "service", service: service %> - <% end %> -
    diff --git a/app/views/trips/_trip.html.erb b/app/views/trips/_trip.html.erb index fa1de9aa..08a8eb09 100644 --- a/app/views/trips/_trip.html.erb +++ b/app/views/trips/_trip.html.erb @@ -1,5 +1,13 @@ -
  • <%= "Отправление: #{trip.start_time}" %>
    • -
  • <%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %>
    • -
  • <%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %>
    • -
  • <%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %>
    • -
  • <%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %>
    • +
      +
    • <%= "Отправление: #{trip.start_time}" %>
      • +
    • <%= "Прибытие: #{(Time.parse(trip.start_time) + trip.duration_minutes.minutes).strftime('%H:%M')}" %>
      • +
    • <%= "В пути: #{trip.duration_minutes / 60}ч. #{trip.duration_minutes % 60}мин." %>
      • +
    • <%= "Цена: #{trip.price_cents / 100}р. #{trip.price_cents % 100}коп." %>
      • +
    • <%= "Автобус: #{trip.bus.model} №#{trip.bus.number}" %>
      • + <% if trip.bus.services.present? %> +
    • Сервисы в автобусе:
      • +
        + <%= render partial: 'service', collection: trip.bus.services %> +
      + <% end %> +
    \ No newline at end of file diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce41..d123e89e 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -5,12 +5,4 @@ <%= "В расписании #{@trips.count} рейсов" %> -<% @trips.each do |trip| %> -
      - <%= render "trip", trip: trip %> - <% if trip.bus.services.present? %> - <%= render "services", services: trip.bus.services %> - <% end %> -
    - <%= render "delimiter" %> -<% end %> +<%= render partial: 'trip', collection: @trips, spacer_template: 'delimiter' %> diff --git a/case-study.md b/case-study.md index de137c08..744687c8 100644 --- a/case-study.md +++ b/case-study.md @@ -202,3 +202,16 @@ Views: 2928.7ms | ActiveRecord: 22.0ms. С отключенным rack-mini-profiler: Production: Views: 345.8ms | ActiveRecord: 22.2ms Development: Views: 984.0ms | ActiveRecord: 17.3ms + +## Оптимизация рендеринга + +Как бы ни хотелось продолжить, но в дальнейшей оптимизации запросов не вижу +смысла, можно было бы еще добавить добавить индекс на start_time таблицы trips, +но это ускорило бы и без того быстрый запрос (на моём ноутбуке этот запрос +выполняется за 3мс) и в любом случае это не позволит добиться придуманного мной +целевого показателя. + +Поэтому единственным способом достижения цели является оптимизация реднеринга. +Используя механизм render collection удалось добиться формирования страницы +в production окружении в пределах 150-200 мс без использования кэширования, +что соответствует целевым показателям. \ No newline at end of file From aa452cb808eb43431ebe4d39b3e6383defaf3697 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Sun, 2 Apr 2023 03:18:02 +0300 Subject: [PATCH 20/34] Fix list of used instruments --- case-study.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/case-study.md b/case-study.md index 744687c8..8f784698 100644 --- a/case-study.md +++ b/case-study.md @@ -168,8 +168,8 @@ ## Основные инструменты для исследования -* rack-mini-profiler + stackprof -* bullet +* rails log +* rack-mini-profiler ## Первичный анализ From b98ea4f76758bfc1c298297106d59a4f085d9a94 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Mon, 3 Apr 2023 00:38:00 +0300 Subject: [PATCH 21/34] Update task requirements --- case-study.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/case-study.md b/case-study.md index 8f784698..5521e3eb 100644 --- a/case-study.md +++ b/case-study.md @@ -214,4 +214,18 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms Поэтому единственным способом достижения цели является оптимизация реднеринга. Используя механизм render collection удалось добиться формирования страницы в production окружении в пределах 150-200 мс без использования кэширования, -что соответствует целевым показателям. \ No newline at end of file +что соответствует целевым показателям. + +# Оптимизация импорта: империя наносит ответный удар + +Снова придумываю пользовательскую историю. +Отдел продаж начал продавать функционалость позволяющую провести миграцию +с системы конкурентов. + +Объем данных в исходной системе может быть произвольным. +Формат данных для импорта тот же и не может быть изменён. + +## Что сделать + +Нужно оптимизировать механизм загрузки данных из файла таким образом чтобы +потребление памяти не зависила от объёма данных. \ No newline at end of file From aa1f6d4e123a29b5a9d8fedafcb3feac40d3d01d Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Mon, 3 Apr 2023 01:04:12 +0300 Subject: [PATCH 22/34] Add stream import idea --- case-study.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/case-study.md b/case-study.md index 5521e3eb..50c1b2e7 100644 --- a/case-study.md +++ b/case-study.md @@ -228,4 +228,23 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms ## Что сделать Нужно оптимизировать механизм загрузки данных из файла таким образом чтобы -потребление памяти не зависила от объёма данных. \ No newline at end of file +потребление памяти не зависила от объёма данных. + +## Общий план действий + +Необходимо реализизовать потоковое чтение и импорт данных. +Основными проблемами является то что данные нужно импортировать в несколько +таблиц, нет возможности продолжительное время "накапливать" данные для +последующего импорта. +До начала работ по оптимизации желательно уточнить фунциональные требования: +* необходимось сохранения вызова валидаций при создании объектов; +* необходимость выполнения всех действий в рамках единой транзакции. + +Для интереса я буду исходить из необходимости сохранения старого поведения, +поэтому вариант с импортом данных напрямую в PG из Readme.md будет +нецелесообразен, так как всё равно портребует создания объектов для запуска +валидаций. + +Текущая идея состоит в том, чтобы при потоковом чтении из исходного файла +формировать буфер записей определённого размера с последующим импортом +с помощью activerecord-import. From 2ce38132e6ff260d73efaaf1201b99d9def4dff0 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 03:07:02 +0300 Subject: [PATCH 23/34] Enable gc for benchmark measure (was disabled by mistake) --- perfolab/my_app/import_stand.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/perfolab/my_app/import_stand.rb b/perfolab/my_app/import_stand.rb index 65002a9e..91ae06f6 100644 --- a/perfolab/my_app/import_stand.rb +++ b/perfolab/my_app/import_stand.rb @@ -19,6 +19,7 @@ :benchmark, type: :benchmark, runner_options: { + gc_disable: false, warmup: 1, arguments: ['medium'] } From 9952b56a57cffd065408fa727094423c7c9ce97f Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Mon, 3 Apr 2023 17:13:40 +0300 Subject: [PATCH 24/34] Add oj parse --- Gemfile | 3 ++- Gemfile.lock | 2 ++ lib/my_app/import.rb | 27 ++++++++++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 9f143ee5..721e3e40 100644 --- a/Gemfile +++ b/Gemfile @@ -29,4 +29,5 @@ gem 'perfolab', github: 'Tab10id/perfolab', branch: :temp_26 gem "activerecord-import", "~> 1.4" -gem "rack-mini-profiler" \ No newline at end of file +gem "rack-mini-profiler" +gem "oj", "~> 3.14" diff --git a/Gemfile.lock b/Gemfile.lock index 502db72e..75c13d47 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -93,6 +93,7 @@ GEM nio4r (2.3.1) nokogiri (1.10.2) mini_portile2 (~> 2.4.0) + oj (3.14.2) pg (1.1.4) puma (3.12.1) rack (2.0.6) @@ -165,6 +166,7 @@ DEPENDENCIES bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) + oj (~> 3.14) perfolab! pg (>= 0.18, < 2.0) puma (~> 3.11) diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index 5b99288f..87959d09 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -3,6 +3,24 @@ class Import attr_reader :file_name BusesService = Class.new(ActiveRecord::Base) + class TripsJsonStreamHandler < ::Oj::ScHandler + def hash_start + {} + end + + def hash_set(h,k,v) + h[k] = v + end + + def array_start + [] + end + + def array_append(a,v) + a << v + end + end + def initialize(file_name) @file_name = file_name @imported_buses = {} @@ -13,7 +31,7 @@ def initialize(file_name) end def call - json = JSON.parse(File.read(file_name)) + json = parse ActiveRecord::Base.transaction do City.delete_all @@ -43,6 +61,13 @@ def call private + def parse + io = File.open(file_name, 'r') + Oj.sc_parse(TripsJsonStreamHandler.new, io) + ensure + io.close + end + def find_or_create_city(city_name) @imported_cities[city_name] ||= City.create(name: city_name) end From 2d271fd9ab1e0a20caca2db8c1738443894266e1 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Mon, 3 Apr 2023 17:14:14 +0300 Subject: [PATCH 25/34] Allow to parse gzipped files --- lib/my_app/import.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index 87959d09..718fa7e1 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -62,7 +62,12 @@ def call private def parse - io = File.open(file_name, 'r') + io = + if File.extname(file_name).downcase == '.gz' + Zlib::GzipReader.open(file_name) + else + File.open(file_name, 'r') + end Oj.sc_parse(TripsJsonStreamHandler.new, io) ensure io.close From 7ab2db6fb30dd1556945d25bf82d04a080d3904d Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Mon, 3 Apr 2023 17:19:15 +0300 Subject: [PATCH 26/34] Cleanup code --- lib/my_app/import.rb | 54 ++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index 718fa7e1..05e109be 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -31,36 +31,42 @@ def initialize(file_name) end def call - json = parse - ActiveRecord::Base.transaction do - City.delete_all - Bus.delete_all - Service.delete_all - Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') - - json.each do |trip| - from = find_or_create_city(trip['from']) - to = find_or_create_city(trip['to']) - service_names = trip['bus']['services'] - services = find_or_create_services(service_names) - bus_number = trip['bus']['number'] - bus_model = trip['bus']['model'] - bus = find_or_create_bus(bus_number, bus_model, services) - - start_time = trip['start_time'] - duration = trip['duration_minutes'] - price = trip['price_cents'] - add_trip(bus, duration, from, price, start_time, to) - end - BusesService.import(@buses_services) - Trip.import(@imported_trips) + db_clear + import end end private + def import + json = parse + json.each do |trip| + from = find_or_create_city(trip['from']) + to = find_or_create_city(trip['to']) + service_names = trip['bus']['services'] + services = find_or_create_services(service_names) + bus_number = trip['bus']['number'] + bus_model = trip['bus']['model'] + bus = find_or_create_bus(bus_number, bus_model, services) + + start_time = trip['start_time'] + duration = trip['duration_minutes'] + price = trip['price_cents'] + add_trip(bus, duration, from, price, start_time, to) + end + BusesService.import(@buses_services) + Trip.import(@imported_trips) + end + + def db_clear + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + ActiveRecord::Base.connection.execute('delete from buses_services;') + end + def parse io = if File.extname(file_name).downcase == '.gz' From e8a725c15ce14301834962f0b69d47387f4ed38a Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 01:37:53 +0300 Subject: [PATCH 27/34] Implement batch import from parse stream --- lib/my_app/import.rb | 49 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/lib/my_app/import.rb b/lib/my_app/import.rb index 05e109be..7f423ef5 100644 --- a/lib/my_app/import.rb +++ b/lib/my_app/import.rb @@ -1,28 +1,43 @@ module MyApp class Import - attr_reader :file_name + attr_reader :file_name, :batch_size BusesService = Class.new(ActiveRecord::Base) class TripsJsonStreamHandler < ::Oj::ScHandler + def initialize(&block) + @root = true + @block = block + end + def hash_start {} end - def hash_set(h,k,v) + def hash_set(h, k, v) h[k] = v end def array_start - [] + if @root + @root = false + return + else + [] + end end - def array_append(a,v) - a << v + def array_append(a, v) + if a + a << v + else + @block.call(v) + end end end - def initialize(file_name) + def initialize(file_name, batch_size: 1000) @file_name = file_name + @batch_size = batch_size @imported_buses = {} @imported_services = {} @imported_cities = {} @@ -40,8 +55,7 @@ def call private def import - json = parse - json.each do |trip| + parse do |trip| from = find_or_create_city(trip['from']) to = find_or_create_city(trip['to']) service_names = trip['bus']['services'] @@ -67,14 +81,14 @@ def db_clear ActiveRecord::Base.connection.execute('delete from buses_services;') end - def parse + def parse(&block) io = if File.extname(file_name).downcase == '.gz' Zlib::GzipReader.open(file_name) else File.open(file_name, 'r') end - Oj.sc_parse(TripsJsonStreamHandler.new, io) + Oj.sc_parse(TripsJsonStreamHandler.new(&block), io) ensure io.close end @@ -98,13 +112,21 @@ def find_or_create_bus(bus_number, model, services) unless bus bus = Bus.create!(number: bus_number, model: model) services.each do |service| - @buses_services << {bus_id: bus.id, service_id: service.id} + add_bus_service(bus, service) end @imported_buses[bus_number] = bus end bus end + def add_bus_service(bus, service) + @buses_services << { bus_id: bus.id, service_id: service.id } + if @buses_services.size == batch_size + BusesService.import(@buses_services) + @buses_services = [] + end + end + def add_trip(bus, duration, from, price, start_time, to) @imported_trips << Trip.new( @@ -115,6 +137,11 @@ def add_trip(bus, duration, from, price, start_time, to) duration_minutes: duration, price_cents: price, ) + + if @imported_trips.size == batch_size + Trip.import(@imported_trips) + @imported_trips = [] + end end end end From 68694a2eb0935097a548d4409572bce593122c98 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 01:51:58 +0300 Subject: [PATCH 28/34] Allow to set batch size in rake task --- lib/tasks/utils.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 1608ba4c..2b63a9bc 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -3,5 +3,5 @@ task :reload_json, [:file_name] => :environment do |_task, args| require Rails.root.join('lib/my_app/import') - MyApp::Import.new(args.file_name).call + MyApp::Import.new(args.file_name, batch_size: ENV.fetch('BATCH_SIZE', 1000).to_i).call end From ad0d363c7642fc2bb4af8cf230ccc565f3684438 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 01:52:44 +0300 Subject: [PATCH 29/34] Add stream parse and batch import study report --- case-study.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/case-study.md b/case-study.md index 50c1b2e7..c3bf91f7 100644 --- a/case-study.md +++ b/case-study.md @@ -248,3 +248,19 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms Текущая идея состоит в том, чтобы при потоковом чтении из исходного файла формировать буфер записей определённого размера с последующим импортом с помощью activerecord-import. + +## Потоковое чтение и импорт блоками + +Для реализации потокового чтения была использована библиотека oj. +Благодаря использованию метода `Oj.sc_parse` и написанию простого +вспомогательного класса удалось сохранить код практически в неизменном виде. +Кроме того была использована встроенная библиотека zlib для возможности +поточного чтения из gz-файла, тем самым избавляя от необходимости распаковки +файлов большого размера. + +Оверхед добавленный механизмом потокового чтения оказался незначительным. + +| benchmark | Previous | Current | Diff % | +|--------------------------------|-----------|---------|-----------------| +| total | 1498ms | 1531ms | 2% | + From c23164b251ac41bab7e0c5586ed675ec3931641d Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 02:36:39 +0300 Subject: [PATCH 30/34] Add study report about activerecord cost --- case-study.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/case-study.md b/case-study.md index c3bf91f7..a3523381 100644 --- a/case-study.md +++ b/case-study.md @@ -264,3 +264,24 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms |--------------------------------|-----------|---------|-----------------| | total | 1498ms | 1531ms | 2% | +## Оптимизация activerecord-import + +Инструменты показывают что большую часть времени скрипт производит импорт данных +с помощью библиотеки activerecord-import. +К сожалению возможности настройки параметров иморта доволько ограничены. +Большая часть времени уходит на валидации, но, как было сказано ранее, от них я +отказываться не плананирую. +Можно было бы на время импорта отключить часть валидаций в тех случаях когда мы +заранее уверены в корректности данных, но этим мне уже заниматься довольно +лениво, к тому же это действие вряд ли бы дало больше 5 процентов уменшения +времени работы скрипта (валидации занимаю 17% от полного времени работы). + +## Создание AR-объектов + +На создание объектов уходит 20% времени. Существенного улучшения результатов +можно добиться только если произвести полный отказ от ActiveRecord, +а следовательно и от встроенных валидаций. +По примерной оценке такая работа могда бы уменьшить время работы импорта +на 60-70%, но данная работа сделала бы скрипт менее поддерживаемым из-за +расхождения валидаций в скрипте импорта и реальных моделей. Вынесение же +валидаций в отдельный модуль не выглядит целесообразным. \ No newline at end of file From d96981ee86c423ad5a384ded5e5d3a81a3d6ac09 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 02:54:38 +0300 Subject: [PATCH 31/34] Add study report about bus import --- case-study.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/case-study.md b/case-study.md index a3523381..49eccef6 100644 --- a/case-study.md +++ b/case-study.md @@ -284,4 +284,23 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms По примерной оценке такая работа могда бы уменьшить время работы импорта на 60-70%, но данная работа сделала бы скрипт менее поддерживаемым из-за расхождения валидаций в скрипте импорта и реальных моделей. Вынесение же -валидаций в отдельный модуль не выглядит целесообразным. \ No newline at end of file +валидаций в отдельный модуль не выглядит целесообразным. + +## Возвращаемся к импорту автобусов + +На создание автобусов уходит 26% времени. На текущий момент автобусы создаются +сразу же по одной записи. Можно было бы произвести массовый импорт автобусов +по аналогии с другими моделями, но это может создать проблемы в будущем. +Причина в том что связь с автобусами явно указывается в модели Trip. +А следовательно записи модели Bus должны быть добавлены до записей модели Trip. +Как известно, на 1000 записей Trip приходится около 1 записи модели Bus, +следовательно для того чтобы массово сохранить хотя бы 10 записей модели Bus +на нужно было бы накопить 10000 записей модели Trip. + +От данного ограничения можно было бы избавиться если вручную выставить +идентификаторы модели Bus до фактического сохранения, но и тут есть проблема, +так как если в будущем кто-то настроит внешние ключи на колонки в БД, +то скрипт внезапно перестанет работать и придётся настраивать отключение +проверки внешних ключей. + +Таким образом, данную оптимизацию так же считаю нецелесообразной. \ No newline at end of file From 2e7adf3e8732a8188e4aa87e548c7cd40c2d1c80 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 03:10:50 +0300 Subject: [PATCH 32/34] Remove wrong assumption --- case-study.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/case-study.md b/case-study.md index 49eccef6..cb0de572 100644 --- a/case-study.md +++ b/case-study.md @@ -146,8 +146,7 @@ ## Предварительные итоги Проверка иморта файла large.json показала что время импорта составляет около -12 секунд, что уже существенно ниже исходных требований, но скрипт всё ещё -можно оптимизировать без особых сложностей. +12 секунд, что уже существенно ниже исходных требований. На файле small.json stackprof показывает что около 50% времени уходит на создание записей модели автобуса, однако на файлах medium.json и large.json на данное действие уходит намного меньше времени в процентном отношении (24% From 340123d37519278fdf378c2f04e7a38965500a49 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 03:15:52 +0300 Subject: [PATCH 33/34] Add time results to study file --- case-study.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/case-study.md b/case-study.md index cb0de572..b0c5bf2f 100644 --- a/case-study.md +++ b/case-study.md @@ -302,4 +302,10 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms то скрипт внезапно перестанет работать и придётся настраивать отключение проверки внешних ключей. -Таким образом, данную оптимизацию так же считаю нецелесообразной. \ No newline at end of file +Таким образом, данную оптимизацию так же считаю нецелесообразной. + +## Результаты + +large.json: 12 секунд +1M.json.gz: 2,5 минуты +10M.json.gz: предположительно 25-30 минут. From da6397f3ecb5985470d207c2c01ca0249f010b19 Mon Sep 17 00:00:00 2001 From: Tab Loid Date: Tue, 4 Apr 2023 23:31:57 +0300 Subject: [PATCH 34/34] fix typo and style --- case-study.md | 57 ++++++++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/case-study.md b/case-study.md index b0c5bf2f..093b25cc 100644 --- a/case-study.md +++ b/case-study.md @@ -2,7 +2,7 @@ ## User story -Тут должны быть описаны пльзовательские истории, которые определят целевые +Тут должны быть описаны пользовательские истории которые определят целевые значения метрик, но так как их нет, то я их придумаю. ### Импорт данных @@ -20,14 +20,14 @@ Система веб-аналитики показывает, что при просмотре расписаний на популярных направлениях пользователи не дожидаются загрузки страницы и уходят с сайта. Британские учёные доказали, что финансово успешные web-проекты должны -формировать основной документ страницы не больше чем за 300 милисекунд. +формировать основной документ страницы не больше чем за 300 миллисекунд. ## Что сделать -* Нужно оптимизировать механизм перезагрузки расписания из файла так, - * файл large.json в пределах минуты. +* Нужно оптимизировать механизм перезагрузки расписания из файла + * файл large.json должен обрабатывался не более минуты. * Необходимо ускорить отображение расписаний - * страница автобусы/Самара/Москва должна открываться за адекватное время + * страница автобусы/Самара/Москва должна открываться менее чем за 300мс # Оптимизация импорта @@ -46,16 +46,16 @@ ## Первичный анализ -Замеры времени импорта показывют что оно равстёт приблизительно линейно в +Замеры времени импорта показывют что оно растёт приблизительно линейно в зависимости от размера файла, что несложно увидеть и при обычном чтении оригинального скрипта. Инструменты показывают что большую часть времени выполняются запросы на поиск и создание объектов. Уже на данном этапе очевидно что необходимо кардинально уменьшать количество -отправляемых в БД запросов. Внимательное изучение скрипт показывает что +отправляемых в БД запросов. Внимательное изучение скрипта показывает что от запросов на поиск данных в бд можно полностью избавиться, так как изначально в БД нет никаких данных. Уменьшения же количества запросов на вставку данных -можно с помощью массового создания объектов. +можно добиться с помощью массового создания объектов. Для удобства можно воспользоваться библиотекой activerecord-import. @@ -81,7 +81,7 @@ |--------------------------------|-----------|---------|----------------| | total | 3014ms | 2976ms | -1% | -Кроме того сразу видно что 59% времени уходит на создание и обновление записей +Кроме того, сразу видно что 59% времени уходит на создание и обновление записей об автобусах. ## Оптимизация создания автобусов @@ -90,7 +90,9 @@ что выполняются множество лишних действий: * лишнее обновление записи, имя модели выставляется уже после создания; * обновление модели и услуг происходит каждый раз, даже если автобус уже есть в БД; -* поиск автобусов в БД, хотя их создание происходило в этом же скрипте; +* поиск автобусов в БД, хотя их создание происходило в этом же скрипте. + +Результат исправления: | benchmark | Previous | Current | Diff % | |--------------------------------|----------|---------|-----------------| @@ -100,23 +102,25 @@ Результат улучшения оказался не столь существенным для файла small.json, на обновление данных об услугах в автобусах всё ещё уходит около 45% времени, -это говорит о том что нужно обтимизировать выставление связей между автобусами -и услугами. +это говорит о том, что нужно оптимизировать выставление связей между автобусами +и сервисами. Оптимизировать это можно несколькими способами, но самым эффективным будет массовое выставление связей уже после создания записей автобусов. +Результат исправления: + | benchmark | Previous | Current | Diff % | |--------------------------------|----------|----------|-----------------| | total | 2628ms | 1450ms | -44% | -## Создание услуг +## Создание сервисов -На поиск и создание услуг уходит 34% времени. При этом заранее известно что -услуг ровно 10 (валидация на имя в модели и подсказка в Readme.md) -Вместо того чтобы осуществлять поиск и создание можно было бы заранее создать +На поиск и создание сервисов уходит 34% времени. При этом заранее известно что +их ровно 10 (валидация на имя в модели и подсказка в Readme.md). +Вместо того чтобы осуществлять поиск и создание, можно было бы заранее создать все 10 записей игнорируя данные из json. Но так как у нас есть требование на полное отсутствие функциональных изменений, то отказываемся от этой идеи, так -как для файла example.json должно быть создано только 3 услуги. +как для файла example.json должно быть создано только 3 сервиса. Так или иначе можно избавиться от необходимости поиска записей в БД. | benchmark | Previous | Current | Diff % | @@ -197,7 +201,8 @@ Views: 5273.9ms | ActiveRecord: 737.0ms, тем не менее общая ка теперь уходит всего 22ms (если верить логу rails) Views: 2928.7ms | ActiveRecord: 22.0ms. Предполагаю что уменьшение времени на вьюхи обусловлено меньшим временем для -формирования отчёта rack-mini-profiler. +формирования отчёта rack-mini-profiler, но это не точно, так как в production +время тоже значительно уменьшилось. С отключенным rack-mini-profiler: Production: Views: 345.8ms | ActiveRecord: 22.2ms Development: Views: 984.0ms | ActiveRecord: 17.3ms @@ -205,7 +210,7 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms ## Оптимизация рендеринга Как бы ни хотелось продолжить, но в дальнейшей оптимизации запросов не вижу -смысла, можно было бы еще добавить добавить индекс на start_time таблицы trips, +смысла, можно было бы еще добавить индекс на start_time таблицы trips, но это ускорило бы и без того быстрый запрос (на моём ноутбуке этот запрос выполняется за 3мс) и в любом случае это не позволит добиться придуманного мной целевого показателя. @@ -226,13 +231,13 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms ## Что сделать -Нужно оптимизировать механизм загрузки данных из файла таким образом чтобы -потребление памяти не зависила от объёма данных. +Нужно оптимизировать механизм загрузки данных из файла таким образом, чтобы +потребление памяти не зависило от объёма данных. ## Общий план действий Необходимо реализизовать потоковое чтение и импорт данных. -Основными проблемами является то что данные нужно импортировать в несколько +Основными проблемами является то, что данные нужно импортировать в несколько таблиц, нет возможности продолжительное время "накапливать" данные для последующего импорта. До начала работ по оптимизации желательно уточнить фунциональные требования: @@ -290,7 +295,7 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms На создание автобусов уходит 26% времени. На текущий момент автобусы создаются сразу же по одной записи. Можно было бы произвести массовый импорт автобусов по аналогии с другими моделями, но это может создать проблемы в будущем. -Причина в том что связь с автобусами явно указывается в модели Trip. +Причина в том, что связь с автобусами явно указывается в модели Trip. А следовательно записи модели Bus должны быть добавлены до записей модели Trip. Как известно, на 1000 записей Trip приходится около 1 записи модели Bus, следовательно для того чтобы массово сохранить хотя бы 10 записей модели Bus @@ -306,6 +311,6 @@ Development: Views: 984.0ms | ActiveRecord: 17.3ms ## Результаты -large.json: 12 секунд -1M.json.gz: 2,5 минуты -10M.json.gz: предположительно 25-30 минут. +* large.json: 12 секунд +* 1M.json.gz: 2,5 минуты +* 10M.json.gz: предположительно 25-30 минут.