From 958ee50a1611ebaaac7f2562bb1040170e6b8ef0 Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Sat, 15 Feb 2025 18:07:55 +0400 Subject: [PATCH 1/8] add spec --- .rspec | 1 + Gemfile | 7 ++ Gemfile.lock | 35 +++++++++ case-study.md | 18 +++++ spec/controllers/trips_controller_spec.rb | 24 ++++++ spec/factories/buses.rb | 6 ++ spec/factories/cities.rb | 5 ++ spec/factories/services.rb | 5 ++ spec/factories/trips.rb | 10 +++ spec/models/bus_spec.rb | 5 ++ spec/models/city_spec.rb | 5 ++ spec/models/service_spec.rb | 5 ++ spec/models/trip_spec.rb | 5 ++ spec/rails_helper.rb | 66 ++++++++++++++++ spec/spec_helper.rb | 96 +++++++++++++++++++++++ spec/tasks/utils_spec.rb | 12 +++ 16 files changed, 305 insertions(+) create mode 100644 .rspec create mode 100644 case-study.md create mode 100644 spec/controllers/trips_controller_spec.rb create mode 100644 spec/factories/buses.rb create mode 100644 spec/factories/cities.rb create mode 100644 spec/factories/services.rb create mode 100644 spec/factories/trips.rb create mode 100644 spec/models/bus_spec.rb create mode 100644 spec/models/city_spec.rb create mode 100644 spec/models/service_spec.rb create mode 100644 spec/models/trip_spec.rb create mode 100644 spec/rails_helper.rb create mode 100644 spec/spec_helper.rb create mode 100644 spec/tasks/utils_spec.rb diff --git a/.rspec b/.rspec new file mode 100644 index 00000000..c99d2e73 --- /dev/null +++ b/.rspec @@ -0,0 +1 @@ +--require spec_helper diff --git a/Gemfile b/Gemfile index 34074dfd..a9c6f890 100644 --- a/Gemfile +++ b/Gemfile @@ -10,5 +10,12 @@ gem 'listen' gem 'bootsnap' gem 'rack-mini-profiler' +group :development, :test do + gem 'rspec-rails', '~> 7.1.1' + gem 'factory_bot_rails', '~> 6.4.4' + gem 'rails-controller-testing' + gem 'rspec-rake' +end + # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] diff --git a/Gemfile.lock b/Gemfile.lock index a9ddd818..ec8a63e7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -82,8 +82,14 @@ GEM connection_pool (2.5.0) crass (1.0.6) date (3.4.1) + diff-lcs (1.6.0) drb (2.2.1) erubi (1.13.1) + factory_bot (6.5.1) + activesupport (>= 6.1.0) + factory_bot_rails (6.4.4) + factory_bot (~> 6.5) + railties (>= 5.0.0) ffi (1.17.1-arm64-darwin) globalid (1.2.1) activesupport (>= 6.1) @@ -156,6 +162,10 @@ GEM activesupport (= 8.0.1) bundler (>= 1.15.0) railties (= 8.0.1) + rails-controller-testing (1.0.5) + actionpack (>= 5.0.1.rc1) + actionview (>= 5.0.1.rc1) + activesupport (>= 5.0.1.rc1) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest @@ -179,6 +189,26 @@ GEM psych (>= 4.0.0) reline (0.6.0) io-console (~> 0.5) + rspec-core (3.13.3) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.3) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.2) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-rails (7.1.1) + actionpack (>= 7.0) + activesupport (>= 7.0) + railties (>= 7.0) + rspec-core (~> 3.13) + rspec-expectations (~> 3.13) + rspec-mocks (~> 3.13) + rspec-support (~> 3.13) + rspec-rake (0.0.4) + rake + rspec-core + rspec-support (3.13.2) securerandom (0.4.1) stringio (3.1.2) thor (1.3.2) @@ -194,15 +224,20 @@ GEM zeitwerk (2.7.1) PLATFORMS + arm64-darwin-21 arm64-darwin-24 DEPENDENCIES bootsnap + factory_bot_rails (~> 6.4.4) listen pg puma rack-mini-profiler rails (~> 8.0.1) + rails-controller-testing + rspec-rails (~> 7.1.1) + rspec-rake tzinfo-data RUBY VERSION diff --git a/case-study.md b/case-study.md new file mode 100644 index 00000000..e3b88ee4 --- /dev/null +++ b/case-study.md @@ -0,0 +1,18 @@ +# Case-study оптимизации + +## Актуальная проблема + +## Разворачиваем приложение + +Приложение использует версию ruby 2.6.3 / rails 5.2.3. На текущий момент уже есть ruby 3.4 / rails 8. +Для начала надо развернуть приложение и убедиться, что оно работает. +Так как ruby достаточно старой версии проще запустить через докер, так как на мак эта версия собирается но с трудом +Также пришлось зафризить версию и сурс одного из гемов, так как эта версия уже удалена + +Запустилось. + +Первое что надо сделать - написать тесты и обновить ruby / rails +Добавим gem rspec-rails и напишем тесты для моделей, контроллера и таски + +Добавила спеки, можно приступать к обновлению ruby / rails + diff --git a/spec/controllers/trips_controller_spec.rb b/spec/controllers/trips_controller_spec.rb new file mode 100644 index 00000000..c5a0137c --- /dev/null +++ b/spec/controllers/trips_controller_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe TripsController, type: :controller do + describe 'GET #index' do + let!(:city_from) { create(:city, name: 'Samara') } + let!(:city_to) { create(:city, name: 'Moscow') } + let!(:trip) { create(:trip, from: city_from, to: city_to, start_time: 1.day.from_now) } + let!(:other_trip) { create(:trip, from: create(:city), to: create(:city)) } + + context 'with valid city names' do + before do + get :index, params: { from: 'Samara', to: 'Moscow' } + end + + it 'returns http success', :aggregate_failures do + expect(response).to have_http_status(:success) + expect(assigns(:from)).to eq(city_from) + expect(assigns(:to)).to eq(city_to) + expect(assigns(:trips)).to eq([trip]) + expect(response).to render_template(:index) + end + end + end +end diff --git a/spec/factories/buses.rb b/spec/factories/buses.rb new file mode 100644 index 00000000..07e902e7 --- /dev/null +++ b/spec/factories/buses.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :bus do + sequence(:number) { |n| "A#{n}23BC" } + model { Bus::MODELS.sample } + end +end diff --git a/spec/factories/cities.rb b/spec/factories/cities.rb new file mode 100644 index 00000000..7f81a06e --- /dev/null +++ b/spec/factories/cities.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :city do + sequence(:name) { |n| "City#{n}" } + end +end diff --git a/spec/factories/services.rb b/spec/factories/services.rb new file mode 100644 index 00000000..bf9fdccf --- /dev/null +++ b/spec/factories/services.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :service do + name { Service::SERVICES.sample } + end +end diff --git a/spec/factories/trips.rb b/spec/factories/trips.rb new file mode 100644 index 00000000..05af46da --- /dev/null +++ b/spec/factories/trips.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :trip do + association :from, factory: :city + association :to, factory: :city + association :bus + start_time { "12:00" } + duration_minutes { 120 } + price_cents { 10000 } + end +end diff --git a/spec/models/bus_spec.rb b/spec/models/bus_spec.rb new file mode 100644 index 00000000..c5e9b2b6 --- /dev/null +++ b/spec/models/bus_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe Bus, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/city_spec.rb b/spec/models/city_spec.rb new file mode 100644 index 00000000..17ead569 --- /dev/null +++ b/spec/models/city_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe City, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb new file mode 100644 index 00000000..c24480f2 --- /dev/null +++ b/spec/models/service_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe Service, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/trip_spec.rb b/spec/models/trip_spec.rb new file mode 100644 index 00000000..e6de3667 --- /dev/null +++ b/spec/models/trip_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe Trip, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb new file mode 100644 index 00000000..0cb7e4fa --- /dev/null +++ b/spec/rails_helper.rb @@ -0,0 +1,66 @@ +# This file is copied to spec/ when you run 'rails generate rspec:install' +require 'spec_helper' +ENV['RAILS_ENV'] ||= 'test' +require_relative '../config/environment' +# Prevent database truncation if the environment is production +abort("The Rails environment is running in production mode!") if Rails.env.production? +require 'rspec/rails' +# Add additional requires below this line. Rails is not loaded until this point! + +# Requires supporting ruby files with custom matchers and macros, etc, in +# spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are +# run as spec files by default. This means that files in spec/support that end +# in _spec.rb will both be required and run as specs, causing the specs to be +# run twice. It is recommended that you do not name files matching this glob to +# end with _spec.rb. You can configure this pattern with the --pattern +# option on the command line or in ~/.rspec, .rspec or `.rspec-local`. +# +# The following line is provided for convenience purposes. It has the downside +# of increasing the boot-up time by auto-requiring all files in the support +# directory. Alternatively, in the individual `*_spec.rb` files, manually +# require only the support files necessary. +# +# Dir[Rails.root.join('spec', 'support', '**', '*.rb')].sort.each { |f| require f } + +# Checks for pending migrations and applies them before tests are run. +# If you are not using ActiveRecord, you can remove these lines. +begin + ActiveRecord::Migration.maintain_test_schema! +rescue ActiveRecord::PendingMigrationError => e + puts e.to_s.strip + exit 1 +end +RSpec.configure do |config| + # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures + # config.fixture_path = ["#{::Rails.root}/spec/fixtures"] + + config.include FactoryBot::Syntax::Methods + + # If you're not using ActiveRecord, or you'd prefer not to run each of your + # examples within a transaction, remove the following line or assign false + # instead of true. + config.use_transactional_fixtures = true + + # You can uncomment this line to turn off ActiveRecord support entirely. + # config.use_active_record = false + + # RSpec Rails can automatically mix in different behaviours to your tests + # based on their file location, for example enabling you to call `get` and + # `post` in specs under `spec/controllers`. + # + # You can disable this behaviour by removing the line below, and instead + # explicitly tag your specs with their type, e.g.: + # + # RSpec.describe UsersController, type: :controller do + # # ... + # end + # + # The different available types are documented in the features, such as in + # https://relishapp.com/rspec/rspec-rails/docs + config.infer_spec_type_from_file_location! + + # Filter lines from Rails gems in backtraces. + config.filter_rails_from_backtrace! + # arbitrary gems may also be filtered via: + # config.filter_gems_from_backtrace("gem name") +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 00000000..e4cd252f --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,96 @@ +# This file was generated by the `rails generate rspec:install` command. Conventionally, all +# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. +# The generated `.rspec` file contains `--require spec_helper` which will cause +# this file to always be loaded, without a need to explicitly require it in any +# files. +# +# Given that it is always loaded, you are encouraged to keep this file as +# light-weight as possible. Requiring heavyweight dependencies from this file +# will add to the boot time of your test suite on EVERY test run, even for an +# individual file that may not need all of that loaded. Instead, consider making +# a separate helper file that requires the additional dependencies and performs +# the additional setup, and require it from the spec files that actually need +# it. +# +# See https://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration +require 'rspec/rake' + +RSpec.configure do |config| + # rspec-expectations config goes here. You can use an alternate + # assertion/expectation library such as wrong or the stdlib/minitest + # assertions if you prefer. + config.expect_with :rspec do |expectations| + # This option will default to `true` in RSpec 4. It makes the `description` + # and `failure_message` of custom matchers include text for helper methods + # defined using `chain`, e.g.: + # be_bigger_than(2).and_smaller_than(4).description + # # => "be bigger than 2 and smaller than 4" + # ...rather than: + # # => "be bigger than 2" + expectations.include_chain_clauses_in_custom_matcher_descriptions = true + end + + # rspec-mocks config goes here. You can use an alternate test double + # library (such as bogus or mocha) by changing the `mock_with` option here. + config.mock_with :rspec do |mocks| + # Prevents you from mocking or stubbing a method that does not exist on + # a real object. This is generally recommended, and will default to + # `true` in RSpec 4. + mocks.verify_partial_doubles = true + end + + # This option will default to `:apply_to_host_groups` in RSpec 4 (and will + # have no way to turn it off -- the option exists only for backwards + # compatibility in RSpec 3). It causes shared context metadata to be + # inherited by the metadata hash of host groups and examples, rather than + # triggering implicit auto-inclusion in groups with matching metadata. + config.shared_context_metadata_behavior = :apply_to_host_groups + +# The settings below are suggested to provide a good initial experience +# with RSpec, but feel free to customize to your heart's content. +=begin + # This allows you to limit a spec run to individual examples or groups + # you care about by tagging them with `:focus` metadata. When nothing + # is tagged with `:focus`, all examples get run. RSpec also provides + # aliases for `it`, `describe`, and `context` that include `:focus` + # metadata: `fit`, `fdescribe` and `fcontext`, respectively. + config.filter_run_when_matching :focus + + # Allows RSpec to persist some state between runs in order to support + # the `--only-failures` and `--next-failure` CLI options. We recommend + # you configure your source control system to ignore this file. + config.example_status_persistence_file_path = "spec/examples.txt" + + # Limits the available syntax to the non-monkey patched syntax that is + # recommended. For more details, see: + # https://rspec.info/features/3-12/rspec-core/configuration/zero-monkey-patching-mode/ + config.disable_monkey_patching! + + # Many RSpec users commonly either run the entire suite or an individual + # file, and it's useful to allow more verbose output when running an + # individual spec file. + if config.files_to_run.one? + # Use the documentation formatter for detailed output, + # unless a formatter has already been configured + # (e.g. via a command-line flag). + config.default_formatter = "doc" + end + + # Print the 10 slowest examples and example groups at the + # end of the spec run, to help surface which specs are running + # particularly slow. + config.profile_examples = 10 + + # Run specs in random order to surface order dependencies. If you find an + # order dependency and want to debug it, you can fix the order by providing + # the seed, which is printed after each run. + # --seed 1234 + config.order = :random + + # Seed global randomization in this process using the `--seed` CLI option. + # Setting this allows you to use `--seed` to deterministically reproduce + # test failures related to randomization by passing the same `--seed` value + # as the one that triggered the failure. + Kernel.srand config.seed +=end +end diff --git a/spec/tasks/utils_spec.rb b/spec/tasks/utils_spec.rb new file mode 100644 index 00000000..dff2fcfb --- /dev/null +++ b/spec/tasks/utils_spec.rb @@ -0,0 +1,12 @@ +require 'rails_helper' + +describe 'reload_json', :rakefile => 'utils', type: :task do + before { task.invoke 'fixtures/example.json' } + + it 'loads data from JSON file', :aggregate_failures do + expect(City.count).to eq(2) + expect(Bus.count).to eq(1) + expect(Service.count).to eq(2) + expect(Trip.count).to eq(10) + end +end From c527ab1da93e150e670d4263c6c68952013b1220 Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Sun, 16 Feb 2025 14:08:35 +0400 Subject: [PATCH 2/8] rubocop, rspec --- Gemfile | 1 + Gemfile.lock | 28 ++++++++++++++++++++++++++++ lib/tasks/utils.rake | 1 + spec/tasks/utils_spec.rb | 40 +++++++++++++++++++++++++++++++++++----- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index a9c6f890..419dacbb 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,7 @@ gem 'puma' gem 'listen' gem 'bootsnap' gem 'rack-mini-profiler' +gem 'rubocop', require: false group :development, :test do gem 'rspec-rails', '~> 7.1.1' diff --git a/Gemfile.lock b/Gemfile.lock index ec8a63e7..bf53a6e9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -72,6 +72,7 @@ GEM securerandom (>= 0.3) tzinfo (~> 2.0, >= 2.0.5) uri (>= 0.13.1) + ast (2.4.2) base64 (0.2.0) benchmark (0.4.0) bigdecimal (3.1.9) @@ -100,6 +101,9 @@ GEM pp (>= 0.6.0) rdoc (>= 4.0.0) reline (>= 0.4.2) + json (2.10.1) + language_server-protocol (3.17.0.4) + lint_roller (1.1.0) listen (3.9.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) @@ -128,6 +132,10 @@ GEM nio4r (2.7.4) nokogiri (1.18.2-arm64-darwin) racc (~> 1.4) + parallel (1.26.3) + parser (3.3.7.1) + ast (~> 2.4.1) + racc pg (1.5.9) pp (0.6.2) prettyprint @@ -181,12 +189,14 @@ GEM rake (>= 12.2) thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) + rainbow (3.1.1) rake (13.2.1) rb-fsevent (0.11.2) rb-inotify (0.11.1) ffi (~> 1.0) rdoc (6.12.0) psych (>= 4.0.0) + regexp_parser (2.10.0) reline (0.6.0) io-console (~> 0.5) rspec-core (3.13.3) @@ -209,12 +219,29 @@ GEM rake rspec-core rspec-support (3.13.2) + rubocop (1.72.1) + json (~> 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) + parallel (~> 1.10) + parser (>= 3.3.0.2) + rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.38.0, < 2.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.38.0) + parser (>= 3.3.1.0) + ruby-progressbar (1.13.0) securerandom (0.4.1) stringio (3.1.2) thor (1.3.2) timeout (0.4.3) tzinfo (2.0.6) concurrent-ruby (~> 1.0) + unicode-display_width (3.1.4) + unicode-emoji (~> 4.0, >= 4.0.4) + unicode-emoji (4.0.4) uri (1.0.2) useragent (0.16.11) websocket-driver (0.7.7) @@ -238,6 +265,7 @@ DEPENDENCIES rails-controller-testing rspec-rails (~> 7.1.1) rspec-rake + rubocop tzinfo-data RUBY VERSION diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index 540fe871..d907206c 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -1,5 +1,6 @@ # Наивная загрузка данных из json-файла в БД # rake reload_json[fixtures/small.json] +desc 'Reload data from json file' task :reload_json, [:file_name] => :environment do |_task, args| json = JSON.parse(File.read(args.file_name)) diff --git a/spec/tasks/utils_spec.rb b/spec/tasks/utils_spec.rb index dff2fcfb..202a25f9 100644 --- a/spec/tasks/utils_spec.rb +++ b/spec/tasks/utils_spec.rb @@ -3,10 +3,40 @@ describe 'reload_json', :rakefile => 'utils', type: :task do before { task.invoke 'fixtures/example.json' } - it 'loads data from JSON file', :aggregate_failures do - expect(City.count).to eq(2) - expect(Bus.count).to eq(1) - expect(Service.count).to eq(2) - expect(Trip.count).to eq(10) + it 'creates buses' do + expect(Bus.all).to contain_exactly(have_attributes(number: '123', model: 'Икарус')) + end + + it 'creates cities' do + expect(City.all).to contain_exactly( + have_attributes(name: 'Москва'), + have_attributes(name: 'Самара') + ) + end + + it 'creates services' do + expect(Service.all).to contain_exactly( + have_attributes(name: 'WiFi'), + have_attributes(name: 'Туалет') + ) + end + + it 'creates trips' do + samara = City.find_by(name: 'Самара') + moscow = City.find_by(name: 'Москва') + bus = Bus.find_by(number: '123') + + expect(Trip.all).to contain_exactly( + have_attributes(start_time: '11:00', duration_minutes: 168, price_cents: 474, from: moscow, to: samara, bus:), + have_attributes(start_time: '17:30', duration_minutes: 37, price_cents: 173, from: samara, to: moscow, bus:), + have_attributes(start_time: '12:00', duration_minutes: 323, price_cents: 672, from: moscow, to: samara, bus:), + have_attributes(start_time: '18:30', duration_minutes: 315, price_cents: 969, from: samara, to: moscow, bus:), + have_attributes(start_time: '13:00', duration_minutes: 304, price_cents: 641, from: moscow, to: samara, bus:), + have_attributes(start_time: '19:30', duration_minutes: 21, price_cents: 663, from: samara, to: moscow, bus:), + have_attributes(start_time: '14:00', duration_minutes: 598, price_cents: 629, from: moscow, to: samara, bus:), + have_attributes(start_time: '20:30', duration_minutes: 292, price_cents: 22, from: samara, to: moscow, bus:), + have_attributes(start_time: '15:00', duration_minutes: 127, price_cents: 795, from: moscow, to: samara, bus:), + have_attributes(start_time: '21:30', duration_minutes: 183, price_cents: 846, from: samara, to: moscow, bus:) + ) end end From a8fa87d980b4c5ef0a124b3b4d8bc7a97f7f9a64 Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Sun, 16 Feb 2025 15:55:00 +0400 Subject: [PATCH 3/8] move logic to service class --- Gemfile | 4 +++ Gemfile.lock | 8 ++++++ app/services/data_loader.rb | 36 +++++++++++++++++++++++++ case-study.md | 19 ++++++------- lib/tasks/utils.rake | 31 +--------------------- spec/services/data_loader_spec.rb | 44 +++++++++++++++++++++++++++++++ spec/tasks/utils_spec.rb | 39 +++------------------------ 7 files changed, 105 insertions(+), 76 deletions(-) create mode 100644 app/services/data_loader.rb create mode 100644 spec/services/data_loader_spec.rb diff --git a/Gemfile b/Gemfile index 419dacbb..a8948362 100644 --- a/Gemfile +++ b/Gemfile @@ -10,6 +10,10 @@ gem 'listen' gem 'bootsnap' gem 'rack-mini-profiler' gem 'rubocop', require: false +gem 'pghero' +gem 'benchmark' +gem 'ruby-prof' +gem 'stackprof' group :development, :test do gem 'rspec-rails', '~> 7.1.1' diff --git a/Gemfile.lock b/Gemfile.lock index bf53a6e9..2b19b8d6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -137,6 +137,8 @@ GEM ast (~> 2.4.1) racc pg (1.5.9) + pghero (3.6.1) + activerecord (>= 6.1) pp (0.6.2) prettyprint prettyprint (0.2.0) @@ -232,8 +234,10 @@ GEM unicode-display_width (>= 2.4.0, < 4.0) rubocop-ast (1.38.0) parser (>= 3.3.1.0) + ruby-prof (1.7.1) ruby-progressbar (1.13.0) securerandom (0.4.1) + stackprof (0.2.27) stringio (3.1.2) thor (1.3.2) timeout (0.4.3) @@ -255,10 +259,12 @@ PLATFORMS arm64-darwin-24 DEPENDENCIES + benchmark bootsnap factory_bot_rails (~> 6.4.4) listen pg + pghero puma rack-mini-profiler rails (~> 8.0.1) @@ -266,6 +272,8 @@ DEPENDENCIES rspec-rails (~> 7.1.1) rspec-rake rubocop + ruby-prof + stackprof tzinfo-data RUBY VERSION diff --git a/app/services/data_loader.rb b/app/services/data_loader.rb new file mode 100644 index 00000000..b5a3afc2 --- /dev/null +++ b/app/services/data_loader.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class DataLoader + def self.load(file_name) + 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 diff --git a/case-study.md b/case-study.md index e3b88ee4..9ff5f924 100644 --- a/case-study.md +++ b/case-study.md @@ -2,17 +2,14 @@ ## Актуальная проблема -## Разворачиваем приложение +1) Добавила в проект тесты +2) Начем с оптимизации импорта данных -Приложение использует версию ruby 2.6.3 / rails 5.2.3. На текущий момент уже есть ruby 3.4 / rails 8. -Для начала надо развернуть приложение и убедиться, что оно работает. -Так как ruby достаточно старой версии проще запустить через докер, так как на мак эта версия собирается но с трудом -Также пришлось зафризить версию и сурс одного из гемов, так как эта версия уже удалена +## Оптимизация импорта данных -Запустилось. - -Первое что надо сделать - написать тесты и обновить ruby / rails -Добавим gem rspec-rails и напишем тесты для моделей, контроллера и таски - -Добавила спеки, можно приступать к обновлению ruby / rails +напишем раннер для запуска / мониторинга времени / памяти выполнения скрипта импорта +1) время выполнения fixtures/small.json - 10.167809 +2) воспользуемся rbspy для профилирования скрипта импорта +3) rbspy / rubyprof выдают много лишних данных при вызове таски, поэтому можно вынести импорт в отдельный класс и профилировать его +4) вынесла в класс DataLoader, перенесла тесты \ No newline at end of file diff --git a/lib/tasks/utils.rake b/lib/tasks/utils.rake index d907206c..7f0b935f 100644 --- a/lib/tasks/utils.rake +++ b/lib/tasks/utils.rake @@ -2,34 +2,5 @@ # rake reload_json[fixtures/small.json] desc 'Reload data from json file' 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;') - - 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 + DataLoader.load(args.file_name) end diff --git a/spec/services/data_loader_spec.rb b/spec/services/data_loader_spec.rb new file mode 100644 index 00000000..715b79c1 --- /dev/null +++ b/spec/services/data_loader_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +RSpec.describe DataLoader do + subject(:load) { described_class.load('fixtures/example.json') } + + before { load } + + it 'creates buses' do + expect(Bus.all).to contain_exactly(have_attributes(number: '123', model: 'Икарус')) + end + + it 'creates cities' do + expect(City.all).to contain_exactly( + have_attributes(name: 'Москва'), + have_attributes(name: 'Самара') + ) + end + + it 'creates services' do + expect(Service.all).to contain_exactly( + have_attributes(name: 'WiFi'), + have_attributes(name: 'Туалет') + ) + end + + it 'creates trips' do + samara = City.find_by(name: 'Самара') + moscow = City.find_by(name: 'Москва') + bus = Bus.find_by(number: '123') + + expect(Trip.all).to contain_exactly( + have_attributes(start_time: '11:00', duration_minutes: 168, price_cents: 474, from: moscow, to: samara, bus:), + have_attributes(start_time: '17:30', duration_minutes: 37, price_cents: 173, from: samara, to: moscow, bus:), + have_attributes(start_time: '12:00', duration_minutes: 323, price_cents: 672, from: moscow, to: samara, bus:), + have_attributes(start_time: '18:30', duration_minutes: 315, price_cents: 969, from: samara, to: moscow, bus:), + have_attributes(start_time: '13:00', duration_minutes: 304, price_cents: 641, from: moscow, to: samara, bus:), + have_attributes(start_time: '19:30', duration_minutes: 21, price_cents: 663, from: samara, to: moscow, bus:), + have_attributes(start_time: '14:00', duration_minutes: 598, price_cents: 629, from: moscow, to: samara, bus:), + have_attributes(start_time: '20:30', duration_minutes: 292, price_cents: 22, from: samara, to: moscow, bus:), + have_attributes(start_time: '15:00', duration_minutes: 127, price_cents: 795, from: moscow, to: samara, bus:), + have_attributes(start_time: '21:30', duration_minutes: 183, price_cents: 846, from: samara, to: moscow, bus:) + ) + end +end diff --git a/spec/tasks/utils_spec.rb b/spec/tasks/utils_spec.rb index 202a25f9..87ab6cae 100644 --- a/spec/tasks/utils_spec.rb +++ b/spec/tasks/utils_spec.rb @@ -1,42 +1,11 @@ require 'rails_helper' describe 'reload_json', :rakefile => 'utils', type: :task do - before { task.invoke 'fixtures/example.json' } + before do + allow(DataLoader).to receive(:load) - it 'creates buses' do - expect(Bus.all).to contain_exactly(have_attributes(number: '123', model: 'Икарус')) + task.invoke 'fixtures/example.json' end - it 'creates cities' do - expect(City.all).to contain_exactly( - have_attributes(name: 'Москва'), - have_attributes(name: 'Самара') - ) - end - - it 'creates services' do - expect(Service.all).to contain_exactly( - have_attributes(name: 'WiFi'), - have_attributes(name: 'Туалет') - ) - end - - it 'creates trips' do - samara = City.find_by(name: 'Самара') - moscow = City.find_by(name: 'Москва') - bus = Bus.find_by(number: '123') - - expect(Trip.all).to contain_exactly( - have_attributes(start_time: '11:00', duration_minutes: 168, price_cents: 474, from: moscow, to: samara, bus:), - have_attributes(start_time: '17:30', duration_minutes: 37, price_cents: 173, from: samara, to: moscow, bus:), - have_attributes(start_time: '12:00', duration_minutes: 323, price_cents: 672, from: moscow, to: samara, bus:), - have_attributes(start_time: '18:30', duration_minutes: 315, price_cents: 969, from: samara, to: moscow, bus:), - have_attributes(start_time: '13:00', duration_minutes: 304, price_cents: 641, from: moscow, to: samara, bus:), - have_attributes(start_time: '19:30', duration_minutes: 21, price_cents: 663, from: samara, to: moscow, bus:), - have_attributes(start_time: '14:00', duration_minutes: 598, price_cents: 629, from: moscow, to: samara, bus:), - have_attributes(start_time: '20:30', duration_minutes: 292, price_cents: 22, from: samara, to: moscow, bus:), - have_attributes(start_time: '15:00', duration_minutes: 127, price_cents: 795, from: moscow, to: samara, bus:), - have_attributes(start_time: '21:30', duration_minutes: 183, price_cents: 846, from: samara, to: moscow, bus:) - ) - end + it { expect(DataLoader).to have_received(:load).with('fixtures/example.json') } end From bc1104c8ab1139c49b98fbe8aa02b1d4a8ad7f76 Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Sun, 16 Feb 2025 19:45:45 +0400 Subject: [PATCH 4/8] json streamer --- .gitignore | 1 + Gemfile | 1 + Gemfile.lock | 5 +++ app/services/data_loader.rb | 57 ++++++++++++++++------------- app/services/json_streamer.rb | 47 ++++++++++++++++++++++++ lib/tasks/profile.rake | 45 +++++++++++++++++++++++ spec/services/json_streamer_spec.rb | 32 ++++++++++++++++ 7 files changed, 162 insertions(+), 26 deletions(-) create mode 100644 app/services/json_streamer.rb create mode 100644 lib/tasks/profile.rake create mode 100644 spec/services/json_streamer_spec.rb diff --git a/.gitignore b/.gitignore index 59c74047..68b1bcf8 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /tmp /log /public +/profile diff --git a/Gemfile b/Gemfile index a8948362..8e410820 100644 --- a/Gemfile +++ b/Gemfile @@ -14,6 +14,7 @@ gem 'pghero' gem 'benchmark' gem 'ruby-prof' gem 'stackprof' +gem 'oj' group :development, :test do gem 'rspec-rails', '~> 7.1.1' diff --git a/Gemfile.lock b/Gemfile.lock index 2b19b8d6..0e786d2e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -132,6 +132,10 @@ GEM nio4r (2.7.4) nokogiri (1.18.2-arm64-darwin) racc (~> 1.4) + oj (3.16.9) + bigdecimal (>= 3.0) + ostruct (>= 0.2) + ostruct (0.6.1) parallel (1.26.3) parser (3.3.7.1) ast (~> 2.4.1) @@ -263,6 +267,7 @@ DEPENDENCIES bootsnap factory_bot_rails (~> 6.4.4) listen + oj pg pghero puma diff --git a/app/services/data_loader.rb b/app/services/data_loader.rb index b5a3afc2..7846573a 100644 --- a/app/services/data_loader.rb +++ b/app/services/data_loader.rb @@ -1,36 +1,41 @@ -# frozen_string_literal: true - class DataLoader - def self.load(file_name) - json = JSON.parse(File.read(file_name)) - ActiveRecord::Base.transaction do + class << self + def load(filename) + ActiveRecord::Base.transaction do + clean_database + + JsonStreamer.stream(filename).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 + + private + + def clean_database 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/app/services/json_streamer.rb b/app/services/json_streamer.rb new file mode 100644 index 00000000..e2089e8f --- /dev/null +++ b/app/services/json_streamer.rb @@ -0,0 +1,47 @@ +class JsonStreamer + def self.stream(filename) + new(filename).stream + end + + def initialize(filename) + @file = File.open(filename, 'r:UTF-8') + end + + def stream + Enumerator.new do |yielder| + loop do + yielder << Oj.load(object) + rescue Oj::ParseError, TypeError => _e + break + end + end + end + + private + + attr_reader :file + + def object + nesting = 0 + str = '' + + while nesting > 0 || str.empty? + ch = file.getc + + return if file.eof? + + case + when ch == '{' + nesting += 1 + str << ch + when ch == '}' + nesting -= 1 + str << ch + when nesting >= 1 + str << ch + end + end + + str + end +end diff --git a/lib/tasks/profile.rake b/lib/tasks/profile.rake new file mode 100644 index 00000000..ae1ff9ff --- /dev/null +++ b/lib/tasks/profile.rake @@ -0,0 +1,45 @@ +namespace :profile do + desc 'Time' + task time: :environment do + require 'benchmark' + + puts(Benchmark.measure { DataLoader.load('fixtures/small.json') }) + end + + desc 'Ruby prof' + task rubyprof: :environment do + report_path = Rails.root.join('profile', 'ruby_prof_reports') + + result = RubyProf::Profile.profile do + DataLoader.load('fixtures/small.json') + end + + printer = RubyProf::GraphHtmlPrinter.new(result) + printer.print(File.open(Rails.root.join(report_path, 'graph.html'), 'w+'), :min_percent=>0) + + printer = RubyProf::CallStackPrinter.new(result) + printer.print(File.open(Rails.root.join(report_path, 'callstack.html'), 'w+')) + end + + desc 'Memory monitoring' + task memory_monitoring: :environment do + def memory_usage + (`ps -o rss= -p #{Process.pid}`.to_i / 1024) + end + + io = File.open(Rails.root.join('profile', 'memory_usage.txt'), 'w') + io << format("INITIAL MEMORY USAGE: %d MB\n", memory_usage) + monitor_thread = Thread.new do + while true + io << format("MEMORY USAGE: %d MB\n", memory_usage) + sleep(1) + end + ensure + io << format("FINAL MEMORY USAGE: %d MB\n", memory_usage) + io.close + end + + DataLoader.load('fixtures/small.json') + monitor_thread.kill + end +end diff --git a/spec/services/json_streamer_spec.rb b/spec/services/json_streamer_spec.rb new file mode 100644 index 00000000..d28ea816 --- /dev/null +++ b/spec/services/json_streamer_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe JsonStreamer do + subject(:stream) { described_class.stream('fixtures/example.json') } + + it 'streams JSON objects' do + expect(stream.to_a).to contain_exactly( + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 168, + 'from' => 'Москва', 'price_cents' => 474, 'start_time' => '11:00', 'to' => 'Самара' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 37, + 'from' => 'Самара', 'price_cents' => 173, 'start_time' => '17:30', 'to' => 'Москва' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 323, + 'from' => 'Москва', 'price_cents' => 672, 'start_time' => '12:00', 'to' => 'Самара' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 315, + 'from' => 'Самара', 'price_cents' => 969, 'start_time' => '18:30', 'to' => 'Москва' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 304, + 'from' => 'Москва', 'price_cents' => 641, 'start_time' => '13:00', 'to' => 'Самара' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 21, + 'from' => 'Самара', 'price_cents' => 663, 'start_time' => '19:30', 'to' => 'Москва' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 598, + 'from' => 'Москва', 'price_cents' => 629, 'start_time' => '14:00', 'to' => 'Самара' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 292, + 'from' => 'Самара', 'price_cents' => 22, 'start_time' => '20:30', 'to' => 'Москва' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 127, + 'from' => 'Москва', 'price_cents' => 795, 'start_time' => '15:00', 'to' => 'Самара' }, + { 'bus' => { 'model' => 'Икарус', 'number' => '123', 'services' => %w[Туалет WiFi] }, 'duration_minutes' => 183, + 'from' => 'Самара', 'price_cents' => 846, 'start_time' => '21:30', 'to' => 'Москва' } + ) + end +end From 3a607f4da79dc1eebf13d982a8d251df279b288f Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Sun, 16 Feb 2025 22:58:18 +0400 Subject: [PATCH 5/8] json streamer --- app/services/json_streamer.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/services/json_streamer.rb b/app/services/json_streamer.rb index e2089e8f..20a9ce37 100644 --- a/app/services/json_streamer.rb +++ b/app/services/json_streamer.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class JsonStreamer def self.stream(filename) new(filename).stream @@ -23,7 +25,7 @@ def stream def object nesting = 0 - str = '' + str = +'' while nesting > 0 || str.empty? ch = file.getc From 7151f82a827121bc574e81fc612132fff2635d76 Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Tue, 18 Feb 2025 18:27:02 +0400 Subject: [PATCH 6/8] json streamer --- app/services/json_streamer.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/app/services/json_streamer.rb b/app/services/json_streamer.rb index 20a9ce37..595a30b0 100644 --- a/app/services/json_streamer.rb +++ b/app/services/json_streamer.rb @@ -32,16 +32,11 @@ def object return if file.eof? - case - when ch == '{' - nesting += 1 - str << ch - when ch == '}' - nesting -= 1 - str << ch - when nesting >= 1 - str << ch - end + nesting += 1 if ch == '{' + str << ch if nesting >= 1 + nesting -= 1 if ch == '}' + + str end str From c536943a470b3c70bb3484202714c5e4afbc2b95 Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Tue, 18 Feb 2025 21:19:42 +0400 Subject: [PATCH 7/8] first part --- app/models/bus.rb | 3 +- app/models/buses_service.rb | 4 + app/models/service.rb | 3 +- app/services/data_loader.rb | 124 ++++++++++++++++++++++-------- case-study.md | 51 +++++++++++- lib/tasks/profile.rake | 20 ++++- spec/services/data_loader_spec.rb | 31 +++++--- 7 files changed, 191 insertions(+), 45 deletions(-) create mode 100644 app/models/buses_service.rb diff --git a/app/models/bus.rb b/app/models/bus.rb index 1dcc54cb..d97de31f 100644 --- a/app/models/bus.rb +++ b/app/models/bus.rb @@ -13,7 +13,8 @@ class Bus < ApplicationRecord ].freeze has_many :trips - has_and_belongs_to_many :services, join_table: :buses_services + has_many :buses_services + has_many :services, through: :buses_services validates :number, presence: true, uniqueness: true validates :model, inclusion: { in: MODELS } diff --git a/app/models/buses_service.rb b/app/models/buses_service.rb new file mode 100644 index 00000000..6219d44e --- /dev/null +++ b/app/models/buses_service.rb @@ -0,0 +1,4 @@ +class BusesService < ApplicationRecord + belongs_to :bus + belongs_to :service +end diff --git a/app/models/service.rb b/app/models/service.rb index 9cbb2a32..1781543c 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -12,7 +12,8 @@ class Service < ApplicationRecord 'Можно не печатать билет', ].freeze - has_and_belongs_to_many :buses, join_table: :buses_services + has_many :buses_services + has_many :buses, through: :buses_services validates :name, presence: true validates :name, inclusion: { in: SERVICES } diff --git a/app/services/data_loader.rb b/app/services/data_loader.rb index 7846573a..50738775 100644 --- a/app/services/data_loader.rb +++ b/app/services/data_loader.rb @@ -1,41 +1,105 @@ +# frozen_string_literal: true + class DataLoader + TRIPS_COMMAND = + "copy trips (from_id, to_id, start_time, duration_minutes, price_cents, bus_id) from stdin with csv delimiter ';'" + CITIES_COMMAND = "copy cities (id, name) from stdin with csv delimiter ';'" + BUSES_COMMAND = "copy buses (id, number, model) from stdin with csv delimiter ';'" + SERVICES_COMMAND = "copy services (id, name) from stdin with csv delimiter ';'" + BUSES_SERVICES_COMMAND = "copy buses_services (bus_id, service_id) from stdin with csv delimiter ';'" + + def self.load(filename) + new(filename).load + end + + def initialize(filename) + @stream = JsonStreamer.stream(filename) + @cities = {} + @services = {} + @buses = {} + @buses_services = [] + end + + def load + ActiveRecord::Base.transaction do + clean_database + connection = ActiveRecord::Base.connection.raw_connection + + ActiveRecord::Base.connection.raw_connection.copy_data TRIPS_COMMAND do + stream.each do |trip| + from = city(trip['from']) + to = city(trip['to']) + + if buses[trip['bus']['number']].nil? + bus_services = [] + trip['bus']['services'].each do |name| + bus_services << service(name) + end - class << self - def load(filename) - ActiveRecord::Base.transaction do - clean_database - - JsonStreamer.stream(filename).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 + create_bus(trip['bus']['number'], trip['bus']['model']) + create_bus_services(buses[trip['bus']['number']], bus_services) 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'], - ) + + bus = buses[trip['bus']['number']] + + connection.put_copy_data("#{from[:id]};#{to[:id]};#{trip['start_time']};#{trip['duration_minutes']};#{trip['price_cents']};#{bus[:id]}\n") + end + end + + ActiveRecord::Base.connection.raw_connection.copy_data CITIES_COMMAND do + cities.each do |name, attrs| + connection.put_copy_data("#{attrs[:id]};#{name}\n") + end + end + + ActiveRecord::Base.connection.raw_connection.copy_data BUSES_COMMAND do + buses.each do |number, attrs| + connection.put_copy_data("#{attrs[:id]};#{number};#{attrs[:model]}\n") + end + end + + ActiveRecord::Base.connection.raw_connection.copy_data SERVICES_COMMAND do + services.each do |name, attrs| + connection.put_copy_data("#{attrs[:id]};#{name}\n") + end + end + + ActiveRecord::Base.connection.raw_connection.copy_data BUSES_SERVICES_COMMAND do + buses_services.each do |bus_id, service_id| + connection.put_copy_data("#{bus_id};#{service_id}\n") end end end + end + + private - private + attr_reader :stream + attr_accessor :cities, :services, :buses, :buses_services + + def clean_database + City.delete_all + Bus.delete_all + Service.delete_all + Trip.delete_all + ActiveRecord::Base.connection.execute('delete from buses_services;') + end + + def city(name) + cities[name] ||= { id: cities.size + 1 } + end + + def service(name) + services[name] ||= { id: services.size + 1 } + end + + def create_bus(number, model) + buses[number] = { id: buses.size + 1, model: } + end - def clean_database - City.delete_all - Bus.delete_all - Service.delete_all - Trip.delete_all - ActiveRecord::Base.connection.execute('delete from buses_services;') + def create_bus_services(bus, services) + services.map do |service| + @buses_services << [bus[:id], service[:id]] end end end diff --git a/case-study.md b/case-study.md index 9ff5f924..873400d4 100644 --- a/case-study.md +++ b/case-study.md @@ -12,4 +12,53 @@ 1) время выполнения fixtures/small.json - 10.167809 2) воспользуемся rbspy для профилирования скрипта импорта 3) rbspy / rubyprof выдают много лишних данных при вызове таски, поэтому можно вынести импорт в отдельный класс и профилировать его -4) вынесла в класс DataLoader, перенесла тесты \ No newline at end of file +4) вынесла в класс DataLoader, перенесла тесты + +Пока выносила обратила внимание что файл считывается в память целиком, также в задании упоминается про файл 1М который весит примерно 3ГБ, перепишем сразу на стриминг +добавила класс jsonStreamer - собирает объекты и возвращает по одному, какой то адекватный гем не нашла для такой обработки файла, теперь используется потоковая обработка файла + + +1) время выполнения не поменялось +2) время выполнения fixtures/small.json - 10.473295 +3) отчет stackfrof - главная точка роста: +в отчете видны 2 главные точки роста - update (41%) и find_or_create_by (25%) +4) что делать с update пока непонятно, а вот такое кол-во find_or_create_by можно заменить +5) Внесем правки в работу с городами, написано что в файле их не больше 100, попробуем собирать их в словать параллельно создавая +6) время выполнения fixtures/small.json - 8.523640 +7) изменения в отчете профилировщика: find_or_create_by снизился до 18% + +1) также у нас собирается еще 2 стравочника - services, buses +2) заменим find_or_create_by и там +3) services: 6.919434, find_or_create_by 23% +4) buses: 5.319638, find_or_create_by больше нет + +1) главная точка роста: update (64%) +2) обновляется автобус сервисами, это обновление в целом выглядит довольно бесполезно, потому что сервисы останутся от последнего trip в файле +3) поэтому обновление сервисами можно в принципе убрать, либо уточнить формат файла, должны ли они добавляться / обновляться или браться пересечение +4) убрали обновление заменив его на создание сервисов при создании автобуса +5) время: 3.025756 +6) отчет профилировщика: update больше нет + +1) главная точка роста: создание записей +2) обратимся к логам и посмотрим на кол-во обращений к базе +3) на файл example к базе было 10 (создание trip) + 2 (проверка существования автобуса + создание) + 4 (проверка городов + создание) + 2 (создание сревисов) = 18 +4) можно воспользоваться стримингом из readme +5) время для small: 0.138599 +6) medium: 0.510029 +7) large: 3.958625 +8) 1M: 38.381950 +9) ну тут уже время упирается в стример, без каких либо преобразований он перебирает файл 1M за 34.973238 +10) причем потребление памяти на 1М не превышает 8МБ +``` +INITIAL MEMORY USAGE: 103 MB +MEMORY USAGE: 103 MB +MEMORY USAGE: 110 MB +... 7 строчек с 110 MB +MEMORY USAGE: 110 MB +MEMORY USAGE: 111 MB +... 26 строчек с 111 MB +MEMORY USAGE: 111 MB +FINAL MEMORY USAGE: 110 MB +``` + + diff --git a/lib/tasks/profile.rake b/lib/tasks/profile.rake index ae1ff9ff..ff660825 100644 --- a/lib/tasks/profile.rake +++ b/lib/tasks/profile.rake @@ -3,7 +3,7 @@ namespace :profile do task time: :environment do require 'benchmark' - puts(Benchmark.measure { DataLoader.load('fixtures/small.json') }) + puts(Benchmark.measure { DataLoader.load('fixtures/1M.json') }) end desc 'Ruby prof' @@ -39,7 +39,23 @@ namespace :profile do io.close end - DataLoader.load('fixtures/small.json') + DataLoader.load('fixtures/1M.json') monitor_thread.kill end + + desc 'Stackprof cli' + task stackprof_cli: :environment do + StackProf.run(mode: :wall, out: Rails.root.join('profile', 'stackprof_reports/stackprof.dump'), interval: 1000) do + DataLoader.load('fixtures/small.json') + end + end + + desc 'Stackprof speedscope' + task stackprof_speedscope: :environment do + profile = StackProf.run(mode: :wall, raw: true) do + DataLoader.load('fixtures/small.json') + end + + File.write(Rails.root.join('profile', 'stackprof_reports/stackprof.json'), JSON.generate(profile)) + end end diff --git a/spec/services/data_loader_spec.rb b/spec/services/data_loader_spec.rb index 715b79c1..366c7595 100644 --- a/spec/services/data_loader_spec.rb +++ b/spec/services/data_loader_spec.rb @@ -23,22 +23,33 @@ ) end + it 'creates buses_services' do + bus = Bus.find_by(number: '123') + wifi = Service.find_by(name: 'WiFi') + toilet = Service.find_by(name: 'Туалет') + + expect(BusesService.all).to contain_exactly( + have_attributes(bus_id: bus.id, service_id: wifi.id), + have_attributes(bus_id: bus.id, service_id: toilet.id) + ) + end + it 'creates trips' do samara = City.find_by(name: 'Самара') moscow = City.find_by(name: 'Москва') bus = Bus.find_by(number: '123') expect(Trip.all).to contain_exactly( - have_attributes(start_time: '11:00', duration_minutes: 168, price_cents: 474, from: moscow, to: samara, bus:), - have_attributes(start_time: '17:30', duration_minutes: 37, price_cents: 173, from: samara, to: moscow, bus:), - have_attributes(start_time: '12:00', duration_minutes: 323, price_cents: 672, from: moscow, to: samara, bus:), - have_attributes(start_time: '18:30', duration_minutes: 315, price_cents: 969, from: samara, to: moscow, bus:), - have_attributes(start_time: '13:00', duration_minutes: 304, price_cents: 641, from: moscow, to: samara, bus:), - have_attributes(start_time: '19:30', duration_minutes: 21, price_cents: 663, from: samara, to: moscow, bus:), - have_attributes(start_time: '14:00', duration_minutes: 598, price_cents: 629, from: moscow, to: samara, bus:), - have_attributes(start_time: '20:30', duration_minutes: 292, price_cents: 22, from: samara, to: moscow, bus:), - have_attributes(start_time: '15:00', duration_minutes: 127, price_cents: 795, from: moscow, to: samara, bus:), - have_attributes(start_time: '21:30', duration_minutes: 183, price_cents: 846, from: samara, to: moscow, bus:) + have_attributes(start_time: '11:00', duration_minutes: 168, price_cents: 474, from_id: moscow.id, to_id: samara.id, bus_id: bus.id), + have_attributes(start_time: '17:30', duration_minutes: 37, price_cents: 173, from_id: samara.id, to_id: moscow.id, bus_id: bus.id), + have_attributes(start_time: '12:00', duration_minutes: 323, price_cents: 672, from_id: moscow.id, to_id: samara.id, bus_id: bus.id), + have_attributes(start_time: '18:30', duration_minutes: 315, price_cents: 969, from_id: samara.id, to_id: moscow.id, bus_id: bus.id), + have_attributes(start_time: '13:00', duration_minutes: 304, price_cents: 641, from_id: moscow.id, to_id: samara.id, bus_id: bus.id), + have_attributes(start_time: '19:30', duration_minutes: 21, price_cents: 663, from_id: samara.id, to_id: moscow.id, bus_id: bus.id), + have_attributes(start_time: '14:00', duration_minutes: 598, price_cents: 629, from_id: moscow.id, to_id: samara.id, bus_id: bus.id), + have_attributes(start_time: '20:30', duration_minutes: 292, price_cents: 22, from_id: samara.id, to_id: moscow.id, bus_id: bus.id), + have_attributes(start_time: '15:00', duration_minutes: 127, price_cents: 795, from_id: moscow.id, to_id: samara.id, bus_id: bus.id), + have_attributes(start_time: '21:30', duration_minutes: 183, price_cents: 846, from_id: samara.id, to_id: moscow.id, bus_id: bus.id) ) end end From 9c09458f040915f42374d39028fcdbad40e1c15b Mon Sep 17 00:00:00 2001 From: elenachekhina Date: Sat, 22 Feb 2025 16:32:14 +0400 Subject: [PATCH 8/8] second part --- Gemfile | 2 + Gemfile.lock | 16 ++ app/controllers/trips_controller.rb | 18 ++- app/views/shared/_pagination.html.erb | 9 ++ app/views/trips/_service.html.erb | 1 - app/views/trips/_services.html.erb | 6 - app/views/trips/_trip.html.erb | 21 ++- app/views/trips/index.html.erb | 13 +- case-study.md | 137 +++++++++++++++--- config/initializers/strong_migrations.rb | 26 ++++ config/routes.rb | 2 +- .../20250222083704_add_index_to_trips.rb | 7 + db/schema.rb | 18 +-- lib/tasks/profile.rake | 2 +- spec/controllers/trips_controller_spec.rb | 8 +- spec/rails_helper.rb | 1 + spec/services/data_loader_spec.rb | 4 + spec/spec_helper.rb | 2 + 18 files changed, 234 insertions(+), 59 deletions(-) create mode 100644 app/views/shared/_pagination.html.erb delete mode 100644 app/views/trips/_service.html.erb delete mode 100644 app/views/trips/_services.html.erb create mode 100644 config/initializers/strong_migrations.rb create mode 100644 db/migrate/20250222083704_add_index_to_trips.rb diff --git a/Gemfile b/Gemfile index 8e410820..9678a03d 100644 --- a/Gemfile +++ b/Gemfile @@ -15,12 +15,14 @@ gem 'benchmark' gem 'ruby-prof' gem 'stackprof' gem 'oj' +gem 'strong_migrations' group :development, :test do gem 'rspec-rails', '~> 7.1.1' gem 'factory_bot_rails', '~> 6.4.4' gem 'rails-controller-testing' gem 'rspec-rake' + gem 'rspec-benchmark' end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem diff --git a/Gemfile.lock b/Gemfile.lock index 0e786d2e..7880b8f0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -75,6 +75,9 @@ GEM ast (2.4.2) base64 (0.2.0) benchmark (0.4.0) + benchmark-malloc (0.2.0) + benchmark-perf (0.6.0) + benchmark-trend (0.4.0) bigdecimal (3.1.9) bootsnap (1.18.4) msgpack (~> 1.2) @@ -205,6 +208,15 @@ GEM regexp_parser (2.10.0) reline (0.6.0) io-console (~> 0.5) + rspec (3.13.0) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-benchmark (0.6.0) + benchmark-malloc (~> 0.2) + benchmark-perf (~> 0.6) + benchmark-trend (~> 0.4) + rspec (>= 3.0) rspec-core (3.13.3) rspec-support (~> 3.13.0) rspec-expectations (3.13.3) @@ -243,6 +255,8 @@ GEM securerandom (0.4.1) stackprof (0.2.27) stringio (3.1.2) + strong_migrations (2.2.0) + activerecord (>= 7) thor (1.3.2) timeout (0.4.3) tzinfo (2.0.6) @@ -274,11 +288,13 @@ DEPENDENCIES rack-mini-profiler rails (~> 8.0.1) rails-controller-testing + rspec-benchmark rspec-rails (~> 7.1.1) rspec-rake rubocop ruby-prof stackprof + strong_migrations tzinfo-data RUBY VERSION diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb index acb38be2..c28d7dff 100644 --- a/app/controllers/trips_controller.rb +++ b/app/controllers/trips_controller.rb @@ -1,7 +1,23 @@ class TripsController < ApplicationController + before_action :set_pagination_params, :set_cities, only: :index + def index + @total_count = Trip.where(from: @from, to: @to).count + @trips = Trip.where(from: @from, to: @to) + .order(:start_time) + .limit(@per).offset(@per * (@page - 1)) + .preload(bus: [:services]) + end + + private + + def set_pagination_params + @page = params[:page].present? ? params[:page].to_i : 1 + @per = params[:per].present? ? params[:per].to_i : 100 + end + + def set_cities @from = City.find_by_name!(params[:from]) @to = City.find_by_name!(params[:to]) - @trips = Trip.where(from: @from, to: @to).order(:start_time) end end diff --git a/app/views/shared/_pagination.html.erb b/app/views/shared/_pagination.html.erb new file mode 100644 index 00000000..317a7ca1 --- /dev/null +++ b/app/views/shared/_pagination.html.erb @@ -0,0 +1,9 @@ +<% if @total_count > @per %> + <% if @page < (@total_count / @per) %> + <%= link_to 'Следующая страница', trips_path(@from.name, @to.name, page: @page + 1) %> + <% end %> + + <% if @page > 1 %> + <%= link_to 'Предыдущая страница', trips_path(@from.name, @to.name, page: @page - 1) %> + <% end %> +<% end %> 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 index fa1de9aa..3714e808 100644 --- a/app/views/trips/_trip.html.erb +++ b/app/views/trips/_trip.html.erb @@ -1,5 +1,16 @@ -
  • <%= "Отправление: #{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? %> +
    • Сервисы в автобусе:
      • +
        + <% trip.bus.services.each do |service| %> +
      • <%= "#{service.name}" %>
        • + <% end %> +
      + <% end %> +
    + diff --git a/app/views/trips/index.html.erb b/app/views/trips/index.html.erb index a60bce41..adcd0a5c 100644 --- a/app/views/trips/index.html.erb +++ b/app/views/trips/index.html.erb @@ -2,15 +2,8 @@ <%= "Автобусы #{@from.name} – #{@to.name}" %>

    - <%= "В расписании #{@trips.count} рейсов" %> + <%= "В расписании #{@total_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 'shared/pagination' %> +<%= render partial: 'trip', collection: @trips, spacer_template: 'delimiter' %> diff --git a/case-study.md b/case-study.md index 873400d4..bd98fcab 100644 --- a/case-study.md +++ b/case-study.md @@ -2,53 +2,67 @@ ## Актуальная проблема -1) Добавила в проект тесты -2) Начем с оптимизации импорта данных +В нашем проекте возникли 2 серьёзных проблемы: +1) Необходимо было обработать файл с расписанием автобусов. У нас уже была программа на ruby, которая умела делать нужную обработку. Она успешно работала на файлах небольшого размера, но для большого файла она работала слишком долго. +2) Страница отображения расписания автобусов тоже работала слишком долго. Пользователи жаловались на долгую загрузку страницы (расписание Таганрог/Владивосток загружалось 42 секунды (~2к записей)). + +Я решила исправить обе проблемы, оптимизировав загрузку и отображение. ## Оптимизация импорта данных -напишем раннер для запуска / мониторинга времени / памяти выполнения скрипта импорта +## Формирование метрики + +Конечная метрика: время выполнения импорта файла `fixtures/large.json` должно укладываться в 60 сек. +Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумала использовать такую метрику: +1) обработка файла после внесенных оптимизаций должна быть меньше, чем до оптимизации + +## Предварительная подготовка + +1) Добавила в проект тесты +2) Написала раннер для профилирования / мониторинга времени / памяти выполнения скрипта импорта +## Ваша находка №1 1) время выполнения fixtures/small.json - 10.167809 2) воспользуемся rbspy для профилирования скрипта импорта 3) rbspy / rubyprof выдают много лишних данных при вызове таски, поэтому можно вынести импорт в отдельный класс и профилировать его 4) вынесла в класс DataLoader, перенесла тесты Пока выносила обратила внимание что файл считывается в память целиком, также в задании упоминается про файл 1М который весит примерно 3ГБ, перепишем сразу на стриминг -добавила класс jsonStreamer - собирает объекты и возвращает по одному, какой то адекватный гем не нашла для такой обработки файла, теперь используется потоковая обработка файла - +добавила класс JsonStreamer - собирает объекты и возвращает по одному. Гемы потоковой обработки выглядят сложновато для достаточно простой задачи, возможно они работают быстрее, но этим можно будет озаботиться попозже -1) время выполнения не поменялось -2) время выполнения fixtures/small.json - 10.473295 -3) отчет stackfrof - главная точка роста: +## Ваша находка №2 +1) время выполнения fixtures/small.json не поменялось - 10.473295 +2) отчет stackfrof - главная точка роста: в отчете видны 2 главные точки роста - update (41%) и find_or_create_by (25%) -4) что делать с update пока непонятно, а вот такое кол-во find_or_create_by можно заменить -5) Внесем правки в работу с городами, написано что в файле их не больше 100, попробуем собирать их в словать параллельно создавая -6) время выполнения fixtures/small.json - 8.523640 -7) изменения в отчете профилировщика: find_or_create_by снизился до 18% +3) что делать с update пока непонятно, а вот такое кол-во find_or_create_by можно заменить +4) Внесем правки в работу с городами, написано что в файле их не больше 100, попробуем собирать их в словарь, параллельно создавая +5) время выполнения fixtures/small.json - 8.523640 +6) изменения в отчете профилировщика: find_or_create_by снизился до 18% +## Ваша находка №3 1) также у нас собирается еще 2 стравочника - services, buses 2) заменим find_or_create_by и там 3) services: 6.919434, find_or_create_by 23% 4) buses: 5.319638, find_or_create_by больше нет -1) главная точка роста: update (64%) +## Ваша находка №4 +1) отчет stackfrof - главная точка роста: update (64%) 2) обновляется автобус сервисами, это обновление в целом выглядит довольно бесполезно, потому что сервисы останутся от последнего trip в файле 3) поэтому обновление сервисами можно в принципе убрать, либо уточнить формат файла, должны ли они добавляться / обновляться или браться пересечение -4) убрали обновление заменив его на создание сервисов при создании автобуса +4) убрали обновление заменив его на создание сервисов при создании автобуса (добавила модель buses_service) 5) время: 3.025756 6) отчет профилировщика: update больше нет -1) главная точка роста: создание записей -2) обратимся к логам и посмотрим на кол-во обращений к базе -3) на файл example к базе было 10 (создание trip) + 2 (проверка существования автобуса + создание) + 4 (проверка городов + создание) + 2 (создание сревисов) = 18 -4) можно воспользоваться стримингом из readme -5) время для small: 0.138599 -6) medium: 0.510029 -7) large: 3.958625 -8) 1M: 38.381950 -9) ну тут уже время упирается в стример, без каких либо преобразований он перебирает файл 1M за 34.973238 -10) причем потребление памяти на 1М не превышает 8МБ +## Ваша находка №5 +1) обратимся к логам и посмотрим на кол-во обращений к базе +2) на файл example (10 рейсов) к базе обращений было: 10 (создание trip) + 2 (проверка существования автобуса + создание) + 4 (проверка городов + создание) + 2 (создание сревисов) = 18 +3) можно воспользоваться алгоритмом и стримингом из readme +4) время для small: 0.138599 +5) medium: 0.510029 +6) large: 3.958625 +7) 1M: 38.381950 +8) ну тут уже время упирается в стример, без каких либо преобразований он перебирает файл 1M за 34.973238 +9) причем потребление памяти на 1М не превышает 8МБ ``` INITIAL MEMORY USAGE: 103 MB MEMORY USAGE: 103 MB @@ -61,4 +75,79 @@ MEMORY USAGE: 111 MB FINAL MEMORY USAGE: 110 MB ``` +## Оптимизация отображения расписания + +## Формирование метрики + +Конечная метрика: любая страница должна грузиться менее 0.3 секунды +Для того, чтобы понимать, дают ли мои изменения положительный эффект на быстродействие программы я придумала использовать такую метрику: +1) время рендеринга страницы после внесенных оптимизаций должно быть меньше, чем до оптимизации + +## Оптимизация + +Набор данных 1M. Для начала загрузим страницу http://localhost:3000/автобусы/Самара/Москва и посмотрим на логи + +## Ваша находка №1 +1) Completed 200 OK in 745ms (Views: 228.0ms | ActiveRecord: 487.9ms (120 queries, 0 cached) | GC: 33.0ms) +2) Выборка занимает бОльшую часть времени, начнем с нее. Также по логам видно, что для каждого trip делается отдельный запрос к базе чтобы достать автобус, а для него сервисы +3) Попробуем воспользоваться preload +4) Completed 200 OK in 157ms (Views: 55.8ms | ActiveRecord: 97.3ms (7 queries, 0 cached) | GC: 6.0ms) +5) Время загрузки уменьшилось и кол-во запросов тоже +6) Рендер тоже ускорился, потому что все вызовы были непосредствено из вьюхи + +## Ваша находка №2 +1) Оптимизируем рендеринг (огромное кол-во рендеров) +2) Сделаем рендер коллекции сервисов +3) Completed 200 OK in 157ms (Views: 52.2ms | ActiveRecord: 92.7ms (7 queries, 0 cached) | GC: 4.5ms) + +## Ваша находка №3 +1) Оптимизируем рендеринг дальше, сделаем рендер коллекции рейсов +2) Completed 200 OK in 153ms (Views: 56.9ms | ActiveRecord: 92.2ms (7 queries, 0 cached) | GC: 1.0ms) +3) В пределах погрешности ничего не поменялось, но все еще отдельно рендерятся delimeter / services + +## Ваша находка №4 +1) Посмотрим на rack-mini-profiler +2) Больше всего времени уходит на запрос SELECT COUNT(*) FROM "trips" WHERE "trips"."from_id" = $1 AND "trips"."to_id" = $2; (126 ms) +3) Посмотрим на план запроса: + Gather (cost=1000.00..24758.70 rows=87 width=34) + Workers Planned: 2 + -> Parallel Seq Scan on trips (cost=0.00..23750.00 rows=36 width=34) + Filter: ((from_id = 10) AND (to_id = 92)) + (4 rows) +4) Избавимся от Seq Scan добавив составной индекс. Так как поиск идет всегда по (from_id, to_id). Также сделаем сразу сортированный индекс по времени +5) План запроса после добавления индекса: + Bitmap Heap Scan on trips (cost=13.75..3377.62 rows=909 width=34) + Recheck Cond: ((from_id = 10) AND (to_id = 92)) + -> Bitmap Index Scan on index_trips_on_from_id_and_to_id (cost=0.00..13.53 rows=909 width=0) + Index Cond: ((from_id = 10) AND (to_id = 92)) + (4 rows) +6) Теперь запрос не является главной точкой роста (2.1 ms) + +## Ваша находка №5 +1) Зальем 10М (~6 минут) и проверим на более объемных данных +2) Расписание Самара/Москва (511 рейсов): Completed 200 OK in 342ms (Views: 321.6ms | ActiveRecord: 15.2ms (7 queries, 0 cached) | GC: 41.2ms) +3) Главной точкой роста опять стал рендеринг, попробуем оптимизировать его еще лучше +4) Попробуем убрать отдельный рендер разделителя и сервисов +5) Воспользовалась spacer_template: Completed 200 OK in 276ms (Views: 217.5ms | ActiveRecord: 51.2ms (7 queries, 0 cached) | GC: 11.7ms) +6) Уберем рендер сервисов в partial trip. некрасиво, но это ускорит рендер +7) Completed 200 OK in 104ms (Views: 85.7ms | ActiveRecord: 14.4ms (7 queries, 0 cached) | GC: 5.2ms) + +## Ваша находка №6 +1) Проверим на большем кол-ве +2) самое большое кол-во рейсов - 4191 (Таганрог - Таганрог) +3) Completed 200 OK in 620ms (Views: 499.6ms | ActiveRecord: 127.1ms (7 queries, 1 cached) | GC: 55.3ms) +4) Время почти в 2 раза больше желательного +5) Посмотрим на это со стороны того, что за раз пользователю не надо видеть все 4к записей, можно добавить пагинацию +6) добавила пагинацию с дефолтным значением 100 записей (гем использовать не стала, потому что в текущем варианте это не сложно сделать) +7) одна страница: Completed 200 OK in 39ms (Views: 23.5ms | ActiveRecord: 10.0ms (7 queries, 0 cached) | GC: 0.0ms) + +Можно закончить оптимизацию на этом, так как время выполнения укладывается в приемлемые рамки + +Для красоты конечно лучше бы добавить турбо фреймы, чтобы полностью не перезагружать страницу при пагинации, но это уже не входят в текущую задачу + +## Результаты +В результате проделанной оптимизации удалось ускорить импорт файла `fixtures/large.json` до 4 секунд и уложиться в метрику. Также удалось ускорить загрузку страницы Таганрог/Владивосток до ~300ms без пагинации, или ~50ms с пагинацией по 100 рейсов на странице +## Защита от регрессии производительности +1) для импорта данных был написан тест на проверку времени выполнения +2) для отображения расписания был бы написан тест на N+1 запрос, если бы rspec-sqlimit был совместим с rails 8.0.1 diff --git a/config/initializers/strong_migrations.rb b/config/initializers/strong_migrations.rb new file mode 100644 index 00000000..625a310a --- /dev/null +++ b/config/initializers/strong_migrations.rb @@ -0,0 +1,26 @@ +# Mark existing migrations as safe +StrongMigrations.start_after = 20250222090128 + +# Set timeouts for migrations +# If you use PgBouncer in transaction mode, delete these lines and set timeouts on the database user +StrongMigrations.lock_timeout = 10.seconds +StrongMigrations.statement_timeout = 1.hour + +# Analyze tables after indexes are added +# Outdated statistics can sometimes hurt performance +StrongMigrations.auto_analyze = true + +# Set the version of the production database +# so the right checks are run in development +# StrongMigrations.target_version = 10 + +# Add custom checks +# StrongMigrations.add_check do |method, args| +# if method == :add_index && args[0].to_s == "users" +# stop! "No more indexes on the users table" +# end +# end + +# Make some operations safe by default +# See https://github.com/ankane/strong_migrations#safe-by-default +# StrongMigrations.safe_by_default = true diff --git a/config/routes.rb b/config/routes.rb index 0bbefa7a..53f04ac9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,4 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html - get "автобусы/:from/:to" => "trips#index" + get "автобусы/:from/:to" => "trips#index", as: :trips end diff --git a/db/migrate/20250222083704_add_index_to_trips.rb b/db/migrate/20250222083704_add_index_to_trips.rb new file mode 100644 index 00000000..57ba3d72 --- /dev/null +++ b/db/migrate/20250222083704_add_index_to_trips.rb @@ -0,0 +1,7 @@ +class AddIndexToTrips < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + add_index :trips, %i[from_id to_id], algorithm: :concurrently, order: { start_time: :asc } + end +end diff --git a/db/schema.rb b/db/schema.rb index f6921e45..3db5ba45 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2,18 +2,18 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `bin/rails +# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # 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[8.0].define(version: 2025_02_22_083704) do # These are extensions that must be enabled in order to support this database - enable_extension "plpgsql" + enable_extension "pg_catalog.plpgsql" + enable_extension "pg_stat_statements" create_table "buses", force: :cascade do |t| t.string "number" @@ -40,6 +40,6 @@ t.integer "duration_minutes" t.integer "price_cents" t.integer "bus_id" + t.index ["from_id", "to_id"], name: "index_trips_on_from_id_and_to_id" end - end diff --git a/lib/tasks/profile.rake b/lib/tasks/profile.rake index ff660825..ddee7dd3 100644 --- a/lib/tasks/profile.rake +++ b/lib/tasks/profile.rake @@ -3,7 +3,7 @@ namespace :profile do task time: :environment do require 'benchmark' - puts(Benchmark.measure { DataLoader.load('fixtures/1M.json') }) + puts(Benchmark.measure { DataLoader.load('fixtures/10M.json') }) end desc 'Ruby prof' diff --git a/spec/controllers/trips_controller_spec.rb b/spec/controllers/trips_controller_spec.rb index c5a0137c..83cd1c28 100644 --- a/spec/controllers/trips_controller_spec.rb +++ b/spec/controllers/trips_controller_spec.rb @@ -7,7 +7,7 @@ let!(:trip) { create(:trip, from: city_from, to: city_to, start_time: 1.day.from_now) } let!(:other_trip) { create(:trip, from: create(:city), to: create(:city)) } - context 'with valid city names' do + describe 'response' do before do get :index, params: { from: 'Samara', to: 'Moscow' } end @@ -20,5 +20,11 @@ expect(response).to render_template(:index) end end + + # describe 'limit database queries' do + # it 'performs under 7 queries' do + # expect { get :index, params: { from: 'Samara', to: 'Moscow' } }.not_to exceed_query_limit(7) + # end + # end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 0cb7e4fa..4e996013 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -35,6 +35,7 @@ # config.fixture_path = ["#{::Rails.root}/spec/fixtures"] config.include FactoryBot::Syntax::Methods + config.include RSpec::Benchmark::Matchers # If you're not using ActiveRecord, or you'd prefer not to run each of your # examples within a transaction, remove the following line or assign false diff --git a/spec/services/data_loader_spec.rb b/spec/services/data_loader_spec.rb index 366c7595..ee7e40f3 100644 --- a/spec/services/data_loader_spec.rb +++ b/spec/services/data_loader_spec.rb @@ -52,4 +52,8 @@ have_attributes(start_time: '21:30', duration_minutes: 183, price_cents: 846, from_id: samara.id, to_id: moscow.id, bus_id: bus.id) ) end + + it 'performs example under 0.05s' do + expect { load }.to perform_under(50).ms.warmup(2).times.sample(5).times + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e4cd252f..2e5efc42 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,6 +14,8 @@ # # See https://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration require 'rspec/rake' +require 'rspec/benchmark' +require 'rspec/sqlimit' RSpec.configure do |config| # rspec-expectations config goes here. You can use an alternate