From 0c904a6b84b850b2c7a94938f813155569c6f041 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 22 Jul 2025 22:41:12 +0200 Subject: [PATCH 1/6] Fix stats calculation performance --- .app_version | 2 +- CHANGELOG.md | 7 ++ app/controllers/stats_controller.rb | 1 + app/models/stat.rb | 7 +- app/queries/stats/daily_distance_query.rb | 64 +++++++++++++++++++ app/services/stats/bulk_calculator.rb | 2 +- app/services/stats/calculate_month.rb | 10 ++- app/views/stats/_stat.html.erb | 2 +- app/views/stats/index.html.erb | 2 +- ...4404_add_index_on_places_geodata_osm_id.rb | 4 +- 10 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 app/queries/stats/daily_distance_query.rb diff --git a/.app_version b/.app_version index 1a44cad7..0f721773 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.30.1 +0.30.2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ab8cb0..e7c5a882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +# [0.30.2] - 2025-07-22 + +## Fixed + +- Stats calculation is now significantly faster. + + # [0.30.1] - 2025-07-22 ## Fixed diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 0300deae..ef5ff993 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -13,6 +13,7 @@ class StatsController < ApplicationController def show @year = params[:year].to_i @stats = current_user.stats.where(year: @year).order(:month) + @year_distances = { @year => Stat.year_distance(@year, current_user) } end def update diff --git a/app/models/stat.rb b/app/models/stat.rb index 0fa4e5e5..e291a71a 100644 --- a/app/models/stat.rb +++ b/app/models/stat.rb @@ -37,12 +37,7 @@ class Stat < ApplicationRecord end def calculate_daily_distances(monthly_points) - timespan.to_a.map.with_index(1) do |day, index| - daily_points = filter_points_for_day(monthly_points, day) - # Calculate distance in meters for consistent storage - distance_meters = Point.total_distance(daily_points, :m) - [index, distance_meters.round] - end + Stats::DailyDistanceQuery.new(monthly_points, timespan).call end def filter_points_for_day(points, day) diff --git a/app/queries/stats/daily_distance_query.rb b/app/queries/stats/daily_distance_query.rb new file mode 100644 index 00000000..4057fe9e --- /dev/null +++ b/app/queries/stats/daily_distance_query.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +class Stats::DailyDistanceQuery + def initialize(monthly_points, timespan) + @monthly_points = monthly_points + @timespan = timespan + end + + def call + daily_distances = daily_distances(monthly_points) + distance_by_day_map = distance_by_day_map(daily_distances) + + convert_to_daily_distances(distance_by_day_map) + end + + private + + attr_reader :monthly_points, :timespan + + def daily_distances(monthly_points) + Stat.connection.select_all(<<-SQL.squish) + WITH points_with_distances AS ( + SELECT + EXTRACT(day FROM to_timestamp(timestamp)) as day_of_month, + CASE + WHEN LAG(lonlat) OVER ( + PARTITION BY EXTRACT(day FROM to_timestamp(timestamp)) + ORDER BY timestamp + ) IS NOT NULL THEN + ST_Distance( + lonlat::geography, + LAG(lonlat) OVER ( + PARTITION BY EXTRACT(day FROM to_timestamp(timestamp)) + ORDER BY timestamp + )::geography + ) + ELSE 0 + END as segment_distance + FROM (#{monthly_points.to_sql}) as points + ) + SELECT + day_of_month, + ROUND(COALESCE(SUM(segment_distance), 0)) as distance_meters + FROM points_with_distances + GROUP BY day_of_month + ORDER BY day_of_month + SQL + end + + def distance_by_day_map(daily_distances) + daily_distances.index_by do |row| + row['day_of_month'].to_i + end + end + + def convert_to_daily_distances(distance_by_day_map) + timespan.to_a.map.with_index(1) do |day, index| + distance_meters = + distance_by_day_map[day.day]&.fetch('distance_meters', 0) || 0 + + [index, distance_meters.to_i] + end + end +end diff --git a/app/services/stats/bulk_calculator.rb b/app/services/stats/bulk_calculator.rb index aa74d60c..b803d3b1 100644 --- a/app/services/stats/bulk_calculator.rb +++ b/app/services/stats/bulk_calculator.rb @@ -21,7 +21,7 @@ module Stats last_calculated_at ||= DateTime.new(1970, 1, 1) time_diff = last_calculated_at.to_i..Time.current.to_i - Point.where(user_id:, timestamp: time_diff).pluck(:timestamp) + Point.where(user_id:, timestamp: time_diff).pluck(:timestamp).uniq end def extract_months(timestamps) diff --git a/app/services/stats/calculate_month.rb b/app/services/stats/calculate_month.rb index e9d6d64d..e996ac4f 100644 --- a/app/services/stats/calculate_month.rb +++ b/app/services/stats/calculate_month.rb @@ -50,7 +50,7 @@ class Stats::CalculateMonth .tracked_points .without_raw_data .where(timestamp: start_timestamp..end_timestamp) - .select(:lonlat, :timestamp, :city, :country) + .select(:lonlat, :timestamp) .order(timestamp: :asc) end @@ -59,7 +59,13 @@ class Stats::CalculateMonth end def toponyms - CountriesAndCities.new(points).call + toponym_points = user + .tracked_points + .without_raw_data + .where(timestamp: start_timestamp..end_timestamp) + .select(:city, :country) + .distinct + CountriesAndCities.new(toponym_points).call end def create_stats_update_failed_notification(user, error) diff --git a/app/views/stats/_stat.html.erb b/app/views/stats/_stat.html.erb index 470d3438..14430331 100644 --- a/app/views/stats/_stat.html.erb +++ b/app/views/stats/_stat.html.erb @@ -20,7 +20,7 @@ <%= area_chart( stat.daily_distance.map { |day, distance_meters| - [day, Stat.convert_distance(distance_meters, current_user.safe_settings.distance_unit).round(2)] + [day, Stat.convert_distance(distance_meters, current_user.safe_settings.distance_unit).round] }, height: '200px', suffix: " #{current_user.safe_settings.distance_unit}", diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb index 2f14fa36..a5ab367b 100644 --- a/app/views/stats/index.html.erb +++ b/app/views/stats/index.html.erb @@ -83,7 +83,7 @@ <% end %> <%= column_chart( @year_distances[year].map { |month_name, distance_meters| - [month_name, Stat.convert_distance(distance_meters, current_user.safe_settings.distance_unit).round(2)] + [month_name, Stat.convert_distance(distance_meters, current_user.safe_settings.distance_unit).round] }, height: '200px', suffix: " #{current_user.safe_settings.distance_unit}", diff --git a/db/migrate/20250721204404_add_index_on_places_geodata_osm_id.rb b/db/migrate/20250721204404_add_index_on_places_geodata_osm_id.rb index 83359ec4..50f8ab97 100644 --- a/db/migrate/20250721204404_add_index_on_places_geodata_osm_id.rb +++ b/db/migrate/20250721204404_add_index_on_places_geodata_osm_id.rb @@ -2,8 +2,8 @@ class AddIndexOnPlacesGeodataOsmId < ActiveRecord::Migration[8.0] disable_ddl_transaction! def change - add_index :places, "(geodata->'properties'->>'osm_id')", - using: :btree, + add_index :places, "(geodata->'properties'->>'osm_id')", + using: :btree, name: 'index_places_on_geodata_osm_id', algorithm: :concurrently end From 9803ccc6a8df41f53be32a0b2be9a7ea27fc2bb0 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 22 Jul 2025 22:44:41 +0200 Subject: [PATCH 2/6] Remove unused method --- app/models/stat.rb | 7 ------- app/services/stats/calculate_month.rb | 1 + 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/app/models/stat.rb b/app/models/stat.rb index e291a71a..f11e29d2 100644 --- a/app/models/stat.rb +++ b/app/models/stat.rb @@ -39,11 +39,4 @@ class Stat < ApplicationRecord def calculate_daily_distances(monthly_points) Stats::DailyDistanceQuery.new(monthly_points, timespan).call end - - def filter_points_for_day(points, day) - beginning_of_day = day.beginning_of_day.to_i - end_of_day = day.end_of_day.to_i - - points.select { |p| p.timestamp.between?(beginning_of_day, end_of_day) } - end end diff --git a/app/services/stats/calculate_month.rb b/app/services/stats/calculate_month.rb index e996ac4f..839ea55a 100644 --- a/app/services/stats/calculate_month.rb +++ b/app/services/stats/calculate_month.rb @@ -65,6 +65,7 @@ class Stats::CalculateMonth .where(timestamp: start_timestamp..end_timestamp) .select(:city, :country) .distinct + CountriesAndCities.new(toponym_points).call end From bdcfb5eb62a18913206d0f754f5fa563ea5d5dc0 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 22 Jul 2025 23:57:25 +0200 Subject: [PATCH 3/6] Stats calculation is now timezone-aware. --- app/models/stat.rb | 12 +++++++- app/queries/stats/daily_distance_query.rb | 11 +++---- benchmark_stats.rb | 30 +++++++++++++++++++ .../future_add_timezone_to_users.rb.example | 16 ++++++++++ 4 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 benchmark_stats.rb create mode 100644 db/migrate/future_add_timezone_to_users.rb.example diff --git a/app/models/stat.rb b/app/models/stat.rb index f11e29d2..2cf26d04 100644 --- a/app/models/stat.rb +++ b/app/models/stat.rb @@ -37,6 +37,16 @@ class Stat < ApplicationRecord end def calculate_daily_distances(monthly_points) - Stats::DailyDistanceQuery.new(monthly_points, timespan).call + Stats::DailyDistanceQuery.new(monthly_points, timespan, user_timezone).call + end + + private + + def user_timezone + # Future: Once user.timezone column exists, uncomment the line below + # user.timezone.presence || Time.zone.name + + # For now, use application timezone + Time.zone.name end end diff --git a/app/queries/stats/daily_distance_query.rb b/app/queries/stats/daily_distance_query.rb index 4057fe9e..1673f1fa 100644 --- a/app/queries/stats/daily_distance_query.rb +++ b/app/queries/stats/daily_distance_query.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Stats::DailyDistanceQuery - def initialize(monthly_points, timespan) + def initialize(monthly_points, timespan, user_timezone = nil) @monthly_points = monthly_points @timespan = timespan + @user_timezone = user_timezone || 'UTC' end def call @@ -15,22 +16,22 @@ class Stats::DailyDistanceQuery private - attr_reader :monthly_points, :timespan + attr_reader :monthly_points, :timespan, :user_timezone def daily_distances(monthly_points) Stat.connection.select_all(<<-SQL.squish) WITH points_with_distances AS ( SELECT - EXTRACT(day FROM to_timestamp(timestamp)) as day_of_month, + EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{user_timezone}')) as day_of_month, CASE WHEN LAG(lonlat) OVER ( - PARTITION BY EXTRACT(day FROM to_timestamp(timestamp)) + PARTITION BY EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{user_timezone}')) ORDER BY timestamp ) IS NOT NULL THEN ST_Distance( lonlat::geography, LAG(lonlat) OVER ( - PARTITION BY EXTRACT(day FROM to_timestamp(timestamp)) + PARTITION BY EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{user_timezone}')) ORDER BY timestamp )::geography ) diff --git a/benchmark_stats.rb b/benchmark_stats.rb new file mode 100644 index 00000000..94049f14 --- /dev/null +++ b/benchmark_stats.rb @@ -0,0 +1,30 @@ +require 'benchmark' + +# Test the optimized stats calculation +data = Benchmark.measure do + user_id = 7 + + last_calculated_at = DateTime.new(1970, 1, 1) + + time_diff = last_calculated_at.to_i..Time.current.to_i + timestamps = Point.where(user_id:, timestamp: time_diff).pluck(:timestamp).uniq + + months = timestamps.group_by do |timestamp| + time = Time.zone.at(timestamp) + [time.year, time.month] + end.keys + + months.each do |year, month| + Stats::CalculateMonth.new(user_id, year, month).call + end +end + +puts "Stats calculation benchmark:" +puts "User Time: #{data.utime}s" +puts "System Time: #{data.stime}s" +puts "Total Time: #{data.real}s" + +# @real=28.869485000148416, +# @stime=2.4980050000000027, +# @total=20.303141999999976, +# @utime=17.805136999999974> diff --git a/db/migrate/future_add_timezone_to_users.rb.example b/db/migrate/future_add_timezone_to_users.rb.example new file mode 100644 index 00000000..caf60a88 --- /dev/null +++ b/db/migrate/future_add_timezone_to_users.rb.example @@ -0,0 +1,16 @@ +# Example migration for adding per-user timezone support +# To use: rename this file to remove .example and run rails db:migrate + +class AddTimezoneToUsers < ActiveRecord::Migration[7.1] + def change + add_column :users, :timezone, :string, default: 'UTC' + add_index :users, :timezone + + # Populate existing users with application timezone + reversible do |dir| + dir.up do + User.update_all(timezone: Time.zone.name) + end + end + end +end \ No newline at end of file From 04a16029a495493d34242ecd4b81162abd127a80 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 22 Jul 2025 23:57:54 +0200 Subject: [PATCH 4/6] Remove benchmark_stats.rb --- benchmark_stats.rb | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 benchmark_stats.rb diff --git a/benchmark_stats.rb b/benchmark_stats.rb deleted file mode 100644 index 94049f14..00000000 --- a/benchmark_stats.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'benchmark' - -# Test the optimized stats calculation -data = Benchmark.measure do - user_id = 7 - - last_calculated_at = DateTime.new(1970, 1, 1) - - time_diff = last_calculated_at.to_i..Time.current.to_i - timestamps = Point.where(user_id:, timestamp: time_diff).pluck(:timestamp).uniq - - months = timestamps.group_by do |timestamp| - time = Time.zone.at(timestamp) - [time.year, time.month] - end.keys - - months.each do |year, month| - Stats::CalculateMonth.new(user_id, year, month).call - end -end - -puts "Stats calculation benchmark:" -puts "User Time: #{data.utime}s" -puts "System Time: #{data.stime}s" -puts "Total Time: #{data.real}s" - -# @real=28.869485000148416, -# @stime=2.4980050000000027, -# @total=20.303141999999976, -# @utime=17.805136999999974> From dfec1afd7e1a2df99af0b6b2d48a1ce5291c592c Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 23 Jul 2025 00:01:41 +0200 Subject: [PATCH 5/6] Remove example migration file --- .../future_add_timezone_to_users.rb.example | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 db/migrate/future_add_timezone_to_users.rb.example diff --git a/db/migrate/future_add_timezone_to_users.rb.example b/db/migrate/future_add_timezone_to_users.rb.example deleted file mode 100644 index caf60a88..00000000 --- a/db/migrate/future_add_timezone_to_users.rb.example +++ /dev/null @@ -1,16 +0,0 @@ -# Example migration for adding per-user timezone support -# To use: rename this file to remove .example and run rails db:migrate - -class AddTimezoneToUsers < ActiveRecord::Migration[7.1] - def change - add_column :users, :timezone, :string, default: 'UTC' - add_index :users, :timezone - - # Populate existing users with application timezone - reversible do |dir| - dir.up do - User.update_all(timezone: Time.zone.name) - end - end - end -end \ No newline at end of file From 25a185b206820f03247c25517b43b0e89fc0786c Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 23 Jul 2025 00:10:48 +0200 Subject: [PATCH 6/6] Add timezone validation to Stats::DailyDistanceQuery --- app/queries/stats/daily_distance_query.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/app/queries/stats/daily_distance_query.rb b/app/queries/stats/daily_distance_query.rb index 1673f1fa..39ae9f4e 100644 --- a/app/queries/stats/daily_distance_query.rb +++ b/app/queries/stats/daily_distance_query.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true class Stats::DailyDistanceQuery - def initialize(monthly_points, timespan, user_timezone = nil) + def initialize(monthly_points, timespan, timezone = nil) @monthly_points = monthly_points @timespan = timespan - @user_timezone = user_timezone || 'UTC' + @timezone = validate_timezone(timezone) end def call @@ -16,22 +16,22 @@ class Stats::DailyDistanceQuery private - attr_reader :monthly_points, :timespan, :user_timezone + attr_reader :monthly_points, :timespan, :timezone def daily_distances(monthly_points) Stat.connection.select_all(<<-SQL.squish) WITH points_with_distances AS ( SELECT - EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{user_timezone}')) as day_of_month, + EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{timezone}')) as day_of_month, CASE WHEN LAG(lonlat) OVER ( - PARTITION BY EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{user_timezone}')) + PARTITION BY EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{timezone}')) ORDER BY timestamp ) IS NOT NULL THEN ST_Distance( lonlat::geography, LAG(lonlat) OVER ( - PARTITION BY EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{user_timezone}')) + PARTITION BY EXTRACT(day FROM (to_timestamp(timestamp) AT TIME ZONE 'UTC' AT TIME ZONE '#{timezone}')) ORDER BY timestamp )::geography ) @@ -62,4 +62,10 @@ class Stats::DailyDistanceQuery [index, distance_meters.to_i] end end + + def validate_timezone(timezone) + return timezone if ActiveSupport::TimeZone.all.any? { |tz| tz.name == timezone } + + 'UTC' + end end