[PATCH 4/5] socket (bsock_recvmsg_internal): avoid arg parsing

[Eric Wong <e@80x24.org>]

* ext/socket/ancdata.c (bsock_recvmsg_internal): avoid arg parsing
  (rsock_bsock_recvmsg): adjust for above change
  (rsock_bsock_recvmsg_nonblock): ditto
  [ruby-core:71439] [Feature #11339]
* ext/socket/rubysocket.h: adjust prototypes for above
* ext/socket/basicsocket.c (rsock_init_basicsocket):
  adjust private methods
* ext/socket/lib/socket.rb (BasicSocket#recvmsg): wrapper method
  (BasicSocket#recvmsg_nonblock): ditto

target 0: a (ruby 2.3.0dev (2015-11-12 trunk 52550) [x86_64-linux])
target 1: b (ruby 2.3.0dev (2015-11-12 avoid-kwarg-capi 52550) [x86_64-linux]

-----------------------------------------------------------
recvmsg_nonblock

require 'socket'
nr = 1_000_000
i = 0
msg = '.'
buf = '.'
begin
  r, w = UNIXSocket.pair(:SEQPACKET)
  while i < nr
    i += 1
    w.sendmsg(msg)
    r.recvmsg_nonblock(1, exception: false)
  end
ensure
  r.close
  w.close
end

-----------------------------------------------------------
raw data:

[["recvmsg_nonblock",
  [[3.721687912940979,
    3.6072621569037437,
    3.580637402832508,
    3.614185404032469,
    3.6029579415917397],
   [2.4694008752703667,
    2.4908322244882584,
    2.5051278844475746,
    2.5037173740565777,
    2.548359278589487]]]]

Elapsed time: 30.646087052 (sec)
-----------------------------------------------------------
benchmark results:
minimum results in each 5 measurements.
Execution time (sec)
name             a       b
recvmsg_nonblock   3.581   2.469

Speedup ratio: compare with the result of `a' (greater is better)
name             b
recvmsg_nonblock   1.450
---
 benchmark/bm_recvmsg_nonblock.rb | 16 +++++++
 ext/socket/ancdata.c             | 97 +++++++---------------------------------
 ext/socket/basicsocket.c         |  8 +++-
 ext/socket/lib/socket.rb         | 71 +++++++++++++++++++++++++++++
 ext/socket/rubysocket.h          |  6 ++-
 5 files changed, 112 insertions(+), 86 deletions(-)
 create mode 100644 benchmark/bm_recvmsg_nonblock.rb

diff --git a/benchmark/bm_recvmsg_nonblock.rb b/benchmark/bm_recvmsg_nonblock.rb
new file mode 100644
index 0000000..3c1056b
--- /dev/null
+++ b/benchmark/bm_recvmsg_nonblock.rb
@@ -0,0 +1,16 @@
+require 'socket'
+nr = 1_000_000
+i = 0
+msg = '.'
+buf = '.'
+begin
+  r, w = UNIXSocket.pair(:SEQPACKET)
+  while i < nr
+    i += 1
+    w.sendmsg(msg)
+    r.recvmsg_nonblock(1, exception: false)
+  end
+ensure
+  r.close
+  w.close
+end
diff --git a/ext/socket/ancdata.c b/ext/socket/ancdata.c
index afa75ef..877975e 100644
--- a/ext/socket/ancdata.c
+++ b/ext/socket/ancdata.c
@@ -1487,11 +1487,11 @@ make_io_for_unix_rights(VALUE ctl, struct cmsghdr *cmh, char *msg_end)
 #endif
 
 static VALUE
-bsock_recvmsg_internal(int argc, VALUE *argv, VALUE sock, int nonblock)
+bsock_recvmsg_internal(VALUE sock,
+		VALUE vmaxdatlen, VALUE vflags, VALUE vmaxctllen,
+		VALUE scm_rights, VALUE ex, int nonblock)
 {
     rb_io_t *fptr;
-    VALUE vmaxdatlen, vmaxctllen, vflags;
-    VALUE vopts;
     int grow_buffer;
     size_t maxdatlen;
     int flags, orig_flags;
@@ -1512,17 +1512,14 @@ bsock_recvmsg_internal(int argc, VALUE *argv, VALUE sock, int nonblock)
     int gc_done = 0;
 #endif
 
-
-    rb_scan_args(argc, argv, "03:", &vmaxdatlen, &vflags, &vmaxctllen, &vopts);
-
-    maxdatlen = NIL_P(vmaxdatlen) ? 4096 : NUM2SIZET(vmaxdatlen);
+    maxdatlen = NUM2SIZET(vmaxdatlen);
 #if defined(HAVE_STRUCT_MSGHDR_MSG_CONTROL)
-    maxctllen = NIL_P(vmaxctllen) ? 4096 : NUM2SIZET(vmaxctllen);
+    maxctllen = NUM2SIZET(vmaxctllen);
 #else
     if (!NIL_P(vmaxctllen))
         rb_raise(rb_eArgError, "control message not supported");
 #endif
-    flags = NIL_P(vflags) ? 0 : NUM2INT(vflags);
+    flags = NUM2INT(vflags);
 #ifdef MSG_DONTWAIT
     if (nonblock)
         flags |= MSG_DONTWAIT;
@@ -1532,7 +1529,7 @@ bsock_recvmsg_internal(int argc, VALUE *argv, VALUE sock, int nonblock)
     grow_buffer = NIL_P(vmaxdatlen) || NIL_P(vmaxctllen);
 
     request_scm_rights = 0;
-    if (!NIL_P(vopts) && RTEST(rb_hash_aref(vopts, ID2SYM(rb_intern("scm_rights")))))
+    if (RTEST(scm_rights))
         request_scm_rights = 1;
 #if !defined(HAVE_STRUCT_MSGHDR_MSG_CONTROL)
     if (request_scm_rights)
@@ -1602,7 +1599,7 @@ bsock_recvmsg_internal(int argc, VALUE *argv, VALUE sock, int nonblock)
             goto retry;
         }
         if (nonblock && (errno == EWOULDBLOCK || errno == EAGAIN)) {
-            if (rsock_opt_false_p(vopts, sym_exception)) {
+            if (ex == Qfalse) {
                 return sym_wait_readable;
             }
             rb_readwrite_sys_fail(RB_IO_WAIT_READABLE, "recvmsg(2) would block");
@@ -1720,85 +1717,21 @@ bsock_recvmsg_internal(int argc, VALUE *argv, VALUE sock, int nonblock)
 #endif
 
 #if defined(HAVE_RECVMSG)
-/*
- * call-seq:
- *    basicsocket.recvmsg(maxmesglen=nil, flags=0, maxcontrollen=nil, opts={}) => [mesg, sender_addrinfo, rflags, *controls]
- *
- * recvmsg receives a message using recvmsg(2) system call in blocking manner.
- *
- * _maxmesglen_ is the maximum length of mesg to receive.
- *
- * _flags_ is bitwise OR of MSG_* constants such as Socket::MSG_PEEK.
- *
- * _maxcontrollen_ is the maximum length of controls (ancillary data) to receive.
- *
- * _opts_ is option hash.
- * Currently :scm_rights=>bool is the only option.
- *
- * :scm_rights option specifies that application expects SCM_RIGHTS control message.
- * If the value is nil or false, application don't expects SCM_RIGHTS control message.
- * In this case, recvmsg closes the passed file descriptors immediately.
- * This is the default behavior.
- *
- * If :scm_rights value is neither nil nor false, application expects SCM_RIGHTS control message.
- * In this case, recvmsg creates IO objects for each file descriptors for
- * Socket::AncillaryData#unix_rights method.
- *
- * The return value is 4-elements array.
- *
- * _mesg_ is a string of the received message.
- *
- * _sender_addrinfo_ is a sender socket address for connection-less socket.
- * It is an Addrinfo object.
- * For connection-oriented socket such as TCP, sender_addrinfo is platform dependent.
- *
- * _rflags_ is a flags on the received message which is bitwise OR of MSG_* constants such as Socket::MSG_TRUNC.
- * It will be nil if the system uses 4.3BSD style old recvmsg system call.
- *
- * _controls_ is ancillary data which is an array of Socket::AncillaryData objects such as:
- *
- *   #<Socket::AncillaryData: AF_UNIX SOCKET RIGHTS 7>
- *
- * _maxmesglen_ and _maxcontrollen_ can be nil.
- * In that case, the buffer will be grown until the message is not truncated.
- * Internally, MSG_PEEK is used and MSG_TRUNC/MSG_CTRUNC are checked.
- *
- * recvmsg can be used to implement recv_io as follows:
- *
- *   mesg, sender_sockaddr, rflags, *controls = sock.recvmsg(:scm_rights=>true)
- *   controls.each {|ancdata|
- *     if ancdata.cmsg_is?(:SOCKET, :RIGHTS)
- *       return ancdata.unix_rights[0]
- *     end
- *   }
- *
- */
 VALUE
-rsock_bsock_recvmsg(int argc, VALUE *argv, VALUE sock)
+rsock_bsock_recvmsg(VALUE sock, VALUE dlen, VALUE flags, VALUE clen,
+		    VALUE scm_rights)
 {
-    return bsock_recvmsg_internal(argc, argv, sock, 0);
+    VALUE ex = Qtrue;
+    return bsock_recvmsg_internal(sock, dlen, flags, clen, scm_rights, ex, 0);
 }
 #endif
 
 #if defined(HAVE_RECVMSG)
-/*
- * call-seq:
- *    basicsocket.recvmsg_nonblock(maxdatalen=nil, flags=0, maxcontrollen=nil, opts={}) => [data, sender_addrinfo, rflags, *controls]
- *
- * recvmsg receives a message using recvmsg(2) system call in non-blocking manner.
- *
- * It is similar to BasicSocket#recvmsg
- * but non-blocking flag is set before the system call
- * and it doesn't retry the system call.
- *
- * By specifying `exception: false`, the _opts_ hash allows you to indicate
- * that recvmsg_nonblock should not raise an IO::WaitWritable exception, but
- * return the symbol :wait_writable instead.
- */
 VALUE
-rsock_bsock_recvmsg_nonblock(int argc, VALUE *argv, VALUE sock)
+rsock_bsock_recvmsg_nonblock(VALUE sock, VALUE dlen, VALUE flags, VALUE clen,
+			     VALUE scm_rights, VALUE ex)
 {
-    return bsock_recvmsg_internal(argc, argv, sock, 1);
+    return bsock_recvmsg_internal(sock, dlen, flags, clen, scm_rights, ex, 1);
 }
 #endif
 
diff --git a/ext/socket/basicsocket.c b/ext/socket/basicsocket.c
index 6bf10e8..1a0120c 100644
--- a/ext/socket/basicsocket.c
+++ b/ext/socket/basicsocket.c
@@ -726,7 +726,11 @@ rsock_init_basicsocket(void)
 
     rb_define_method(rb_cBasicSocket, "sendmsg", rsock_bsock_sendmsg, -1); /* in ancdata.c */
     rb_define_method(rb_cBasicSocket, "sendmsg_nonblock", rsock_bsock_sendmsg_nonblock, -1); /* in ancdata.c */
-    rb_define_method(rb_cBasicSocket, "recvmsg", rsock_bsock_recvmsg, -1); /* in ancdata.c */
-    rb_define_method(rb_cBasicSocket, "recvmsg_nonblock", rsock_bsock_recvmsg_nonblock, -1); /* in ancdata.c */
+
+    /* in ancdata.c */
+    rb_define_private_method(rb_cBasicSocket, "__recvmsg",
+			     rsock_bsock_recvmsg, 4);
+    rb_define_private_method(rb_cBasicSocket, "__recvmsg_nonblock",
+			    rsock_bsock_recvmsg_nonblock, 5);
 
 }
diff --git a/ext/socket/lib/socket.rb b/ext/socket/lib/socket.rb
index edee69a..65e3d02 100644
--- a/ext/socket/lib/socket.rb
+++ b/ext/socket/lib/socket.rb
@@ -322,6 +322,77 @@ class BasicSocket < IO
   def recv_nonblock(len, flag = 0, str = nil, exception: true)
     __recv_nonblock(len, flag, str, exception)
   end
+
+  # call-seq:
+  #    basicsocket.recvmsg(maxmesglen=nil, flags=0, maxcontrollen=nil, opts={}) => [mesg, sender_addrinfo, rflags, *controls]
+  #
+  # recvmsg receives a message using recvmsg(2) system call in blocking manner.
+  #
+  # _maxmesglen_ is the maximum length of mesg to receive.
+  #
+  # _flags_ is bitwise OR of MSG_* constants such as Socket::MSG_PEEK.
+  #
+  # _maxcontrollen_ is the maximum length of controls (ancillary data) to receive.
+  #
+  # _opts_ is option hash.
+  # Currently :scm_rights=>bool is the only option.
+  #
+  # :scm_rights option specifies that application expects SCM_RIGHTS control message.
+  # If the value is nil or false, application don't expects SCM_RIGHTS control message.
+  # In this case, recvmsg closes the passed file descriptors immediately.
+  # This is the default behavior.
+  #
+  # If :scm_rights value is neither nil nor false, application expects SCM_RIGHTS control message.
+  # In this case, recvmsg creates IO objects for each file descriptors for
+  # Socket::AncillaryData#unix_rights method.
+  #
+  # The return value is 4-elements array.
+  #
+  # _mesg_ is a string of the received message.
+  #
+  # _sender_addrinfo_ is a sender socket address for connection-less socket.
+  # It is an Addrinfo object.
+  # For connection-oriented socket such as TCP, sender_addrinfo is platform dependent.
+  #
+  # _rflags_ is a flags on the received message which is bitwise OR of MSG_* constants such as Socket::MSG_TRUNC.
+  # It will be nil if the system uses 4.3BSD style old recvmsg system call.
+  #
+  # _controls_ is ancillary data which is an array of Socket::AncillaryData objects such as:
+  #
+  #   #<Socket::AncillaryData: AF_UNIX SOCKET RIGHTS 7>
+  #
+  # _maxmesglen_ and _maxcontrollen_ can be nil.
+  # In that case, the buffer will be grown until the message is not truncated.
+  # Internally, MSG_PEEK is used and MSG_TRUNC/MSG_CTRUNC are checked.
+  #
+  # recvmsg can be used to implement recv_io as follows:
+  #
+  #   mesg, sender_sockaddr, rflags, *controls = sock.recvmsg(:scm_rights=>true)
+  #   controls.each {|ancdata|
+  #     if ancdata.cmsg_is?(:SOCKET, :RIGHTS)
+  #       return ancdata.unix_rights[0]
+  #     end
+  #   }
+  def recvmsg(dlen = 4096, flags = 0, clen = 4096, scm_rights: false)
+    __recvmsg(dlen, flags, clen, scm_rights)
+  end
+
+  # call-seq:
+  #    basicsocket.recvmsg_nonblock(maxdatalen=nil, flags=0, maxcontrollen=nil, opts={}) => [data, sender_addrinfo, rflags, *controls]
+  #
+  # recvmsg receives a message using recvmsg(2) system call in non-blocking manner.
+  #
+  # It is similar to BasicSocket#recvmsg
+  # but non-blocking flag is set before the system call
+  # and it doesn't retry the system call.
+  #
+  # By specifying `exception: false`, the _opts_ hash allows you to indicate
+  # that recvmsg_nonblock should not raise an IO::WaitWritable exception, but
+  # return the symbol :wait_writable instead.
+  def recvmsg_nonblock(dlen = 4096, flags = 0, clen = 4096,
+                       scm_rights: false, exception: true)
+    __recvmsg_nonblock(dlen, flags, clen, scm_rights, exception)
+  end
 end
 
 class Socket < BasicSocket
diff --git a/ext/socket/rubysocket.h b/ext/socket/rubysocket.h
index d39de0a..652876c 100644
--- a/ext/socket/rubysocket.h
+++ b/ext/socket/rubysocket.h
@@ -369,8 +369,10 @@ VALUE rsock_bsock_sendmsg_nonblock(int argc, VALUE *argv, VALUE sock);
 #endif
 
 #if defined(HAVE_RECVMSG)
-VALUE rsock_bsock_recvmsg(int argc, VALUE *argv, VALUE sock);
-VALUE rsock_bsock_recvmsg_nonblock(int argc, VALUE *argv, VALUE sock);
+VALUE rsock_bsock_recvmsg(VALUE sock, VALUE dlen, VALUE clen, VALUE flags,
+			  VALUE scm_rights);
+VALUE rsock_bsock_recvmsg_nonblock(VALUE sock, VALUE dlen, VALUE clen,
+				   VALUE flags, VALUE scm_rights, VALUE ex);
 ssize_t rsock_recvmsg(int socket, struct msghdr *message, int flags);
 #else
 #define rsock_bsock_recvmsg rb_f_notimplement
-- 
EW