perf: command builder should be called once

Author: supercaracal

.gitignore

@@ -1,3 +1,4 @@
 .bundle
 *.gem
+*.json
 Gemfile.lock

Gemfile

@@ -15,3 +15,4 @@ gem 'rubocop-minitest', require: false
 gem 'rubocop-performance', require: false
 gem 'rubocop-rake', require: false
 gem 'stackprof', platform: :mri
+gem 'vernier', platform: :mri

lib/redis_client/cluster/noop_command_builder.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class RedisClient
+  class Cluster
+    module NoopCommandBuilder
+      module_function
+
+      def generate(args, _kwargs = nil)
+        args
+      end
+    end
+  end
+end

lib/redis_client/cluster/pipeline.rb

@@ -2,6 +2,7 @@
 
 require 'redis_client'
 require 'redis_client/cluster/errors'
+require 'redis_client/cluster/noop_command_builder'
 require 'redis_client/connection_mixin'
 require 'redis_client/middlewares'
 require 'redis_client/pooled'
@@ -229,7 +230,7 @@ def execute # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Met
 
       def append_pipeline(node_key)
         @pipelines ||= {}
-        @pipelines[node_key] ||= ::RedisClient::Cluster::Pipeline::Extended.new(@command_builder)
+        @pipelines[node_key] ||= ::RedisClient::Cluster::Pipeline::Extended.new(::RedisClient::Cluster::NoopCommandBuilder)
         @pipelines[node_key].add_outer_index(@size)
         @size += 1
         @pipelines[node_key]

lib/redis_client/cluster/transaction.rb

@@ -2,6 +2,7 @@
 
 require 'redis_client'
 require 'redis_client/cluster/errors'
+require 'redis_client/cluster/noop_command_builder'
 require 'redis_client/cluster/pipeline'
 
 class RedisClient
@@ -18,7 +19,7 @@ def initialize(router, command_builder, node: nil, slot: nil, asking: false)
         @router = router
         @command_builder = command_builder
         @retryable = true
-        @pipeline = ::RedisClient::Pipeline.new(@command_builder)
+        @pipeline = ::RedisClient::Pipeline.new(::RedisClient::Cluster::NoopCommandBuilder)
         @pending_commands = []
         @node = node
         prepare_tx unless @node.nil?

lib/redis_client/cluster_config.rb

@@ -5,6 +5,7 @@
 require 'redis_client/cluster'
 require 'redis_client/cluster/errors'
 require 'redis_client/cluster/node_key'
+require 'redis_client/cluster/noop_command_builder'
 require 'redis_client/command_builder'
 
 class RedisClient
