perf: use struct instead of hash for memory consumption (#140)

Author: supercaracal

lib/redis_client/cluster/command.rb

@@ -9,6 +9,8 @@ class Cluster
     class Command
       EMPTY_STRING = ''
 
+      RedisCommand = Struct.new('RedisCommand', :first_key_position, :write?, :readonly?, keyword_init: true)
+
       class << self
         def load(nodes)
           errors = []
@@ -17,8 +19,8 @@ def load(nodes)
             break unless cmd.nil?
 
             reply = node.call('COMMAND')
-            details = parse_command_details(reply)
-            cmd = ::RedisClient::Cluster::Command.new(details)
+            commands = parse_command_reply(reply)
+            cmd = ::RedisClient::Cluster::Command.new(commands)
           rescue ::RedisClient::Error => e
             errors << e
           end
@@ -30,15 +32,22 @@ def load(nodes)
 
         private
 
-        def parse_command_details(rows)
+        def parse_command_reply(rows)
           rows&.reject { |row| row[0].nil? }.to_h do |row|
-            [row[0].downcase, { arity: row[1], flags: row[2], first: row[3], last: row[4], step: row[5] }]
+            [
+              row[0].downcase,
+              RedisCommand.new(
+                first_key_position: row[3],
+                write?: row[2].include?('write'),
+                readonly?: row[2].include?('readonly')
+              )
+            ]
           end
         end
       end
 
-      def initialize(details)
-        @details = pick_details(details)
+      def initialize(commands)
+        @commands = commands || {}
       end
 
       def extract_first_key(command)
@@ -51,39 +60,23 @@ def extract_first_key(command)
       end
 
       def should_send_to_primary?(command)
-        dig_details(command, :write)
+        name = ::RedisClient::Cluster::NormalizedCmdName.instance.get_by_command(command)
+        @commands[name]&.write?
       end
 
       def should_send_to_replica?(command)
-        dig_details(command, :readonly)
+        name = ::RedisClient::Cluster::NormalizedCmdName.instance.get_by_command(command)
+        @commands[name]&.readonly?
       end
 
       def exists?(name)
-        key = ::RedisClient::Cluster::NormalizedCmdName.instance.get_by_name(name)
-        @details.key?(key)
+        @commands.key?(::RedisClient::Cluster::NormalizedCmdName.instance.get_by_name(name))
       end
 
       private
 
-      def pick_details(details)
-        (details || {}).transform_values do |detail|
-          {
-            first_key_position: detail[:first],
-            write: detail[:flags].include?('write'),
-            readonly: detail[:flags].include?('readonly')
-          }
-        end
-      end
-
-      def dig_details(command, key)
-        name = ::RedisClient::Cluster::NormalizedCmdName.instance.get_by_command(command)
-        return if name.empty? || !@details.key?(name)
-
-        @details.fetch(name).fetch(key)
-      end
-
       def determine_first_key_position(command) # rubocop:disable Metrics/CyclomaticComplexity
-        case ::RedisClient::Cluster::NormalizedCmdName.instance.get_by_command(command)
+        case name = ::RedisClient::Cluster::NormalizedCmdName.instance.get_by_command(command)
         when 'eval', 'evalsha', 'zinterstore', 'zunionstore' then 3
         when 'object' then 2
         when 'memory'
@@ -93,7 +86,7 @@ def determine_first_key_position(command) # rubocop:disable Metrics/CyclomaticCo
         when 'xread', 'xreadgroup'
           determine_optional_key_position(command, 'streams')
         else
-          dig_details(command, :first_key_position).to_i
+          @commands[name]&.first_key_position.to_i
         end
       end
 

test/redis_client/cluster/test_command.rb

@@ -31,37 +31,36 @@ def test_load
         end
       end
 
-      def test_parse_command_details
-        keys = %i[arity flags first last step].freeze
+      def test_parse_command_reply
         [
           {
             rows: [
               ['get', 2, Set['readonly', 'fast'], 1, 1, 1, Set['@read', '@string', '@fast'], Set[], Set[], Set[]],
               ['set', -3, Set['write', 'denyoom', 'movablekeys'], 1, 1, 1, Set['@write', '@string', '@slow'], Set[], Set[], Set[]]
             ],
             want: {
-              'get' => { arity: 2, flags: Set['readonly', 'fast'], first: 1, last: 1, step: 1 },
-              'set' => { arity: -3, flags: Set['write', 'denyoom', 'movablekeys'], first: 1, last: 1, step: 1 }
+              'get' => { first_key_position: 1, write?: false, readonly?: true },
+              'set' => { first_key_position: 1, write?: true, readonly?: false }
             }
           },
           {
             rows: [
               ['GET', 2, Set['readonly', 'fast'], 1, 1, 1, Set['@read', '@string', '@fast'], Set[], Set[], Set[]]
             ],
             want: {
-              'get' => { arity: 2, flags: Set['readonly', 'fast'], first: 1, last: 1, step: 1 }
+              'get' => { first_key_position: 1, write?: false, readonly?: true }
             }
           },
           { rows: [[]], want: {} },
           { rows: [], want: {} },
           { rows: nil, want: {} }
         ].each_with_index do |c, idx|
           msg = "Case: #{idx}"
-          got = ::RedisClient::Cluster::Command.send(:parse_command_details, c[:rows])
+          got = ::RedisClient::Cluster::Command.send(:parse_command_reply, c[:rows])
           assert_equal(c[:want].size, got.size, msg)
           assert_equal(c[:want].keys.sort, got.keys.sort, msg)
-          c[:want].each do |k1, v|
-            keys.each { |k2| assert_equal(v[k2], got[k1][k2], "#{msg}: #{k2}") }
+          c[:want].each do |k, v|
+            assert_equal(v, got[k].to_h, "#{msg}: #{k}")
           end
         end
       end
@@ -134,60 +133,6 @@ def test_exists?
         end
       end
 
-      def test_pick_details
-        keys = %i[first_key_position write readonly].freeze
-        [
-          {
-            details: {
-              'get' => { arity: 2, flags: Set['readonly', 'fast'], first: 1, last: 1, step: 1 },
-              'set' => { arity: -3, flags: Set['write', 'denyoom', 'movablekeys'], first: 1, last: 1, step: 1 }
-            },
-            want: {
-              'get' => { first_key_position: 1, write: false, readonly: true },
-              'set' => { first_key_position: 1, write: true, readonly: false }
-            }
-          },
-          { details: {}, want: {} },
-          { details: nil, want: {} }
-        ].each_with_index do |c, idx|
-          msg = "Case: #{idx}"
-          cmd = ::RedisClient::Cluster::Command.new(c[:details])
-          got = cmd.send(:pick_details, c[:details])
-          assert_equal(c[:want].size, got.size, msg)
-          assert_equal(c[:want].keys.sort, got.keys.sort, msg)
-          c[:want].each do |k1, v|
-            keys.each { |k2| assert_equal(v[k2], got[k1][k2], "#{msg}: #{k2}") }
-          end
-        end
-      end
-
-      def test_dig_details
-        cmd = ::RedisClient::Cluster::Command.new(
-          {
-            'get' => { arity: 2, flags: Set['readonly', 'fast'], first: 1, last: 1, step: 1 },
-            'set' => { arity: -3, flags: Set['write', 'denyoom', 'movablekeys'], first: 1, last: 1, step: 1 }
-          }
-        )
-        [
-          { params: { command: %w[SET foo 1], key: :first_key_position }, want: 1 },
-          { params: { command: %w[SET foo 1], key: :write }, want: true },
-          { params: { command: %w[set foo 1], key: :write }, want: true },
-          { params: { command: %w[SET foo 1], key: :readonly }, want: false },
-          { params: { command: %w[GET foo], key: :first_key_position }, want: 1 },
-          { params: { command: %w[GET foo], key: :write }, want: false },
-          { params: { command: %w[GET foo], key: :readonly }, want: true },
-          { params: { command: %w[get foo], key: :readonly }, want: true },
-          { params: { command: %w[UNKNOWN foo], key: :readonly }, want: nil },
-          { params: { command: [['SET'], 'foo', 1], key: :write }, want: true },
-          { params: { command: [], key: :readonly }, want: nil },
-          { params: { command: nil, key: :readonly }, want: nil }
-        ].each_with_index do |c, idx|
-          msg = "Case: #{idx}"
-          got = cmd.send(:dig_details, c[:params][:command], c[:params][:key])
-          c[:want].nil? ? assert_nil(got, msg) : assert_equal(c[:want], got, msg)
-        end
-      end
-
       def test_determine_first_key_position
         cmd = ::RedisClient::Cluster::Command.load(@raw_clients)
         [