Re: [Tarantool-patches] [PATCH 13/20] net.box: rewrite send_and_recv_{iproto, console} in C

[Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Wed, Aug 04, 2021 at 01:06:04AM +0200, Vladislav Shpilevoy wrote:
> >>> +        local hdr, body_rpos, body_end = internal.send_and_recv_iproto(
> >>
> >> 3. Indexing 'internal' via '.' is not free. It is a lookup
> >> in a hash. You might want to save internal.send_and_recv_iproto into
> >> a global variable when the module loads first time and use the
> >> cached value. Non-cached version is a bit faster only for FFI, but
> >> here you are using Lua C - cache should be good.
> >>
> >>> +            connection:fd(), send_buf, recv_buf, timeout)
> >>
> >> Another idea is to cache 'connection:fd()' result into a variable in
> >> the root of create_transport() function. And update it when the
> >> connetion is re-established. Although you probably move this all to
> >> C later as well, I didn't reach the last commits yet.
> > 
> > The calling function is moved to C later in the patch set so these
> > comments will become irrelevant.
> > 
> > Regarding caching function name (instead of accessing via dot operator),
> > eventually there will be only two hot C functions that could benefit
> > from this:
> > 
> >   internal.perform_request
> >   internal.perform_async_request
> > 
> > I tried caching their names, but saw no performance gain at all in my
> > test. I also tried removing fiber_self and fiber_clock aliases from
> > net_box.lua and accessing these functions as fiber.<name> - again no
> > difference.
> 
> Both can be explained by jitting. I wasn't sure if 'internal' is jitted,
> but seems so. While jit works, there probably won't be any difference. Now
> it is just inconsistent because you have a couple of methods cached:
> 
> 	local encode_method   = internal.encode_method
> 	local decode_greeting = internal.decode_greeting

I compared performance of fiber_self vs fiber.self by running it 100M
times in a loop. Here are the results over 10 runs:

 fiber.self : 15.279 +- 0.021 MRPS
 fiber_self : 15.357 +- 0.023 MRPS

(fiber.self is a Lua C function)

So caching function name does result in 0.5% boost.

Okay, let's add aliases for the hot functions. Here's the patch I
applied on top of the series:
--
From 58687162d5102ffabc42fb2ef9a561ee7b218ab1 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Wed, 4 Aug 2021 16:51:06 +0300
Subject: [PATCH] net.box: add Lua aliases for perform_request and
 perform_async_request

These two are hot functions. Let's add aliases for them to avoid a
lookup in a Lua table on each call.

At the same time, internal.decode_greeting is called only when a
connection is established so we don't need to have an alias for it.

Part of #6241

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 9da900933fbc..ad65a3dd528f 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -18,7 +18,8 @@ local check_index_arg     = box.internal.check_index_arg
 local check_space_arg     = box.internal.check_space_arg
 local check_primary_index = box.internal.check_primary_index
 
-local decode_greeting = internal.decode_greeting
+local perform_request_impl          = internal.perform_request
+local perform_async_request_impl    = internal.perform_async_request
 
 local TIMEOUT_INFINITY = 500 * 365 * 86400
 local VSPACE_ID        = 281
@@ -79,7 +80,7 @@ local function establish_connection(host, port, timeout)
         s:close()
         return nil, err
     end
-    local greeting, err = decode_greeting(msg)
+    local greeting, err = internal.decode_greeting(msg)
     if not greeting then
         s:close()
         return nil, err
@@ -280,9 +281,9 @@ local function create_transport(host, port, user, password, callback,
         if send_buf:size() == 0 then
             worker_fiber:wakeup()
         end
-        return internal.perform_async_request(requests, send_buf, buffer,
-                                              skip_header, method, on_push,
-                                              on_push_ctx, format, ...)
+        return perform_async_request_impl(requests, send_buf, buffer,
+                                          skip_header, method, on_push,
+                                          on_push_ctx, format, ...)
     end
 
     --
@@ -301,9 +302,9 @@ local function create_transport(host, port, user, password, callback,
         if send_buf:size() == 0 then
             worker_fiber:wakeup()
         end
-        return internal.perform_request(timeout, requests, send_buf, buffer,
-                                        skip_header, method, on_push,
-                                        on_push_ctx, format, ...)
+        return perform_request_impl(timeout, requests, send_buf, buffer,
+                                    skip_header, method, on_push, on_push_ctx,
+                                    format, ...)
     end
 
     -- PROTOCOL STATE MACHINE (WORKER FIBER) --