fix try_flush_interval < 1 and queued_chunk_flush_interval < 1 to work

[sonots]

We were supposed to support try_flush_interval < 1 and queue_chunk_flush_interval < 1, but the interval to call try_flush was still minimum 1 sec because Engine.now is Time.now.to_i.

Illustration: The problem of the old logic:

        until @finish
          time = Engine.now

          if @next_time <= time # (4) have to wait to be 1427488240.1 <= 1427488241
            @mutex.unlock
            begin
              @next_time = @output.try_flush # (1) 1427488240.1
            ensure
              @mutex.lock
            end
            next_wait = @next_time - Engine.now # (2) 0.1
          else
            next_wait = @next_time - time
          end

          # (3) will wait 0.1 sec
          cond_wait(next_wait) if next_wait > 0
        end

@next_time can be like 1427488240.1 when queued_chunk_flush_interval 0.1.
But, time exceeds the @next_time only when it became 1427488241 (1 sec passed) because Engine.now is integer.

[sonots]

cc: @shyouhei

[shyouhei]

What about just directly use Time.now + 1.0. Time objects can be compared or subtracted as-is.

irb(main):001:0> next_time = Time.now + 1.0
=> 2015-03-28 11:24:03 +0900
irb(main):002:0> next_wait = next_time - Time.now
=> 0.719691

[sonots]

float + float is faster than time + float, and time < float results in error

irb(main):001:0> Time.now < Time.now.to_f
ArgumentError: comparison of Time with 1427512006.7472174 failed
        from (irb):1:in `<'

but I think we can change it to time object here. let me try.

[sonots]

I made change like this to use time object > https://gist.github.com/sonots/3cd09b3022460ec1aac8

But, test did not pass. I felt it makes code complicated.
Also, note that float + float is faster than time + float.

My opinion is to use float rather than time object. Any opinions? > ALL

PS. Test passed with this additional change, though > https://gist.github.com/sonots/3cd09b3022460ec1aac8#comment-1422381

[shyouhei]

OK. I now feel that changing to Time objects here is kinda overkill to this original issue. It is a good thing to have, but we can live without them.

[sonots]

@repeatedly ping

[shyouhei]

👍 LGTM

[sonots]

I am going to merge this if there is no objection because this is critical for us.

[sonots]

Merged!

[sonots]

cherry-picked to v0.10

[repeatedly]

I hope code itself is no problem.
For safety, adding new method like Engine.now is better because refactoring Time.now.to_f is hard for sub-second support.

[shyouhei]

Thank you.

In case you plan to refactor, I recommend you to just use Time.now directly as I wrote earlier. That should suffice every needs, if not that is a ruby's design flaw.