Nonblock without exception #166

HoneyryderChuck · 2016-01-08T17:28:38Z

Related to #139 .

Should boost performance, as exception handling is a quite expensive VM operation.

Left support for ruby 2.0.0, although the ball is on someone else to drop it.

…ypatch of .describe anymore

…one doesn't need to handle Wait* Exception (Closes celluloid#139)

…to nonblock_without_exception Conflicts: lib/celluloid/io/stream.rb spec/celluloid/io/actor_spec.rb spec/celluloid/io/dns_resolver_spec.rb spec/celluloid/io/mailbox_spec.rb spec/celluloid/io/ssl_server_spec.rb spec/celluloid/io/ssl_socket_spec.rb spec/celluloid/io/tcp_server_spec.rb spec/celluloid/io/tcp_socket_spec.rb spec/celluloid/io/udp_socket_spec.rb spec/celluloid/io/unix_server_spec.rb spec/celluloid/io/unix_socket_spec.rb spec/celluloid/io_spec.rb

digitalextremist · 2016-01-08T18:59:57Z

@TiagoCardoso1983 are there benchmarks available? I'm curious to see some

… and using it to encapsulate nonblock reads and writes inside the streams

HoneyryderChuck · 2016-01-08T19:42:14Z

@digitalextremist not for celluloid-io, but maybe @tarcieri has some, as the project README claims some numbers comparing it to eventmachine and node.js.

HoneyryderChuck · 2016-01-08T19:49:39Z

And then I looked at the benchmarks directory.... :)

with ruby 2.1.6

# master
Calculating -------------------------------------
               spawn       154 i/100ms
               calls       415 i/100ms
         async calls       165 i/100ms
-------------------------------------------------
               spawn     1607.1 (±4.4%) i/s -       8162 in   5.088789s
               calls     4123.4 (±14.7%) i/s -      20335 in   5.027527s
         async calls     8537.5 (±17.2%) i/s -      41415 in   5.012115s

#nonblock without exceptions
Calculating -------------------------------------                       
               spawn       155 i/100ms                                  
               calls       446 i/100ms                                  
         async calls       173 i/100ms                                  
-------------------------------------------------                       
               spawn     1650.4 (±7.1%) i/s -       8215 in   5.004736s 
               calls     4059.4 (±9.4%) i/s -      20516 in   5.094898s 
         async calls     8781.6 (±14.8%) i/s -      42904 in   5.001161s

There is improvement, although I was expecting more.

HoneyryderChuck · 2016-01-08T19:51:09Z

Then I looked at the benchmark file, and it's not doing any IO. Maybe we better wait for @tarcieri to answer.

tarcieri · 2016-01-08T19:58:38Z

I think the performance implications are going to vary widely depending on which Ruby VM you're on. Exceptional async I/O used to create a new exception object and mix IO::WaitReadable/WaitWritable into the instance on MRI 1.9.3/2.0, which busts the method cache. This was fixed in later versions, but these are also the versions that support this feature.

I don't have hard numbers.

HoneyryderChuck · 2016-01-08T20:26:14Z

actually I was wrong, celluloid-io doesn't have any benchmarks, reel does. @digitalextremist , it's your house, right?

digitalextremist · 2016-01-08T20:41:26Z

The Reel benchmarks are outdated badly if not irrelevant, unfortunately 👎

ioquatix · 2016-01-08T21:38:05Z

The traditional expectation that exception handling is expensive is true in many compiled languages since everything else is really fast, but in Ruby that may not hold true.

HoneyryderChuck · 2016-01-08T23:03:39Z

it is true, and by the reasons stated before. check performance difference between raise/resque and throw/catch, for example. This can be indeed a huge performance boost for IO scenarios.

tarcieri · 2016-01-09T02:28:06Z

@ioquatix the situation was actually a lot more dire than that in MRI for quite awhile, as the exceptions generated for EAGAIN events in the async I/O APIs mixed a module into the exception instance. In addition to creating a new metaclass each time an exception is thrown (allocating even more memory and increasing GC pressure that much more) this also busts the method cache, and in the versions of MRI that did this MRI only has a single global method cache. This results in the entire application running much slower because the class hierarchy changes each time one of these exceptions is thrown. It's pretty much the same thing I was describing in this blog post:

https://tonyarcieri.com/dci-in-ruby-is-completely-broken

I believe in 2.2 they added special exception classes (IO::EAGAINWaitReadable and IO::EAGAINWaitWritable) which already have IO::WaitReadable and IO::WaitWritable mixed in so that no longer happens (this is exactly what JRuby and Rubinius were already doing). Using exception: false at least avoids allocating objects for the exception and collecting the stack trace (as many more objects).

ioquatix · 2016-01-09T12:13:03Z

