From cf54b77fec2905926fb58b492300700603cfd06d Mon Sep 17 00:00:00 2001 From: Denis Nazmutdinov Date: Thu, 22 Aug 2019 00:15:12 +0300 Subject: [PATCH 1/3] [Task-3] Move rake logic to service, add tests; Add case-study file --- .gitignore | 1 + case-study.md | 21 +++++++++++++ lib/populate_database.rb | 43 ++++++++++++++++++++++++++ lib/tasks/utils.rake | 36 ++++------------------ test/lib/populate_database_test.rb | 48 ++++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 31 deletions(-) create mode 100644 case-study.md create mode 100644 lib/populate_database.rb create mode 100644 test/lib/populate_database_test.rb diff --git a/.gitignore b/.gitignore index 59c74047..7c414dbc 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /tmp /log /public +.idea diff --git a/case-study.md b/case-study.md new file mode 100644 index 00000000..9e147388 --- /dev/null +++ b/case-study.md @@ -0,0 +1,21 @@ +## 1. Оптимизация загрузки данных в базу приложения + +#### Проблема +Рейк-таск для загрузки данных в базу приложения работает слишком долго: ~8 секунд для файла с 1_000 записей, ~67 секунд для файла с 10_000 записей. + +#### Метрика +Чтобы понять, несут ли изменения положительный эффект, в качестве метрики выбрано время загрузки файлов поставляемых с программой. Также в процессе оптимизации планирую следить за потреблением памяти. + +#### Гарантия корректности работы программы +Для гарантии корректной работы написан тест проверяющий что в базу корректно загружается файл `fixtures/example.json` + +#### Feedback-loop +Для получения быстрой обратной связи добавлены скрипты генерации отчетов профилировщиков. + +#### Процесс оптимизации (WIP) + +#### Результат (WIP) +В результате проведенной оптимизации удалось + +#### Защита от регрессии (WIP) +Для защиты от регрессии добавлен тест, который проверяет время загрузки файла с 10к записей. diff --git a/lib/populate_database.rb b/lib/populate_database.rb new file mode 100644 index 00000000..2ff0fd83 --- /dev/null +++ b/lib/populate_database.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +# Populates database with records from dump file +class PopulateDatabase + include Singleton + + class << self + delegate :call, to: :instance + end + + def call(file_path:) + json = JSON.parse(File.read(file_path)) + + 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 diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe871..36d9b300 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,34 +1,8 @@ -# Наивная загрузка данных из json-файла в БД -# rake reload_json[fixtures/small.json] -task :reload_json, [:file_name] => :environment do |_task, args| - json = JSON.parse(File.read(args.file_name)) +# frozen_string_literal: true - ActiveRecord::Base.transaction do - City.delete_all - Bus.delete_all - Service.delete_all - Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') +require 'populate_database' - 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 +desc 'Loads datum from *.json dump file to DB' +task :reload_json, [:file_path] => :environment do |_task, args| + PopulateDatabase.call(file_path: args.file_path) end diff --git a/test/lib/populate_database_test.rb b/test/lib/populate_database_test.rb new file mode 100644 index 00000000..b71ef58c --- /dev/null +++ b/test/lib/populate_database_test.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require_relative '../test_helper' +require 'minitest/autorun' +require 'populate_database' + +class PopulateDatabaseTest < Minitest::Test + + def teardown + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + end + + def test_call_populates_db_with_correct_data + PopulateDatabase.call(file_path: 'fixtures/example.json') + + assert_equal %w[Москва Самара], City.pluck(:name).sort + assert_equal [%w[123 Икарус]], Bus.pluck(:number, :model) + assert_equal %w[WiFi Туалет], Service.pluck(:name).sort + assert_equal %w[11:00 12:00 13:00 14:00 15:00 17:30 18:30 19:30 20:30 21:30], Trip.pluck(:start_time).sort + end + + def test_call_populates_db_with_correct_references + PopulateDatabase.call(file_path: 'fixtures/example.json') + + bus = Bus.take + trip = Trip.take + + assert_equal 2, bus.services.count + assert_equal 10, bus.trips.count + assert_equal 1, Service.take.buses.count + assert_instance_of City, trip.from + assert_instance_of City, trip.to + assert_instance_of Bus, trip.bus + end + + def test_call_is_idempotent + PopulateDatabase.call(file_path: 'fixtures/example.json') + PopulateDatabase.call(file_path: 'fixtures/example.json') + + assert_equal 2, City.count + assert_equal 1, Bus.count + assert_equal 2, Service.count + assert_equal 10, Trip.count + end +end From 170cb3522c5805725e6baebb112dcd9a6f972625 Mon Sep 17 00:00:00 2001 From: Denis Nazmutdinov Date: Fri, 30 Aug 2019 21:27:19 +0300 Subject: [PATCH 2/3] [Task-3] Optimize data uploading time; add specs --- Gemfile | 3 ++ Gemfile.lock | 5 ++ app/models/buses_service.rb | 5 ++ app/models/service.rb | 2 +- case-study.md | 35 ++++++++++++- lib/populate_database.rb | 83 +++++++++++++++++++++--------- profiling/Dockerfile | 5 ++ profiling/memory_prof.rb | 14 +++++ profiling/pgbadger.sh | 6 +++ profiling/ruby_prof.rb | 17 ++++++ test/lib/populate_database_test.rb | 8 +++ 11 files changed, 157 insertions(+), 26 deletions(-) create mode 100644 app/models/buses_service.rb create mode 100644 profiling/Dockerfile create mode 100644 profiling/memory_prof.rb create mode 100755 profiling/pgbadger.sh create mode 100644 profiling/ruby_prof.rb diff --git a/Gemfile b/Gemfile index e20b1260..20af42b1 100644 --- a/Gemfile +++ b/Gemfile @@ -17,6 +17,7 @@ group :development do # Access an interactive console on exception pages or by calling 'console' anywhere in the code. gem 'web-console', '>= 3.3.0' gem 'listen', '>= 3.0.5', '< 3.2' + gem 'ruby-prof' end group :test do @@ -24,3 +25,5 @@ end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] +# A library for bulk insertion of data into your database using ActiveRecord +gem 'activerecord-import' diff --git a/Gemfile.lock b/Gemfile.lock index fccf6f5f..acd54e67 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -33,6 +33,8 @@ GEM activemodel (= 5.2.3) activesupport (= 5.2.3) arel (>= 9.0) + activerecord-import (1.0.2) + activerecord (>= 3.2) activestorage (5.2.3) actionpack (= 5.2.3) activerecord (= 5.2.3) @@ -109,6 +111,7 @@ GEM rb-fsevent (0.10.3) rb-inotify (0.10.0) ffi (~> 1.0) + ruby-prof (1.0.0) ruby_dep (1.5.0) sprockets (3.7.2) concurrent-ruby (~> 1.0) @@ -134,12 +137,14 @@ PLATFORMS ruby DEPENDENCIES + activerecord-import bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) pg (>= 0.18, < 2.0) puma (~> 3.11) rails (~> 5.2.3) + ruby-prof tzinfo-data web-console (>= 3.3.0) diff --git a/app/models/buses_service.rb b/app/models/buses_service.rb new file mode 100644 index 00000000..a9b0c838 --- /dev/null +++ b/app/models/buses_service.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +# Model for convenient access to join table +class BusesService < ApplicationRecord +end diff --git a/app/models/service.rb b/app/models/service.rb index 9cbb2a32..e2eb93e7 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -9,7 +9,7 @@ class Service < ApplicationRecord 'Телевизор общий', 'Телевизор индивидуальный', 'Стюардесса', - 'Можно не печатать билет', + 'Можно не печатать билет' ].freeze has_and_belongs_to_many :buses, join_table: :buses_services diff --git a/case-study.md b/case-study.md index 9e147388..bd15a55b 100644 --- a/case-study.md +++ b/case-study.md @@ -13,9 +13,40 @@ Для получения быстрой обратной связи добавлены скрипты генерации отчетов профилировщиков. #### Процесс оптимизации (WIP) +В исходном состоянии скрипт загрузки имеет следующие показатели для файла small.json (1к записей, размер ~304K) +Потребление памяти (вместе со всем rails приложением) +``` +Total allocated: 467.01 MB (4716422 objects) +Total retained: 10.68 MB (9833 objects) +``` +Время работы - ~9 сек + +Ruby-prof по времени не показал точек роста - отчет в основном состоит из внутренностей Rails. + +Собрал лог из базы данных и скормил его pgbadger - оказалось что для вставки 1_000 записей выполняется 13_528 запросов к базе. +Из них 68% (9_253) это SELECT, т.е. даже не вставка данных. Несложно было догадаться что основная масса из них это проверка "справочников" и валидации AR выполняемые при сохранении данных. +Решил сделать как было предложено в подсказках к заданию - использовать гем `activerecord-import` а также собрать справочники и загрузить их одной пачкой. + +Благодаря этим действиям удалось значительно снизить время загрузки данных : +Без AR валидаций small.json - ~1.1 секунд, large.json - ~7.7 секунд +С AR валидациями small.json - ~1.3 секунд, large.json - ~18.4 секунд +(данные, включая время старта окружения) + +Потребление памяти также сократилось: +small.json: +``` +Total allocated: 24.79 MB (159304 objects) +Total retained: 10.61 MB (9125 objects) +``` +large.json +``` +Total allocated: 710.76 MB (5513967 objects) +Total retained: 10.61 MB (9120 objects) +``` +Естественно что без стриминга мы грузим в память файл который обрабатываем целиком, но bonus часть я не выполнял, так что оставляю как есть. #### Результат (WIP) -В результате проведенной оптимизации удалось +В результате проведенной оптимизации удалось уложиться в рамки задачи. #### Защита от регрессии (WIP) -Для защиты от регрессии добавлен тест, который проверяет время загрузки файла с 10к записей. +Для защиты от регрессии добавлен тест, который проверяет время загрузки файла с 1к записей. diff --git a/lib/populate_database.rb b/lib/populate_database.rb index 2ff0fd83..eea934b1 100644 --- a/lib/populate_database.rb +++ b/lib/populate_database.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Populates database with records from dump file +# Populates database with records from json file class PopulateDatabase include Singleton @@ -8,36 +8,73 @@ class << self delegate :call, to: :instance end - def call(file_path:) + def call(file_path:, validate: false) json = JSON.parse(File.read(file_path)) + clear_database + process_common_entities(json, validate) + process_trips(json, validate) + end + + private + + def clear_database 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'], - ) + BusesService.delete_all + end + end + + def process_common_entities(json, validate) + cities = Set.new + buses = Set.new + services = Set.new + + json.each do |trip| + cities << { name: trip['from'] } + cities << { name: trip['to'] } + buses << { number: trip['bus']['number'], model: trip['bus']['model'] } + + trip['bus']['services'].each { |service_name| services << { name: service_name } } + end + + City.import cities.to_a, validate: validate + Bus.import buses.to_a, validate: validate + Service.import services.to_a, validate: validate + end + + def process_trips(json, validate) + cities = City.pluck(:name, :id).to_h + buses = Bus.all.each_with_object({}) do |bus, memo| + key = [bus.number, bus.model] + memo[key] = bus.id + end + services = Service.pluck(:name, :id).to_h + buses_services = Set.new + trips = [] + + json.each do |trip| + bus_key = [trip['bus']['number'], trip['bus']['model']] + bus_id = buses[bus_key] + trip['bus']['services'].each do |service_name| + service_id = services[service_name] + buses_services << { bus_id: bus_id, service_id: service_id } end + + trips << { + from_id: cities[trip['from']], + to_id: cities[trip['to']], + start_time: trip['start_time'], + duration_minutes: trip['duration_minutes'], + price_cents: trip['price_cents'], + bus_id: bus_id + } end + + BusesService.import buses_services.to_a, validate: false + Trip.import trips, validate: validate, batch_size: 1_000 end end diff --git a/profiling/Dockerfile b/profiling/Dockerfile new file mode 100644 index 00000000..1b74f90d --- /dev/null +++ b/profiling/Dockerfile @@ -0,0 +1,5 @@ +FROM perl:5.28-stretch +RUN apt-get update && apt-get install wget tar +RUN wget https://github.com/darold/pgbadger/archive/v11.0.tar.gz && tar xzf v11.0.tar.gz +RUN cd pgbadger-11.0/ && perl Makefile.PL && make && make install +CMD bash diff --git a/profiling/memory_prof.rb b/profiling/memory_prof.rb new file mode 100644 index 00000000..74d74023 --- /dev/null +++ b/profiling/memory_prof.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +puts 'Loading environment' +require 'memory_profiler' +require_relative '../config/environment' +require 'populate_database' + +puts 'Profiling...' +report = MemoryProfiler.report do + PopulateDatabase.call(file_path: 'fixtures/large.json') +end + +puts 'Print results to file' +report.pretty_print(color_output: false, scale_bytes: true, to_file: 'profiling/memory_profiler.txt') diff --git a/profiling/pgbadger.sh b/profiling/pgbadger.sh new file mode 100755 index 00000000..730c62cc --- /dev/null +++ b/profiling/pgbadger.sh @@ -0,0 +1,6 @@ +#!/bin/bash + +docker run -it --rm \ + -v $(pwd):/prof \ + pgbadger:1.0 \ + pgbadger -o /prof/out.html /prof/postgresql-2019-08-28_082731.log diff --git a/profiling/ruby_prof.rb b/profiling/ruby_prof.rb new file mode 100644 index 00000000..6d31765b --- /dev/null +++ b/profiling/ruby_prof.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +puts 'Loading environment' +require_relative '../config/environment' +require 'populate_database' + +# RubyProf.measure_mode = RubyProf::MEMORY +RubyProf.measure_mode = RubyProf::WALL_TIME + +puts 'Profiling...' +result = RubyProf.profile do + PopulateDatabase.call(file_path: 'fixtures/small.json') +end + +puts 'Print results to file' +printer = RubyProf::FlatPrinter.new(result) +printer.print(File.open("profiling/flat.txt", 'w+')) diff --git a/test/lib/populate_database_test.rb b/test/lib/populate_database_test.rb index b71ef58c..d6ce7a1a 100644 --- a/test/lib/populate_database_test.rb +++ b/test/lib/populate_database_test.rb @@ -3,6 +3,7 @@ require_relative '../test_helper' require 'minitest/autorun' require 'populate_database' +require 'benchmark' class PopulateDatabaseTest < Minitest::Test @@ -45,4 +46,11 @@ def test_call_is_idempotent assert_equal 2, Service.count assert_equal 10, Trip.count end + + def test_work_time + time = Benchmark.realtime { + PopulateDatabase.call(file_path: 'fixtures/small.json') + } + assert_operator 0.5, :>, time.real + end end From a5ffea15a4dedbd8d252d52c8c932e0a6f6cdc6b Mon Sep 17 00:00:00 2001 From: Denis Nazmutdinov Date: Tue, 3 Sep 2019 20:36:12 +0300 Subject: [PATCH 3/3] [Task-3] Optimize page loading time; add specs --- Gemfile | 4 ++ Gemfile.lock | 11 ++++++ app/controllers/trips_controller.rb | 2 +- app/views/trips/_delimiter.html.erb | 1 - app/views/trips/_service.html.erb | 1 - app/views/trips/_services.html.erb | 6 --- app/views/trips/_trip.html.erb | 5 --- app/views/trips/index.html.erb | 18 +++++++-- case-study.md | 45 +++++++++++++++++++++-- config/environments/development.rb | 6 +++ config/routes.rb | 2 + test/application_system_test_case.rb | 19 +++++++++- test/controllers/trips_controller_test.rb | 24 ++++++++++++ 13 files changed, 120 insertions(+), 24 deletions(-) delete mode 100644 app/views/trips/_delimiter.html.erb delete mode 100644 app/views/trips/_service.html.erb delete mode 100644 app/views/trips/_services.html.erb delete mode 100644 app/views/trips/_trip.html.erb create mode 100644 test/controllers/trips_controller_test.rb diff --git a/Gemfile b/Gemfile index 20af42b1..a9baae36 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,10 @@ group :development do gem 'web-console', '>= 3.3.0' gem 'listen', '>= 3.0.5', '< 3.2' gem 'ruby-prof' + gem 'meta_request' + # gem 'bullet' + gem 'pghero' + gem 'pg_query', '>= 0.9.0' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index acd54e67..90d99263 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -69,6 +69,9 @@ GEM mini_mime (>= 0.1.1) marcel (0.3.3) mimemagic (~> 0.3.2) + meta_request (0.7.2) + rack-contrib (>= 1.1, < 3) + railties (>= 3.0.0, < 7) method_source (0.9.2) mimemagic (0.3.3) mini_mime (1.0.1) @@ -79,8 +82,13 @@ GEM nokogiri (1.10.2) mini_portile2 (~> 2.4.0) pg (1.1.4) + pg_query (1.1.0) + pghero (2.3.0) + activerecord (>= 5) puma (3.12.1) rack (2.0.6) + rack-contrib (2.1.0) + rack (~> 2.0) rack-test (1.1.0) rack (>= 1.0, < 3) rails (5.2.3) @@ -141,7 +149,10 @@ DEPENDENCIES bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) + meta_request pg (>= 0.18, < 2.0) + pg_query (>= 0.9.0) + pghero puma (~> 3.11) rails (~> 5.2.3) ruby-prof diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be2..c35410ee 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).includes(bus: :services).order(:start_time).to_a end end diff --git a/app/views/trips/_delimiter.html.erb b/app/views/trips/_delimiter.html.erb deleted file mode 100644 index 3f845ad0..00000000 --- a/app/views/trips/_delimiter.html.erb +++ /dev/null @@ -1 +0,0 @@ -==================================================== diff --git a/app/views/trips/_service.html.erb b/app/views/trips/_service.html.erb deleted file mode 100644 index 178ea8c0..00000000 --- a/app/views/trips/_service.html.erb +++ /dev/null @@ -1 +0,0 @@ -
  • <%= "#{service.name}" %>
    • 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 deleted file mode 100644 index fa1de9aa..00000000 --- a/app/views/trips/_trip.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -
  • <%= "Отправление: #{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}" %>
    • diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce41..e6c70ee4 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -2,15 +2,25 @@ <%= "Автобусы #{@from.name} – #{@to.name}" %>

    - <%= "В расписании #{@trips.count} рейсов" %> + <%= "В расписании #{@trips.size} рейсов" %>

    <% @trips.each do |trip| %>
      - <%= render "trip", trip: trip %> +
    • <%= "Отправление: #{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 "services", services: trip.bus.services %> +
    • Сервисы в автобусе:
      • +
        + <% trip.bus.services.each do |service| %> +
      • <%= "#{service.name}" %>
        • + <% end %> +
      <% end %>
    - <%= render "delimiter" %> + ==================================================== <% end %> diff --git a/case-study.md b/case-study.md index bd15a55b..82bd7b46 100644 --- a/case-study.md +++ b/case-study.md @@ -1,4 +1,4 @@ -## 1. Оптимизация загрузки данных в базу приложения +### 1. Оптимизация загрузки данных в базу приложения #### Проблема Рейк-таск для загрузки данных в базу приложения работает слишком долго: ~8 секунд для файла с 1_000 записей, ~67 секунд для файла с 10_000 записей. @@ -12,7 +12,7 @@ #### Feedback-loop Для получения быстрой обратной связи добавлены скрипты генерации отчетов профилировщиков. -#### Процесс оптимизации (WIP) +#### Процесс оптимизации В исходном состоянии скрипт загрузки имеет следующие показатели для файла small.json (1к записей, размер ~304K) Потребление памяти (вместе со всем rails приложением) ``` @@ -45,8 +45,45 @@ Total retained: 10.61 MB (9120 objects) ``` Естественно что без стриминга мы грузим в память файл который обрабатываем целиком, но bonus часть я не выполнял, так что оставляю как есть. -#### Результат (WIP) +#### Результат В результате проведенной оптимизации удалось уложиться в рамки задачи. -#### Защита от регрессии (WIP) +#### Защита от регрессии Для защиты от регрессии добавлен тест, который проверяет время загрузки файла с 1к записей. + + +### 2. Оптимизация времени загрузки страницы + +#### Проблема +Страница с расписанием автобусов загружается слишком долго. + +#### Метрика +Чтобы понять, несут ли изменения положительный эффект, в качестве метрики выбрано время загрузки страницы для файла large.json. + +#### Гарантия корректности работы программы +Для гарантии корректной работы написан тест проверяющий что контроллер выдает корректный html для файла `example.json` + +#### Feedback-loop +Профилировщики выдают отчеты прямо на оптимизируемой странице. + +#### Процесс оптимизации +Перед оптимизацией страница загружалась за ~15-19 секунд для файла large.json. + +Rails-panel и rack mini profiler показали что основная масса времени тратится на рендеринг. +Решил собрать все темплейты в один файл, поскольку они очень простые. +В результате время загрузки страницы почти не изменилось. + +И оба профайлера выше и логи показали что страница делает ~2000 запросов в базу, прямо из вьюх (видимо поэтому rails panel отнес все затраты времени в рендеринг) +Уменьшил количество запросов к базе поправив N+2 (заюзал `includes`), и сделал принудительную загрузку данных в контроллере (`.to_a` на запросе) +Стало 6 запросов к базе, время загрузки сократилось значительно - до ~450-500 ms. Плюс стало четко видно сколько времени занимает непосредственно рендеринг - ~80-120ms + +Добавил индексов на id поля, но pg_hero поругал меня за дубликаты, пришлось удалить :) + +#### Результат +В результате проведенной оптимизации удалось уложиться в рамки задачи (~500ms)? + +#### Защита от регрессии +Для защиты от регрессии добавил тест, который проверяет время ответа контроллера. К сожалению время ответа очень сильно различается от теста к тесту, поэтому с запасом указал 0.5 сек для файла example.json + +#### P.S. +Понимаю, что нет предела совершенству - можно и "справочники" превратить в константы, и кеширование добавить во вьюхи и пагинацию на страницы, но устал :) diff --git a/config/environments/development.rb b/config/environments/development.rb index 1311e3e4..34feade8 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -58,4 +58,10 @@ # Use an evented file watcher to asynchronously detect changes in source code, # routes, locales, etc. This feature depends on the listen gem. config.file_watcher = ActiveSupport::EventedFileUpdateChecker + + # config.after_initialize do + # Bullet.enable = true + # Bullet.alert = true + # Bullet.console = true + # end end diff --git a/config/routes.rb b/config/routes.rb index a2da6a7b..394410ca 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,7 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html + mount PgHero::Engine, at: 'pghero' if Rails.env.development? + get "/" => "statistics#index" get "автобусы/:from/:to" => "trips#index" end diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index d19212ab..dbc04faf 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -1,5 +1,20 @@ -require "test_helper" +require_relative "test_helper" class ApplicationSystemTestCase < ActionDispatch::SystemTestCase - driven_by :selenium, using: :chrome, screen_size: [1400, 1400] + driven_by( + :selenium, + using: :chrome, + screen_size: [1400, 900], + options: { + desired_capabilities: { + chromeOptions: { + args: %w[headless disable-gpu], + prefs: { + 'modifyheaders.headers.name' => 'Accept-Language', + 'modifyheaders.headers.value' => 'ja,en', + } + } + } + } + ) end diff --git a/test/controllers/trips_controller_test.rb b/test/controllers/trips_controller_test.rb new file mode 100644 index 00000000..ae0ff89b --- /dev/null +++ b/test/controllers/trips_controller_test.rb @@ -0,0 +1,24 @@ +require_relative '../test_helper' +require 'populate_database' +require 'benchmark' + +class ArticlesControllerTest < ActionDispatch::IntegrationTest + VALID_BODY = "\n\n \n Task4\n \n \n\n \n \n \n\n \n

    \n Автобусы Самара – Москва\n

    \n

    \n В расписании 5 рейсов\n

    \n\n
      \n
    • Отправление: 17:30
      • \n
    • Прибытие: 18:07
      • \n
    • В пути: 0ч. 37мин.
      • \n
    • Цена: 1р. 73коп.
      • \n
    • Автобус: Икарус №123
      • \n\n
    • Сервисы в автобусе:
      • \n
        \n
      • Туалет
        • \n
      • WiFi
        • \n
      \n
    \n ====================================================\n
      \n
    • Отправление: 18:30
      • \n
    • Прибытие: 23:45
      • \n
    • В пути: 5ч. 15мин.
      • \n
    • Цена: 9р. 69коп.
      • \n
    • Автобус: Икарус №123
      • \n\n
    • Сервисы в автобусе:
      • \n
        \n
      • Туалет
        • \n
      • WiFi
        • \n
      \n
    \n ====================================================\n
      \n
    • Отправление: 19:30
      • \n
    • Прибытие: 19:51
      • \n
    • В пути: 0ч. 21мин.
      • \n
    • Цена: 6р. 63коп.
      • \n
    • Автобус: Икарус №123
      • \n\n
    • Сервисы в автобусе:
      • \n
        \n
      • Туалет
        • \n
      • WiFi
        • \n
      \n
    \n ====================================================\n
      \n
    • Отправление: 20:30
      • \n
    • Прибытие: 01:22
      • \n
    • В пути: 4ч. 52мин.
      • \n
    • Цена: 0р. 22коп.
      • \n
    • Автобус: Икарус №123
      • \n\n
    • Сервисы в автобусе:
      • \n
        \n
      • Туалет
        • \n
      • WiFi
        • \n
      \n
    \n ====================================================\n
      \n
    • Отправление: 21:30
      • \n
    • Прибытие: 00:33
      • \n
    • В пути: 3ч. 3мин.
      • \n
    • Цена: 8р. 46коп.
      • \n
    • Автобус: Икарус №123
      • \n\n
    • Сервисы в автобусе:
      • \n
        \n
      • Туалет
        • \n
      • WiFi
        • \n
      \n
    \n ====================================================\n\n \n\n" + + test 'Should return correct html' do + PopulateDatabase.call(file_path: 'fixtures/example.json') + get URI.encode('/автобусы/Самара/Москва') + + assert_response :success + assert_equal VALID_BODY, @response.body + end + + def test_work_time + PopulateDatabase.call(file_path: 'fixtures/small.json') + + time = Benchmark.realtime { + get URI.encode('/автобусы/Самара/Москва') + } + assert_operator 0.5, :>, time.real + end +end