From 400db5803633c92cf5ac0adf1ec7de32211aedea Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 23 Jan 2019 18:12:30 +0100 Subject: [PATCH 1/3] Use activerecord-import for bulk importing tracepoint records Non-rigourous testing shows a significant speedup, even on ssds. --- Gemfile | 1 + Gemfile.lock | 3 +++ app/models/trace.rb | 11 ++++++++++- test/models/trace_test.rb | 4 ++++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 372b7d021..83f298101 100644 --- a/Gemfile +++ b/Gemfile @@ -46,6 +46,7 @@ gem "image_optim_rails" # Load rails plugins gem "actionpack-page_caching" gem "active_record_union" +gem "activerecord-import" gem "cancancan" gem "composite_primary_keys", "~> 11.1.0" gem "config" diff --git a/Gemfile.lock b/Gemfile.lock index 2b5cc1dd5..57feba1fd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -40,6 +40,8 @@ GEM activemodel (= 5.2.2.1) activesupport (= 5.2.2.1) arel (>= 9.0) + activerecord-import (0.28.1) + activerecord (>= 3.2) activestorage (5.2.2.1) actionpack (= 5.2.2.1) activerecord (= 5.2.2.1) @@ -423,6 +425,7 @@ DEPENDENCIES aasm actionpack-page_caching active_record_union + activerecord-import annotate autoprefixer-rails (~> 8.6.3) better_errors diff --git a/app/models/trace.rb b/app/models/trace.rb index 892d41a2a..bd8ab72b3 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -289,6 +289,8 @@ class Trace < ActiveRecord::Base # If there are any existing points for this trace then delete them Tracepoint.where(:gpx_id => id).delete_all + # Gather the trace points together for a bulk import + tracepoints = [] gpx.points do |point| if first f_lat = point.latitude @@ -303,9 +305,16 @@ class Trace < ActiveRecord::Base tp.timestamp = point.timestamp tp.gpx_id = id tp.trackid = point.segment - tp.save! + tracepoints << tp end + # Run the before_save and before_create callbacks, and then import them in bulk with activerecord-import + tracepoints.each do |tp| + tp.run_callbacks(:save) { false } + tp.run_callbacks(:create) { false } + end + Tracepoint.import(tracepoints) + if gpx.actual_points.positive? max_lat = Tracepoint.where(:gpx_id => id).maximum(:latitude) min_lat = Tracepoint.where(:gpx_id => id).minimum(:latitude) diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb index f03488a61..81120f0e7 100644 --- a/test/models/trace_test.rb +++ b/test/models/trace_test.rb @@ -206,6 +206,10 @@ class TraceTest < ActiveSupport::TestCase trace.reload assert_equal 1, Tracepoint.where(:gpx_id => trace.id).count + + # Check that the tile has been set prior to the bulk import + # i.e. that the callbacks have been run correctly + assert_equal 3221331576, Tracepoint.where(:gpx_id => trace.id).first.tile end end From 07fdcf638ea8f7200b561f4cc190f231929cda0b Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 6 Mar 2019 09:27:33 +0100 Subject: [PATCH 2/3] Raise exception if there is an error, and import tracepoints in batches --- app/models/trace.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/trace.rb b/app/models/trace.rb index bd8ab72b3..3d23d02c8 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -313,7 +313,7 @@ class Trace < ActiveRecord::Base tp.run_callbacks(:save) { false } tp.run_callbacks(:create) { false } end - Tracepoint.import(tracepoints) + Tracepoint.import!(tracepoints, :batch_size => 1_000) if gpx.actual_points.positive? max_lat = Tracepoint.where(:gpx_id => id).maximum(:latitude) From c1bf73bee42000aadb56cdaeab650e26a284b0b2 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 20 Mar 2019 10:14:11 +0100 Subject: [PATCH 3/3] Use an enumerator for gpx.points, and process the the points in batches --- app/models/trace.rb | 46 ++++++++++++++++++++++++--------------------- lib/gpx.rb | 2 ++ 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/app/models/trace.rb b/app/models/trace.rb index 3d23d02c8..0e8763328 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -289,31 +289,35 @@ class Trace < ActiveRecord::Base # If there are any existing points for this trace then delete them Tracepoint.where(:gpx_id => id).delete_all - # Gather the trace points together for a bulk import - tracepoints = [] - gpx.points do |point| - if first - f_lat = point.latitude - f_lon = point.longitude - first = false + gpx.points.each_slice(1_000) do |points| + # Gather the trace points together for a bulk import + tracepoints = [] + + points.each do |point| + if first + f_lat = point.latitude + f_lon = point.longitude + first = false + end + + tp = Tracepoint.new + tp.lat = point.latitude + tp.lon = point.longitude + tp.altitude = point.altitude + tp.timestamp = point.timestamp + tp.gpx_id = id + tp.trackid = point.segment + tracepoints << tp end - tp = Tracepoint.new - tp.lat = point.latitude - tp.lon = point.longitude - tp.altitude = point.altitude - tp.timestamp = point.timestamp - tp.gpx_id = id - tp.trackid = point.segment - tracepoints << tp - end + # Run the before_save and before_create callbacks, and then import them in bulk with activerecord-import + tracepoints.each do |tp| + tp.run_callbacks(:save) { false } + tp.run_callbacks(:create) { false } + end - # Run the before_save and before_create callbacks, and then import them in bulk with activerecord-import - tracepoints.each do |tp| - tp.run_callbacks(:save) { false } - tp.run_callbacks(:create) { false } + Tracepoint.import!(tracepoints) end - Tracepoint.import!(tracepoints, :batch_size => 1_000) if gpx.actual_points.positive? max_lat = Tracepoint.where(:gpx_id => id).maximum(:latitude) diff --git a/lib/gpx.rb b/lib/gpx.rb index 5664e0099..721e5608b 100644 --- a/lib/gpx.rb +++ b/lib/gpx.rb @@ -13,6 +13,8 @@ module GPX end def points + return enum_for(:points) unless block_given? + @possible_points = 0 @actual_points = 0 @tracksegs = 0