Skip to content

Commit

Permalink
perf(Filters) - increase performance of updating charge filters (#2696)
Browse files Browse the repository at this point in the history
## Description 

Based on profiling information we found out that the `to_h` method on
ChargeFilter is called often when updating a charge with a large ( >
100) amount of filters.

A second observation is that the `touch` method means we're updating all
charge filters every time, which has poor performance when you have a
lot of those for a given charge.

## Changes

- cache `to_h` 
- updated_at is used for ordering the filters, but if you have > 100
we've chosen to optimize and not touch all records. This means stuff
will change in the UI, but at that amount nobody is really managing them
based on order.
  • Loading branch information
nudded authored Oct 16, 2024
1 parent b860426 commit 34c82c5
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 17 deletions.
16 changes: 8 additions & 8 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ GEM
listen (3.9.0)
rb-fsevent (~> 0.10, >= 0.10.3)
rb-inotify (~> 0.9, >= 0.9.10)
logger (1.6.0)
logger (1.6.1)
lograge (0.14.0)
actionpack (>= 4)
activesupport (>= 4)
Expand Down Expand Up @@ -591,7 +591,7 @@ GEM
ast (~> 2.4.1)
racc
pg (1.5.7)
prism (0.30.0)
prism (1.2.0)
pry (0.14.2)
coderay (~> 1.1)
method_source (~> 1.0)
Expand Down Expand Up @@ -652,7 +652,7 @@ GEM
rb-fsevent (0.11.2)
rb-inotify (0.11.1)
ffi (~> 1.0)
rbs (3.5.2)
rbs (3.6.1)
logger
rdoc (6.7.0)
psych (>= 4.0.0)
Expand Down Expand Up @@ -736,13 +736,13 @@ GEM
rubocop (~> 1.61)
rubocop-thread_safety (0.5.1)
rubocop (>= 0.90.0)
ruby-lsp (0.17.15)
ruby-lsp (0.20.0)
language_server-protocol (~> 3.17.0)
prism (>= 0.29.0, < 0.31)
prism (>= 1.2, < 2.0)
rbs (>= 3, < 4)
sorbet-runtime (>= 0.5.10782)
ruby-lsp-rails (0.3.13)
ruby-lsp (>= 0.17.12, < 0.18.0)
ruby-lsp-rails (0.3.19)
ruby-lsp (>= 0.20.0, < 0.21.0)
ruby-progressbar (1.13.0)
sass-rails (6.0.0)
sassc-rails (~> 2.1, >= 2.1.1)
Expand Down Expand Up @@ -796,7 +796,7 @@ GEM
snaky_hash (2.0.1)
hashie
version_gem (~> 1.1, >= 1.1.1)
sorbet-runtime (0.5.11535)
sorbet-runtime (0.5.11602)
sprockets (4.2.1)
concurrent-ruby (~> 1.0)
rack (>= 2.2.4, < 4)
Expand Down
4 changes: 2 additions & 2 deletions app/models/charge_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ def display_name(separator: ', ')
end

def to_h
values.each_with_object({}) do |filter_value, result|
@to_h ||= values.each_with_object({}) do |filter_value, result|
result[filter_value.billable_metric_filter.key] = filter_value.values
end
end

def to_h_with_all_values
values.each_with_object({}) do |filter_value, result|
@to_h_with_all_values ||= values.each_with_object({}) do |filter_value, result|
values = filter_value.values
values = filter_value.billable_metric_filter.values if values == [ChargeFilterValue::ALL_FILTER_VALUES]

Expand Down
13 changes: 6 additions & 7 deletions app/services/charge_filters/create_or_update_batch_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,22 @@ def call
return result
end

# We only care about order when you have less than 100 filters.
touch = filters_params.size < 100

ActiveRecord::Base.transaction do
filters_params.each do |filter_param|
# NOTE: since a filter could be a refinement of another one, we have to make sure
# that we are targeting the right one
filter = filters.find do |f|
next unless f.to_h.sort == filter_param[:values].sort

f.values.all? do |value|
filter_param[:values][value.key].sort == value.values.sort
end
f.to_h.sort == filter_param[:values].sort
end

filter ||= charge.filters.new

filter.invoice_display_name = filter_param[:invoice_display_name]
filter.properties = filter_param[:properties]
if filter.save! && !filter.changed?
if filter.save! && touch && !filter.changed?
# NOTE: Make sure update_at is touched even if not changed to keep the right order
filter.touch # rubocop:disable Rails/SkipsModelValidations
end
Expand All @@ -48,7 +47,7 @@ def call
)

filter_value.values = values
if filter_value.save! && !filter_value.changed?
if filter_value.save! && touch && !filter_value.changed?
# NOTE: Make sure update_at is touched even if not changed to keep the right order
filter_value.touch # rubocop:disable Rails/SkipsModelValidations
end
Expand Down

0 comments on commit 34c82c5

Please sign in to comment.