dumping ground for random patches and texts
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: spew@80x24.org
Subject: [REJECT] openssl: avoid kwarg parsing from C API at startup
Date: Wed,  2 Dec 2015 02:06:54 +0000	[thread overview]
Message-ID: <20151202020654.18328-1-e@80x24.org> (raw)

Avoiding kwarg parsing from the C API for accept_nonblock
and connect_nonblock should reduce hash allocations and
improve performance as it did for the regular socket variants.

However, It's only a 1% speedup, so probably not worth it given the
overheads of OpenSSL.  It's also entirely possible I'm benchmarking
this completely wrong because I'm not too familiar with OpenSSL.

target 0: a (ruby 2.3.0dev (2015-12-02 trunk 52842) [x86_64-linux]) at "/home/ew/rrrr/b/ruby"
target 1: b (ruby 2.3.0dev (2015-12-02 avoid-kwarg-capi 52842) [x86_64-linux]) at "/home/ew/ruby/b/ruby"

-----------------------------------------------------------
ssl_startup

require 'openssl'
require 'socket'

test_key_dh_1024 = OpenSSL::PKey::DH.new <<-_end_of_pem_
-----BEGIN DH PARAMETERS-----
MIGHAoGBAKnKQ8MNK6nYZzLrrcuTsLxuiJGXoOO5gT+tljOTbHBuiktdMTITzIY0
pFxIvjG05D7HoBZQfrR0c92NGWPkAiCkhQKB8JCbPVzwNLDy6DZ0pmofDKrEsYHG
AQjjxMXhwULlmuR/K+WwlaZPiLIBYalLAZQ7ZbOPeVkJ8ePao0eLAgEC
-----END DH PARAMETERS-----
  _end_of_pem_

test_key_dh_1024.priv_key = OpenSSL::BN.new("48561834C67E65FFD2A9B47F41" \
"E5E78FDC95C387428FDB1E4B0188B64D1643C3A8D3455B945B7E8C4D166010C7C2" \
"CE23BFB9BEF43D0348FE7FA5284B0225E7FE1537546D114E3D8A4411B9B9351AB4" \
"51E1A358F50ED61B1F00DA29336EEBBD649980AC86D76AF8BBB065298C2052672E" \
"EF3EF13AB47A15275FC2836F3AC74CEA", 16)

tmp_dh_callback = proc { test_key_dh_1024 }
srv_ctx = OpenSSL::SSL::SSLContext.new
srv_ctx.ciphers = 'ADH'.freeze
srv_ctx.tmp_dh_callback = tmp_dh_callback
require 'io/wait'
require 'tmpdir'
Dir.mktmpdir do |dir|
  path = "#{dir}/.sock"

  srv = UNIXServer.new(path)
  pid = fork do
    while c = srv.accept
      ssl = OpenSSL::SSL::SSLSocket.new(c, srv_ctx)
      case ssl.accept_nonblock(exception: false)
      when :wait_readable
        c.wait_readable
      when :wait_writable
        c.wait_writable
      else
        break
      end while true
      ssl.sync_close = true
      ssl.close
    end
  end

  buf = ''.b
  ctx = OpenSSL::SSL::SSLContext.new
  ctx.ciphers = 'ADH'.freeze

  6000.times do
    c = UNIXSocket.new(path)
    ssl = OpenSSL::SSL::SSLSocket.new(c, ctx)
    case ssl.connect_nonblock(exception: false)
    when :wait_readable
      c.wait_readable
    when :wait_writable
      c.wait_writable
    else
      break
    end while true
    ssl.sync_close = true
    ssl.read(1, buf)
    ssl.close
  end
  Process.kill(:TERM, pid)
  Process.waitpid2(pid)
end

a	15.832569818012416
a	15.803461923962459
a	15.811633185017854
a	15.771200725110248
a	15.769214095082134
b	15.616948876995593
b	15.963800825877115
b	15.977134739980102
b	15.620980642037466
b	15.675114969955757

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

[["ssl_startup",
  [[15.832569818012416,
    15.803461923962459,
    15.811633185017854,
    15.771200725110248,
    15.769214095082134],
   [15.616948876995593,
    15.963800825877115,
    15.977134739980102,
    15.620980642037466,
    15.675114969955757]]]]

Elapsed time: 157.843995469 (sec)
-----------------------------------------------------------
benchmark results:
minimum results in each 5 measurements.
Execution time (sec)
name	a	b
ssl_startup	15.769	15.617

