Fix linking timeout

[ioquatix]

Let me know if you would like me to make a PR directly against timers - or perhaps you can do it. Let me know what you think of this solution.

[tarcieri]

I'd suggest using the actor's @timers and using #current_offset to recalculate the timeout

[ioquatix]

Okay, I will try that.

[ioquatix]

I haven't updated the code since I wanted to see if the fixes were working upstream, and here are the results: https://travis-ci.org/ioquatix/rubydns/jobs/26184701

So basically RubyDNS now works very nicely on Celluloid - faster than Bind9 for the specific case I'm testing too!

[tarcieri]

@ioquatix woohoo! 🎉

[ioquatix]

Okay, so I'm looking at the other options to implement this and trying to justify one way or the other. I believe the current PR is appropriate, but the code should be integrated into 'timers' gem to reduce coupling between celluloid and hitimes.

Here is my reasoning to support the original PR:

a/ Anything that uses @timers and #current_offset is going to increase coupling between the timeout logic and the actor implementation. This makes it harder to test the behaviour in isolation (currently would be very easy to test Timers::Timeout).

b/ There are multiple places in the code where we need to easily and correctly handle exclusive timeouts. This isn't the only one from what I've seen. Having a class which implements this logic correctly and with a fool-proof API is useful. If it is tied to the implementation of Actor/@timers this is less flexible.

c/ Are there efficiency gains for integrating with the existing timer state? I thought about this too. Because it's an exclusive loop, we can only assume that the time computation is done once per iteration of the loop (e.g. a system call for the monotonic time). Whether we use @timers.current_offset or our own internal @interval in Timeout, we make the same number of system calls, so there is no practical difference in efficiency that I can tell.

Let me know your thoughts.

[ioquatix]

I think I mentioned on IRC, but I will add, 3 times "wall clock time" faster than event machine, by my rough estimations too.

EDIT: Need to do some more investigation on this claim, looks like it might still be a bit slower when running on Travis - but on my development machine it is faster :D

[tarcieri]

@ioquatix looks good, but I'd suggest moving Timers::Timeout to the timers gem

[ioquatix]

@tarcieri That was my original intention but I wanted to get it working before doing that.

Let me know if you would like me to make a PR directly against timers

[tarcieri]

Yeah, make a PR against timers, and I can release 2.1.0

[ioquatix]

Um, it would be good to fix the other issues in travis so the build status reflects the PR correctly 👍

[tarcieri]

ws