em-synchrony icon indicating copy to clipboard operation
em-synchrony copied to clipboard

EM-Synchrony doesn't work with Enumerator

Open prepor opened this issue 12 years ago • 8 comments

Hello! Simple case:

require 'em-synchrony'
EM.synchrony do
  start = Time.now
  enum = Enumerator.new do |y|
    3.times do |i|
      EM::Synchrony.sleep 100
      y.yield i
    end
  end
  puts enum.next
  puts "Time: #{Time.now - start}"
  EM.stop
end

And it will return value (nil) instantly, because Enumerator uses Fibers internally and we have yielded from its fiber in EM::Synchrony.sleep.

prepor avatar Mar 11 '12 15:03 prepor

Hmmm, wow.. I am really surprised this hasn't show up before! Nice one :-)

This seems to do the trick for me:

require 'em-synchrony'

EM.synchrony do
  start = Time.now

  enum = Enumerator.new do |y|
    3.times do |i|
        y << i
      end
  end

  enum.each do |v|
    puts v
    EM::Synchrony.sleep 1
  end

  puts "Time: #{Time.now - start}"
  EM.stop
end

# $> ruby test.rb 
# 0
# 1
# 2
# Time: 3.001568

igrigorik avatar Mar 12 '12 01:03 igrigorik

The actual problem was with slow IO in Enumerator. The usecase was something like

enumerator = Enumerator.new do |enum|
  ActiveRecordModel.some.query.each do |item|
    enum.yield item.title
  end
end
EM::Synchrony::FiberIterator.new(enumerator, 10).each do |title|
  # processing
end

And in EM::Iterator next is called on enumerators, so it fails

fl00r avatar Mar 12 '12 07:03 fl00r

In fact, there are more simple trick:


require 'em-synchrony'
EM.synchrony do
  start = Time.now
  enum = Enumerator.new do |y|
    3.times do |i|
      EM::Synchrony.sleep 1
      y.yield i
    end
  end
  enum.each { |i| puts i }
  puts "Time: #{Time.now - start}"
  EM.stop
end

So, rule is sound like this: "do not use em-sync in body of enumerator with #next"

prepor avatar Mar 12 '12 07:03 prepor

https://gist.github.com/2023354

funny-falcon avatar Mar 12 '12 17:03 funny-falcon

Another way:

ROOT = Fiber.current

EM.run do
  Fiber.new do
    en = Enumerator.new do |y|
      3.times do |i|
        f = Fiber.current
        EM.add_timer(1) { f.transfer }
        ROOT.transfer
        y << i
      end
    end
    puts en.next
    EM.stop
  end.resume
end

Fiber.yield in EM.Synchrony is always "ok, we can't do anything more in this fiber and must wait callback", so always transfer root fiber (with event loop) is ok, i think.

prepor avatar Mar 12 '12 18:03 prepor

@funny-falcon's patch seems to do the trick.. but I'm having a hard time trying to figure out the actual patch in there :-)

igrigorik avatar Mar 13 '12 05:03 igrigorik

So, do we want to merge any of this into master? If not, I'll close the bug.

igrigorik avatar Jun 18 '12 23:06 igrigorik

But bug are exist, but it is impossible to fix it in current EM-Syncrhony correctly.

prepor avatar Jun 21 '12 07:06 prepor