Speedup ratio: compare with the result of `a' (greater is better)
name	b
ssl_startup	1.010
---
 ext/openssl/lib/openssl/ssl.rb |  48 ++++++++++++++++++++
 ext/openssl/ossl_ssl.c         | 101 ++++++++++++++---------------------------
 2 files changed, 81 insertions(+), 68 deletions(-)

diff --git a/ext/openssl/lib/openssl/ssl.rb b/ext/openssl/lib/openssl/ssl.rb
index d3ae155..b273173 100644
--- a/ext/openssl/lib/openssl/ssl.rb
+++ b/ext/openssl/lib/openssl/ssl.rb
@@ -322,6 +322,54 @@ def session
         nil
       end
 
+      # call-seq:
+      #    ssl.accept_nonblock([options]) => self
+      #
+      # Initiates the SSL/TLS handshake as a server in non-blocking manner.
+      #
+      #   # emulates blocking accept
+      #   begin
+      #     ssl.accept_nonblock
+      #   rescue IO::WaitReadable
+      #     IO.select([s2])
+      #     retry
+      #   rescue IO::WaitWritable
+      #     IO.select(nil, [s2])
+      #     retry
+      #   end
+      #
+      # By specifying `exception: false`, the options hash allows you to
+      # indicate that accept_nonblock should not raise an IO::WaitReadable or
+      # IO::WaitWritable exception, but return the symbol :wait_readable or
+      # :wait_writable instead.
+      def accept_nonblock(exception: true)
+        __accept_nonblock(exception)
+      end
+
+      # call-seq:
+      #    ssl.connect_nonblock([options]) => self
+      #
+      # Initiates the SSL/TLS handshake as a client in non-blocking manner.
+      #
+      #   # emulates blocking connect
+      #   begin
+      #     ssl.connect_nonblock
+      #   rescue IO::WaitReadable
+      #     IO.select([s2])
+      #     retry
+      #   rescue IO::WaitWritable
+      #     IO.select(nil, [s2])
+      #     retry
+      #   end
+      #
+      # By specifying `exception: false`, the options hash allows you to
+      # indicate that connect_nonblock should not raise an IO::WaitReadable or
+      # IO::WaitWritable exception, but return the symbol :wait_readable or
+      # :wait_writable instead.
+      def connect_nonblock(exception: true)
+        __connect_nonblock(exception)
+      end
+
       private
 
       def using_anon_cipher?
diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index 553eb14..5018dcb 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1271,14 +1271,19 @@ no_exception_p(VALUE opts)
     return 0;
 }
 
+/*
+ * nbex -
+ * * undef - blocking
+ * * true - non-blocking, raise exceptions on common errors
+ * * false - non-blocking, no exceptions on common errors
+ */
 static VALUE
-ossl_start_ssl(VALUE self, int (*func)(), const char *funcname, VALUE opts)
+ossl_start_ssl(VALUE self, int (*func)(), const char *funcname, VALUE nbex)
 {
     SSL *ssl;
     rb_io_t *fptr;
     int ret, ret2;
     VALUE cb_state;
-    int nonblock = opts != Qfalse;
 
     rb_ivar_set(self, ID_callback_state, Qnil);
 
@@ -1297,15 +1302,23 @@ ossl_start_ssl(VALUE self, int (*func)(), const char *funcname, VALUE opts)
 
 	switch((ret2 = ssl_get_error(ssl, ret))){
 	case SSL_ERROR_WANT_WRITE:
-            if (no_exception_p(opts)) { return sym_wait_writable; }
-            write_would_block(nonblock);
-            rb_io_wait_writable(FPTR_TO_FD(fptr));
-            continue;
+	    switch (nbex) {
+	    case Qfalse: return sym_wait_writable;
+	    case Qundef:
+		rb_io_wait_writable(FPTR_TO_FD(fptr));
+		continue;
+	    default:
+		write_would_block(1);
+	    }
 	case SSL_ERROR_WANT_READ:
-            if (no_exception_p(opts)) { return sym_wait_readable; }
-            read_would_block(nonblock);
-            rb_io_wait_readable(FPTR_TO_FD(fptr));
-            continue;
+	    switch (nbex) {
+	    case Qfalse: return sym_wait_readable;
+	    case Qundef:
+		rb_io_wait_readable(FPTR_TO_FD(fptr));
+		continue;
+	    default:
+		read_would_block(1);
+	    }
 	case SSL_ERROR_SYSCALL:
 	    if (errno) rb_sys_fail(funcname);
 	    ossl_raise(eSSLError, "%s SYSCALL returned=%d errno=%d state=%s", funcname, ret2, errno, SSL_state_string_long(ssl));
@@ -1329,40 +1342,15 @@ ossl_ssl_connect(VALUE self)
 {
     ossl_ssl_setup(self);
 
-    return ossl_start_ssl(self, SSL_connect, "SSL_connect", Qfalse);
+    return ossl_start_ssl(self, SSL_connect, "SSL_connect", Qundef);
 }
 
-/*
- * call-seq:
- *    ssl.connect_nonblock([options]) => self
- *
- * Initiates the SSL/TLS handshake as a client in non-blocking manner.
- *
- *   # emulates blocking connect
- *   begin
- *     ssl.connect_nonblock
- *   rescue IO::WaitReadable
- *     IO.select([s2])
- *     retry
- *   rescue IO::WaitWritable
- *     IO.select(nil, [s2])
- *     retry
- *   end
- *
- * By specifying `exception: false`, the options hash allows you to indicate
- * that connect_nonblock should not raise an IO::WaitReadable or
- * IO::WaitWritable exception, but return the symbol :wait_readable or
- * :wait_writable instead.
- */
 static VALUE
-ossl_ssl_connect_nonblock(int argc, VALUE *argv, VALUE self)
+ossl_ssl_connect_nonblock(VALUE self, VALUE ex)
 {
-    VALUE opts;
-    rb_scan_args(argc, argv, "0:", &opts);
-
     ossl_ssl_setup(self);
 
-    return ossl_start_ssl(self, SSL_connect, "SSL_connect", opts);
+    return ossl_start_ssl(self, SSL_connect, "SSL_connect", ex);
 }
 
 /*
@@ -1377,40 +1365,15 @@ ossl_ssl_accept(VALUE self)
 {
     ossl_ssl_setup(self);
 
-    return ossl_start_ssl(self, SSL_accept, "SSL_accept", Qfalse);
+    return ossl_start_ssl(self, SSL_accept, "SSL_accept", Qundef);
 }
 
-/*
- * call-seq:
- *    ssl.accept_nonblock([options]) => self
- *
- * Initiates the SSL/TLS handshake as a server in non-blocking manner.
- *
- *   # emulates blocking accept
- *   begin
- *     ssl.accept_nonblock
- *   rescue IO::WaitReadable
- *     IO.select([s2])
- *     retry
- *   rescue IO::WaitWritable
- *     IO.select(nil, [s2])
- *     retry
- *   end
- *
- * By specifying `exception: false`, the options hash allows you to indicate
- * that accept_nonblock should not raise an IO::WaitReadable or
- * IO::WaitWritable exception, but return the symbol :wait_readable or
- * :wait_writable instead.
- */
 static VALUE
-ossl_ssl_accept_nonblock(int argc, VALUE *argv, VALUE self)
+ossl_ssl_accept_nonblock(VALUE self, VALUE ex)
 {
-    VALUE opts;
-
-    rb_scan_args(argc, argv, "0:", &opts);
     ossl_ssl_setup(self);
 
-    return ossl_start_ssl(self, SSL_accept, "SSL_accept", opts);
+    return ossl_start_ssl(self, SSL_accept, "SSL_accept", ex);
 }
 
 static VALUE
@@ -2287,9 +2250,11 @@ Init_ossl_ssl(void)
     rb_define_const(mSSLExtConfig, "OPENSSL_NO_SOCK", Qfalse);
     rb_define_alloc_func(cSSLSocket, ossl_ssl_s_alloc);
     rb_define_method(cSSLSocket, "connect",    ossl_ssl_connect, 0);
-    rb_define_method(cSSLSocket, "connect_nonblock",    ossl_ssl_connect_nonblock, -1);
+    rb_define_private_method(cSSLSocket, "__connect_nonblock",
+			     ossl_ssl_connect_nonblock, 1);
     rb_define_method(cSSLSocket, "accept",     ossl_ssl_accept, 0);
-    rb_define_method(cSSLSocket, "accept_nonblock", ossl_ssl_accept_nonblock, -1);
+    rb_define_private_method(cSSLSocket, "__accept_nonblock",
+			     ossl_ssl_accept_nonblock, 1);
     rb_define_method(cSSLSocket, "sysread",    ossl_ssl_read, -1);
     rb_define_private_method(cSSLSocket, "sysread_nonblock",    ossl_ssl_read_nonblock, -1);
     rb_define_method(cSSLSocket, "syswrite",   ossl_ssl_write, 1);
-- 
EW


                 reply	other threads:[~2015-12-02  2:06 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151202020654.18328-1-e@80x24.org \
    --to=e@80x24.org \
    --cc=spew@80x24.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).