@tarcieri sounds like the whole thing is over engineered up the wazoo, your blog was informative :)

HoneyryderChuck · 2016-01-09T14:52:33Z

When handled in UDPSocket class, will probably solve #156

HoneyryderChuck · 2016-01-11T14:18:08Z

Just checked the documentation, it seems that the same usage of exceptions doesn't apply there...

HoneyryderChuck · 2016-01-11T14:28:58Z

Wrong again, apparently support was added in ruby 2.3.0: http://docs.ruby-lang.org/en/2.3.0/UDPSocket.html

…thod

HoneyryderChuck · 2016-01-16T00:46:32Z

is this good to merge? or is the build an artifact coming from celluloid behaving bad?

tarcieri · 2016-01-16T20:54:08Z

I don't think so. It doesn't look complete. Nowhere are you actually passing :exception => false to a read_nonblock or write_nonblock call...

…ket, as the arguments are directly passed and one should leverage it himself

HoneyryderChuck · 2016-01-17T00:08:31Z

ups, completely overlooked.

…rm dns queries (this one is only compatible with ruby 2.3.0)

tarcieri · 2016-01-18T17:49:55Z

lib/celluloid/io/dns_resolver.rb

@@ -42,7 +42,10 @@ def resolve(hostname)

        query = build_query(hostname)
        @socket.send query.encode, 0, @server.to_s, DNS_PORT
-        data, _ = @socket.recvfrom(MAX_PACKET_SIZE)
+        data, _ = RUBY_VERSION >= "2.3" ?


I mean, this definitely seems like the wrong place to put this check. In the DNS resolver? It should be using a Celluloid::IO::UDPSocket, and these details should be pushed down there.

…exception: false flag all the time

HoneyryderChuck · 2016-01-18T18:32:05Z

better?

tarcieri · 2016-01-18T18:36:45Z

lib/celluloid/io/stream.rb

@@ -80,6 +71,20 @@ def syswrite(string)
        total_written
      end

+      # TODO: remove after ending ruby 2.0.0 support
+      if RUBY_VERSION >= "2.1" 
+        def read_nonblock(*args, **options)


This changes the semantics of this method on Ruby >= "2.1" making it exceptionless by default and unilaterally (i.e. even if exception: true were passed)

I'd rather we preserve Ruby core semantics as much as possible.

How about an internal __read_nonblock method which is always exceptional on Ruby < 2.1 and always exceptionless on 2.1+?

I'll get back at this tomorrow. But isn't that what this method is doing already? No redefinition is happening under ruby < 2.1, it just delegates to the socket.

You are redefining it under Ruby 2.1+ such that:

The exception: ... parameter is completely ignored

The default is switched from exceptional to exceptionless

…lt, unless someone is messing with the :exception option

HoneyryderChuck · 2016-01-19T10:30:08Z

better? It seems that the default for ruby 2.1+ is still exceptionless. Isn't it the purpose? Because I don't see otherwise the condition to be used. Should I test if I'm inside an actor and then switch to exceptionless mode?

tarcieri · 2016-01-20T04:26:53Z

FWIW, we've received reports of problems in http.rb's timeout implementation which this PR is copypasta'd from, so I think there's probably some more work needed here:

httprb/http#298 (comment)

HoneyryderChuck · 2016-01-23T14:45:25Z

@tarcieri are you sure that it's about the exception handling? The problem in http may arise also from the io/wait API usage in favour of IO.select, as one favours poll() syscall over select(), and maybe the interval handling is different (?). possible guess.

tarcieri · 2016-01-23T17:28:14Z

@TiagoCardoso1983 we haven't figured out the exact cause yet

HoneyryderChuck · 2016-04-26T19:54:42Z

@tarcieri I just checked you reverted to using IO.select in http.rb . So, what is the gist? Unusable with timeouts? Funky with fd's other than network sockets? I had complaints on another project where using io/wait calls for IO.pipe fds has a different behaviour than using them with IO.select.

tarcieri · 2016-04-26T22:05:29Z

@TiagoCardoso1983 see this issue:

httprb/http#298

Switching from IO.select to io/wait has an indeterminate difference with respect to timeout behavior. They are not semantically equivalent.

I've been meaning to dig deeper into this issue but haven't yet. The result was a complete breakage of timeout handling.

HoneyryderChuck · 2016-04-27T05:58:54Z

I see. See net-ssh/net-ssh#303 (comment) for what I meant with API incompatibility. As I was investigating, io/wait uses poll API in Linux and falls back to select in MacOS and Windows, but I suspect that the calls semantics are different. A bit unfortunate IMO.

tarcieri · 2016-04-27T06:54:26Z

I'd like to pull an strace-style syscall profile against the two APIs to see what actually differs. Just haven't had the time yet.

