Cache the result of is_sharded?

[KJTsanaktsidis]

It can't possibly change.... and it's eating ~2% of our app's CPU?????

Depending on connection parameters, it seems that the Ticket#show endpoint in Classic is spending ~2% of total CPU time inside this check for is_sharded?

Here's a flamegraph snippet that shows where some of the time is going:

Basically, is_sharded? calls shard_names which winds up validating the config by checking e.g. that all the shard numbers are integers every time it's called. That obviously only needs to happen once.

I'm open to caching this at a different level than what I went for, but it seems this might move the needle a bit.

[bquorning]

The ActiveRecordShards::Model module is being extended onto ActiveRecord::Base, not included. So it’s already working at the class level, and self.class.foo is just calling methods on the Class class. 😬

You’re probably more looking for a diff like this:

diff --git a/lib/active_record_shards/model.rb b/lib/active_record_shards/model.rb
index 7d3c658..2dadd33 100644
--- a/lib/active_record_shards/model.rb
+++ b/lib/active_record_shards/model.rb
@@ -11,7 +11,9 @@ module ActiveRecordShards
     end
 
     def is_sharded? # rubocop:disable Naming/PredicateName
-      if self == ActiveRecord::Base
+      return @_ars_model_class_is_sharded if defined?(@_ars_model_class_is_sharded)
+
+      @_ars_model_class_is_sharded = if self == ActiveRecord::Base
         sharded != false && supports_sharding?
       elsif self == base_class
         if sharded.nil?

or even…

diff --git a/lib/active_record_shards/model.rb b/lib/active_record_shards/model.rb
index 7d3c658..a9944bd 100644
--- a/lib/active_record_shards/model.rb
+++ b/lib/active_record_shards/model.rb
@@ -11,7 +11,9 @@ module ActiveRecordShards
     end
 
     def is_sharded? # rubocop:disable Naming/PredicateName
-      if self == ActiveRecord::Base
+      return sharded unless sharded.nil?
+
+      self.sharded = if self == ActiveRecord::Base
         sharded != false && supports_sharding?
       elsif self == base_class
         if sharded.nil?

[bquorning]

The method chain supports_sharding? → shard_names → config_for_env → shard_env → app_env which can change during execution (even if it seems unlikely).

I am curious to hear your thoughts on #312 as an alternative (but not quite as optimized) solution.

[KJTsanaktsidis]

I think your suggestion works @bquorning.

I checked that caching the is_sharded? status & environment doesn't cause any funny load order issues - when I ran classic with eager loading on, I got the same result both with and without this patch:

[1] [zendesk][development] pry(main)> ActiveRecord::Base.descendants.partition(&:is_sharded?).map(&:count)
=> [429, 54]