From ec6d99658ec169815420a295177f5a48bf8a5ca1 Mon Sep 17 00:00:00 2001 From: "maksim.rabau" Date: Sat, 18 May 2024 02:16:23 +0300 Subject: [PATCH 1/2] Perform optimization --- Gemfile | 3 + Gemfile.lock | 14 ++- app/controllers/trips_controller.rb | 2 +- app/models/buses_service.rb | 6 + app/services/tasks/import_json_data.rb | 113 ++++++++++++++++++ app/views/trips/_delimiter.html.erb | 1 - app/views/trips/_services.html.erb | 6 - app/views/trips/_trip.html.erb | 11 ++ app/views/trips/index.html.erb | 12 +- case-study.md | 60 ++++++++++ config/database.yml | 2 + ...210140_add_uniqueness_constraint_to_bus.rb | 5 + ...4_add_uniqueness_constraint_to_services.rb | 5 + ...636_add_uniqueness_constraint_to_cities.rb | 5 + ..._uniqueness_constraint_to_buses_service.rb | 5 + db/schema.rb | 6 +- lib/tasks/utils.rake | 41 ++----- 17 files changed, 246 insertions(+), 51 deletions(-) create mode 100644 app/models/buses_service.rb create mode 100644 app/services/tasks/import_json_data.rb delete mode 100644 app/views/trips/_delimiter.html.erb delete mode 100644 app/views/trips/_services.html.erb create mode 100644 case-study.md create mode 100644 db/migrate/20240514210140_add_uniqueness_constraint_to_bus.rb create mode 100644 db/migrate/20240514210534_add_uniqueness_constraint_to_services.rb create mode 100644 db/migrate/20240514210636_add_uniqueness_constraint_to_cities.rb create mode 100644 db/migrate/20240515233732_add_uniqueness_constraint_to_buses_service.rb diff --git a/Gemfile b/Gemfile index e20b1260..f6d23dcb 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,9 @@ gem 'rails', '~> 5.2.3' gem 'pg', '>= 0.18', '< 2.0' gem 'puma', '~> 3.11' gem 'bootsnap', '>= 1.1.0', require: false +gem 'oj' +gem 'activerecord-import' +gem 'rack-mini-profiler' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console diff --git a/Gemfile.lock b/Gemfile.lock index fccf6f5f..da406f9c 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.6.0) + activerecord (>= 4.2) activestorage (5.2.3) actionpack (= 5.2.3) activerecord (= 5.2.3) @@ -51,7 +53,7 @@ GEM concurrent-ruby (1.1.5) crass (1.0.4) erubi (1.8.0) - ffi (1.10.0) + ffi (1.15.5) globalid (0.4.2) activesupport (>= 4.2.0) i18n (1.6.0) @@ -68,7 +70,9 @@ GEM marcel (0.3.3) mimemagic (~> 0.3.2) 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) @@ -76,9 +80,12 @@ GEM nio4r (2.3.1) nokogiri (1.10.2) mini_portile2 (~> 2.4.0) + oj (3.13.23) pg (1.1.4) puma (3.12.1) rack (2.0.6) + rack-mini-profiler (3.1.1) + rack (>= 1.2.0) rack-test (1.1.0) rack (>= 1.0, < 3) rails (5.2.3) @@ -134,11 +141,14 @@ PLATFORMS ruby DEPENDENCIES + activerecord-import bootsnap (>= 1.1.0) byebug listen (>= 3.0.5, < 3.2) + oj pg (>= 0.18, < 2.0) puma (~> 3.11) + rack-mini-profiler rails (~> 5.2.3) tzinfo-data web-console (>= 3.3.0) 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/app/models/buses_service.rb b/app/models/buses_service.rb new file mode 100644 index 00000000..976a0c10 --- /dev/null +++ b/app/models/buses_service.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class BusesService < ApplicationRecord + belongs_to :bus + belongs_to :service +end diff --git a/app/services/tasks/import_json_data.rb b/app/services/tasks/import_json_data.rb new file mode 100644 index 00000000..4918df74 --- /dev/null +++ b/app/services/tasks/import_json_data.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +module Tasks + class ImportJsonData + BATCH_SIZE = 1000 + + attr_accessor :cities, :trips, :services, :buses_hash, :buses_array, :buses_services, :cities_hash + attr_reader :filepath + + class << self + def call(filepath:) + new(filepath).call + end + end + + def initialize(filepath) + @filepath = filepath + @cities = [] + @cities_hash = {} + @trips = [] + @services = {} + @buses_hash = {} + @buses_array = [] + @buses_services = [] + end + + def call + ActiveRecord::Base.transaction do + create_services + + File.open(filepath) do |ff| + nesting = 0 + str = +'' + + until ff.eof? + ch = ff.read(1) # читаем по одному символу + if ch == '{' # начинается объект, повышается вложенность + nesting += 1 + str << ch + elsif ch == '}' # заканчивается объект, понижается вложенность + nesting -= 1 + str << ch + if nesting.zero? # если закончился объкет уровня trip, парсим и импортируем его + trip = Oj.load(str) + process_trip(trip) + str = +'' + end + elsif nesting >= 1 + str << ch + end + end + end + end + end + + private + + def create_services + Service::SERVICES.each do |service| + services[service] = Service.create!(name: service).id + end + end + + def process_trip(trip) + bus_key = "#{trip['bus']['number']}-#{trip['bus']['model']}" + + trip['bus']['services'].each do |service| + buses_services << [bus_key, services[service]] + end + + buses_array << [trip['bus']['number'], trip['bus']['model']] + + cities << [trip['from']] + cities << [trip['to']] + + trips << [trip['from'], trip['to'], trip['start_time'], trip['duration_minutes'], trip['price_cents'], bus_key] + + if trips.size == BATCH_SIZE + City.import %i[name], cities.uniq!, on_duplicate_key_ignore: true + update_cities_hash(cities) + + Bus.import %i[number model], buses_array, on_duplicate_key_ignore: true + update_bus_hash(buses_array) + + trips.map! do |trip| + [cities_hash[trip[0]], cities_hash[trip[1]], trip[2], trip[3], trip[4], buses_hash[trip[5]]] + end + Trip.import %i[from_id to_id start_time duration_minutes price_cents bus_id], trips + + buses_services.map! { |buses_service| [buses_hash[buses_service.first], buses_service.last] } + BusesService.import %i[bus_id service_id], buses_services, on_duplicate_key_ignore: true + + @buses_array = [] + @trips = [] + @cities = [] + @buses_services = [] + end + end + + def update_bus_hash(buses_array) + buses_array.uniq.each do |bus| + key = "#{bus.first}-#{bus.last}" + buses_hash[*key] = Bus.find_by(number: bus.first, model: bus.last).id unless buses_hash[*key] + end + end + + def update_cities_hash(cities) + cities.each do |city| + cities_hash[*city] = City.find_by(name: city).id unless cities_hash[*city] + end + end + 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/_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 @@ -
  • Сервисы в автобусе:
    • - diff --git a/app/views/trips/_trip.html.erb b/app/views/trips/_trip.html.erb index fa1de9aa..4f2243bd 100644 --- a/app/views/trips/_trip.html.erb +++ b/app/views/trips/_trip.html.erb @@ -1,5 +1,16 @@ + + +==================================================== diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce41..9aa038d9 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -2,15 +2,7 @@ <%= "Автобусы #{@from.name} – #{@to.name}" %>

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

    -<% @trips.each do |trip| %> - - <%= render "delimiter" %> -<% end %> +<%= render :partial => "trip", :collection => @trips %> diff --git a/case-study.md b/case-study.md new file mode 100644 index 00000000..bd18420f --- /dev/null +++ b/case-study.md @@ -0,0 +1,60 @@ +## Задача 1: Импорт данных +Нужно оптимизировать механизм перезагрузки расписания из файла так, чтобы он импортировал файл _large.json_ в БД posgresql в пределах минуты + +# Ход выполнения +Сразу было видно, даже без профилировщиков, что под каждую запись в бд делается отдельный INSERT запрос - это занимает основную часть времени. +Нужно применить подход с BATCH INSERT. Для этого использовался гем **activerecord-import**. Основная проблема была импортировать связи, т.к +требуется внешний ключ, т.е знание id непосредственно на уровне бд. Чтобы не делать лишних SELECT запросов, использовал хранение id автобусов, городов и сервисов в хеше на уровне оперативной памяти (хеш обновлялся каждый раз при добавлении новых объектов в базу) + +Также добавлял индексы (в том числе и составные) на уникальность, т.к при BATCH INSERT возможно появления дубликатов и нужна валидация на уровне бд + +# Результат +Для оценки скорости работы рейк таски использовал библиотеку Benchmark + +``` +bundle exec rake reload_json[fixtures/large.json] + +Total time is: 21.044087463000324 +``` + +-> получилось добиться импорта файла _large.json_ за +- 20 секунд + + +## Задача 2: Отображение расписаний +Сами страницы расписаний тоже формируются не эффективно и при росте объёмов начинают сильно тормозить. + +Нужно найти и устранить проблемы, замедляющие формирование этих страниц + +Предварительно импортирован файл _large.json_ + +# Ход выполнения +Для оценки скорости работы загрузки страницы использовал гем **rack-mini-profiler** + +### Находка №1 +- видна проблема n+1 при прогрузке ассоциаций в index.html.erb `trip.bus.services` +- Добавил `preload` в контроллер +- рендер страницы ускорился с 5 секунд до 2.7 секунд + +### Находка №2 +- видна долгая загрузка partial _trip.html.erb и _service.html.erb +- удалил конструкцию с `@trips.each` и переписал на более быстрый подход рендера с конструкцией `render partial: "trip", collection: @trips` +- аналогично для сервисов: `<%= render partial: "service", collection: trip.bus.services %>` +- рендер страницы ускорился с 2.7 секунд до 500мс + +### Находка №3 +- Делался лишний запрос `SELECT COUNT(*)` - исправил вызов `.count` на `.size` на @trips + +### Находка №4 +- Рендер вьюх всё ещё занимал основное время +- Дропнул ненужную вьюху `_delimiter.html.erb` и просто вставил текст из неё в родительскую вьюху `_trip.html.erb` +- рендер страницы ускорился с 500мс до 260мс + +# Результат +Для оценки скорости смотрел девелопмент лог и вывод гема rack-mini-profiler + +``` +Completed 200 OK in 263ms (Views: 235.8ms | ActiveRecord: 25.0ms) + +``` + +-> получилось добиться открытия страницы за 263мс diff --git a/config/database.yml b/config/database.yml index e116cfa6..a1658234 100644 --- a/config/database.yml +++ b/config/database.yml @@ -20,6 +20,8 @@ default: &default # For details on connection pooling, see Rails configuration guide # http://guides.rubyonrails.org/configuring.html#database-pooling pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> + username: task-4_development + password: new_password development: <<: *default diff --git a/db/migrate/20240514210140_add_uniqueness_constraint_to_bus.rb b/db/migrate/20240514210140_add_uniqueness_constraint_to_bus.rb new file mode 100644 index 00000000..5738c8bb --- /dev/null +++ b/db/migrate/20240514210140_add_uniqueness_constraint_to_bus.rb @@ -0,0 +1,5 @@ +class AddUniquenessConstraintToBus < ActiveRecord::Migration[5.2] + def change + add_index :buses, [:number, :model], unique: true + end +end diff --git a/db/migrate/20240514210534_add_uniqueness_constraint_to_services.rb b/db/migrate/20240514210534_add_uniqueness_constraint_to_services.rb new file mode 100644 index 00000000..61a5e1f5 --- /dev/null +++ b/db/migrate/20240514210534_add_uniqueness_constraint_to_services.rb @@ -0,0 +1,5 @@ +class AddUniquenessConstraintToServices < ActiveRecord::Migration[5.2] + def change + add_index :services, :name, unique: true + end +end diff --git a/db/migrate/20240514210636_add_uniqueness_constraint_to_cities.rb b/db/migrate/20240514210636_add_uniqueness_constraint_to_cities.rb new file mode 100644 index 00000000..48414cc7 --- /dev/null +++ b/db/migrate/20240514210636_add_uniqueness_constraint_to_cities.rb @@ -0,0 +1,5 @@ +class AddUniquenessConstraintToCities < ActiveRecord::Migration[5.2] + def change + add_index :cities, :name, unique: true + end +end diff --git a/db/migrate/20240515233732_add_uniqueness_constraint_to_buses_service.rb b/db/migrate/20240515233732_add_uniqueness_constraint_to_buses_service.rb new file mode 100644 index 00000000..866a5ded --- /dev/null +++ b/db/migrate/20240515233732_add_uniqueness_constraint_to_buses_service.rb @@ -0,0 +1,5 @@ +class AddUniquenessConstraintToBusesService < ActiveRecord::Migration[5.2] + def change + add_index :buses_services, [:bus_id, :service_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index f6921e45..c76991f5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_03_30_193044) do +ActiveRecord::Schema.define(version: 2024_05_15_233732) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -18,19 +18,23 @@ create_table "buses", force: :cascade do |t| t.string "number" t.string "model" + t.index ["number", "model"], name: "index_buses_on_number_and_model", unique: true end create_table "buses_services", force: :cascade do |t| t.integer "bus_id" t.integer "service_id" + t.index ["bus_id", "service_id"], name: "index_buses_services_on_bus_id_and_service_id", unique: true end create_table "cities", force: :cascade do |t| t.string "name" + t.index ["name"], name: "index_cities_on_name", unique: true end create_table "services", force: :cascade do |t| t.string "name" + t.index ["name"], name: "index_services_on_name", unique: true end create_table "trips", force: :cascade do |t| diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe871..7d4804b8 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,34 +1,15 @@ -# Наивная загрузка данных из json-файла в БД -# rake reload_json[fixtures/small.json] task :reload_json, [:file_name] => :environment do |_task, args| - json = JSON.parse(File.read(args.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;') + time = Benchmark.realtime { + ActiveRecord::Base.transaction do + Service.delete_all + Bus.delete_all + Trip.delete_all + BusesService.delete_all + City.delete_all + end - 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) + Tasks::ImportJsonData.call(filepath: args.file_name) + } - 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 + puts "Total time is: #{time}" end From 6f2d0b221ddd12c35b2a61354ce4fea52e573fe3 Mon Sep 17 00:00:00 2001 From: "maksim.rabau" Date: Sat, 18 May 2024 02:29:14 +0300 Subject: [PATCH 2/2] Change .size to .length --- app/views/trips/index.html.erb | 2 +- case-study.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index 9aa038d9..44cb564b 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -2,7 +2,7 @@ <%= "Автобусы #{@from.name} – #{@to.name}" %>

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

    <%= render :partial => "trip", :collection => @trips %> diff --git a/case-study.md b/case-study.md index bd18420f..f4d907b1 100644 --- a/case-study.md +++ b/case-study.md @@ -42,7 +42,7 @@ Total time is: 21.044087463000324 - рендер страницы ускорился с 2.7 секунд до 500мс ### Находка №3 -- Делался лишний запрос `SELECT COUNT(*)` - исправил вызов `.count` на `.size` на @trips +- Делался лишний запрос `SELECT COUNT(*)` - исправил вызов `.count` на `.length` на @trips ### Находка №4 - Рендер вьюх всё ещё занимал основное время