Tiago Cardoso added 3 commits January 8, 2016 18:24

adding RSpec namespace (3.4.0) doesn't contain the global space monke…

21ddab6

…ypatch of .describe anymore

using the new read_ and write_nonblock signature (ruby 2.1) in which …

5321f59

…one doesn't need to handle Wait* Exception (Closes celluloid#139)

HoneyryderChuck force-pushed the nonblock_without_exception branch from 7a31f22 to 8098ebc Compare January 8, 2016 17:40

adapted the code from the http gem according to tarcieri's suggestion…

de71524

… and using it to encapsulate nonblock reads and writes inside the streams

moving the #perform_io to the socket base class, so that udp can use it

f4160d6

encapsulating nonblock usage for UDPSockets within the #perform_io me…

ecdd882

…thod

adding exception: false option to the nonblocking APIs, except UDPSoc…

13630b7

…ket, as the arguments are directly passed and one should leverage it himself

HoneyryderChuck mentioned this pull request Jan 18, 2016

Bump version to 0.17.3 and update CHANGES.md #168

Merged

adding the exception: false option when using the udp socket to perfo…

d21ac9c

…rm dns queries (this one is only compatible with ruby 2.3.0)

tarcieri reviewed Jan 18, 2016
View reviewed changes

Tiago Cardoso added 3 commits January 18, 2016 19:19

defining #recvfrom method in UDPSocket according to ruby version

b397086

redefining #read* and write_nonblock for ruby 2.1 or bigger; passing …

d621577

…exception: false flag all the time

adding WaitReadable condition to spec only when ruby under 2.1

f68e551

tarcieri reviewed Jan 18, 2016
View reviewed changes

Tiago Cardoso added 2 commits January 19, 2016 11:15

use Timeout.timeout, ruby 2.3 is deprecating Kernel.timeout

c85cac9

making nonblock calls within celluloid streams exceptionless by defau…

1e8b3d6

…lt, unless someone is messing with the :exception option

Merge branch 'master' into nonblock_without_exception

339b771

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nonblock without exception #166

Nonblock without exception #166

HoneyryderChuck commented Jan 8, 2016

digitalextremist commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

tarcieri commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

digitalextremist commented Jan 8, 2016

ioquatix commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

tarcieri commented Jan 9, 2016

ioquatix commented Jan 9, 2016

HoneyryderChuck commented Jan 9, 2016

HoneyryderChuck commented Jan 11, 2016

HoneyryderChuck commented Jan 11, 2016

HoneyryderChuck commented Jan 16, 2016

tarcieri commented Jan 16, 2016

HoneyryderChuck commented Jan 17, 2016

tarcieri Jan 18, 2016

HoneyryderChuck commented Jan 18, 2016

tarcieri Jan 18, 2016

HoneyryderChuck Jan 18, 2016

tarcieri Jan 18, 2016

HoneyryderChuck commented Jan 19, 2016

tarcieri commented Jan 20, 2016

HoneyryderChuck commented Jan 23, 2016

tarcieri commented Jan 23, 2016

HoneyryderChuck commented Apr 26, 2016

tarcieri commented Apr 26, 2016

HoneyryderChuck commented Apr 27, 2016

tarcieri commented Apr 27, 2016

Nonblock without exception #166

Are you sure you want to change the base?

Nonblock without exception #166

Conversation

HoneyryderChuck commented Jan 8, 2016

digitalextremist commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

tarcieri commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

digitalextremist commented Jan 8, 2016

ioquatix commented Jan 8, 2016

HoneyryderChuck commented Jan 8, 2016

tarcieri commented Jan 9, 2016

ioquatix commented Jan 9, 2016

HoneyryderChuck commented Jan 9, 2016

HoneyryderChuck commented Jan 11, 2016

HoneyryderChuck commented Jan 11, 2016

HoneyryderChuck commented Jan 16, 2016

tarcieri commented Jan 16, 2016

HoneyryderChuck commented Jan 17, 2016

tarcieri Jan 18, 2016

Choose a reason for hiding this comment

HoneyryderChuck commented Jan 18, 2016

tarcieri Jan 18, 2016

Choose a reason for hiding this comment

HoneyryderChuck Jan 18, 2016

Choose a reason for hiding this comment

tarcieri Jan 18, 2016

Choose a reason for hiding this comment

HoneyryderChuck commented Jan 19, 2016

tarcieri commented Jan 20, 2016

HoneyryderChuck commented Jan 23, 2016

tarcieri commented Jan 23, 2016

HoneyryderChuck commented Apr 26, 2016

tarcieri commented Apr 26, 2016

HoneyryderChuck commented Apr 27, 2016

tarcieri commented Apr 27, 2016