@@ -64,7 +65,7 @@ def initialize( # rubocop:disable Metrics/ParameterLists
     end
 
     def inspect
-      "#<#{self.class.name} #{startup_nodes.values}>"
+      "#<#{self.class.name} #{startup_nodes.values.map { |v| v.except(:command_builder) }}>"
     end
 
     def read_timeout
@@ -187,6 +188,7 @@ def build_startup_nodes(configs)
     def augment_client_config(config)
       config = @client_config.merge(config)
       config = config.merge(host: @fixed_hostname) unless @fixed_hostname.empty?
+      config[:command_builder] = ::RedisClient::Cluster::NoopCommandBuilder # prevent twice call
       config
     end
   end

test/prof_stack2.rb

@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+require 'vernier'
+require 'redis_cluster_client'
+require 'testing_constants'
+
+module ProfStack2
+  SIZE = 40
+  ATTEMPTS = 1000
+
+  module_function
+
+  def run
+    client = make_client
+    prepare(client)
+
+    case mode = ENV.fetch('PROFILE_MODE', :single).to_sym
+    when :single
+      execute(client, mode) do |client|
+        ATTEMPTS.times { |i| client.call('GET', "key#{i}") }
+      end
+    when :transaction
+      execute(client, mode) do |client|
+        ATTEMPTS.times do |i|
+          client.multi do |tx|
+            SIZE.times do |j|
+              n = SIZE * i + j
+              tx.call('SET', "{group:#{i}}:key:#{n}", n)
+            end
+          end
+        end
+      end
+    when :pipeline
+      execute(client, mode) do |client|
+        ATTEMPTS.times do |i|
+          client.pipelined do |pi|
+            SIZE.times do |j|
+              n = SIZE * i + j
+              pi.call('GET', "key#{n}")
+            end
+          end
+        end
+      end
+    end
+  end
+
+  def execute(client, mode)
+    Vernier.profile(out: "vernier_#{mode}.json") do
+      yield(client)
+    end
+  end
+
+  def make_client
+    ::RedisClient.cluster(
+      nodes: TEST_NODE_URIS,
+      replica: true,
+      replica_affinity: :random,
+      fixed_hostname: TEST_FIXED_HOSTNAME,
+      # concurrency: { model: :on_demand, size: 6 },
+      # concurrency: { model: :pooled, size: 6 },
+      concurrency: { model: :none },
+      **TEST_GENERIC_OPTIONS
+    ).new_client
+  end
+
+  def prepare(client)
+    ATTEMPTS.times do |i|
+      client.pipelined do |pi|
+        SIZE.times do |j|
+          n = SIZE * i + j
+          pi.call('SET', "key#{n}", "val#{n}")
+        end
+      end
+    end
+  end
+end
+
+ProfStack2.run

test/redis_client/test_cluster_config.rb

@@ -38,38 +38,38 @@ def test_startup_nodes
         {
           config: ::RedisClient::ClusterConfig.new,
           want: {
-            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379 }
+            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379, command_builder: ::RedisClient::Cluster::NoopCommandBuilder }
           }
         },
         {
           config: ::RedisClient::ClusterConfig.new(replica: true),
           want: {
-            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379 }
+            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379, command_builder: ::RedisClient::Cluster::NoopCommandBuilder }
           }
         },
         {
           config: ::RedisClient::ClusterConfig.new(fixed_hostname: 'endpoint.example.com'),
           want: {
-            '127.0.0.1:6379' => { host: 'endpoint.example.com', port: 6379 }
+            '127.0.0.1:6379' => { host: 'endpoint.example.com', port: 6379, command_builder: ::RedisClient::Cluster::NoopCommandBuilder }
           }
         },
         {
           config: ::RedisClient::ClusterConfig.new(timeout: 1),
           want: {
-            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379, timeout: 1 }
+            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379, timeout: 1, command_builder: ::RedisClient::Cluster::NoopCommandBuilder }
           }
         },
         {
           config: ::RedisClient::ClusterConfig.new(nodes: ['redis://1.2.3.4:1234', 'rediss://5.6.7.8:5678']),
           want: {
-            '1.2.3.4:1234' => { host: '1.2.3.4', port: 1234 },
-            '5.6.7.8:5678' => { host: '5.6.7.8', port: 5678, ssl: true }
+            '1.2.3.4:1234' => { host: '1.2.3.4', port: 1234, command_builder: ::RedisClient::Cluster::NoopCommandBuilder },
+            '5.6.7.8:5678' => { host: '5.6.7.8', port: 5678, ssl: true, command_builder: ::RedisClient::Cluster::NoopCommandBuilder }
           }
         },
         {
           config: ::RedisClient::ClusterConfig.new(custom: { foo: 'bar' }),
           want: {
-            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379, custom: { foo: 'bar' } }
+            '127.0.0.1:6379' => { host: '127.0.0.1', port: 6379, custom: { foo: 'bar' }, command_builder: ::RedisClient::Cluster::NoopCommandBuilder }
           }
         }
       ].each_with_index do |c, idx|
@@ -182,13 +182,15 @@ def test_client_config_for_node
         nodes: ['redis://username:[email protected]:1234', 'rediss://5.6.7.8:5678'],
         custom: { foo: 'bar' }
       )
-      assert_equal({
-                     host: '9.9.9.9',
-                     port: 9999,
-                     username: 'username',
-                     password: 'password',
-                     custom: { foo: 'bar' }
-                   }, config.client_config_for_node('9.9.9.9:9999'))
+      want = {
+        host: '9.9.9.9',
+        port: 9999,
+        username: 'username',
+        password: 'password',
+        custom: { foo: 'bar' },
+        command_builder: ::RedisClient::Cluster::NoopCommandBuilder
+      }
+      assert_equal(want, config.client_config_for_node('9.9.9.9:9999'))
     end
 
     def test_client_config_id