concurrent-ruby icon indicating copy to clipboard operation
concurrent-ruby copied to clipboard

Support Hash of Futures for Promises.zip

Open f3ndot opened this issue 7 years ago • 7 comments

Reading the guide for the Promises work in 1.1.x I came across the fantastic Concurrent::Promises.zip method.

It's interface, per the guide, expects an array of Futures and will return a final value of said array's values. This is beneficial, but what would make my (and I suspect many other's) code cleaner is to optionally accept a Hash of promises instead of an Array.

e.g.

map_of_work = {
  multiply: Concurrent::Promises.future { 3*2 },
  divide: Concurrent::Promises.future { 3/2 },
  add: Concurrent::Promises.future { 3 + 2 },
  subtract: Concurrent::Promises.future { 3 - 2 },
}
# => {:multiply=>#<Concurrent::Promises::Future:0x00007fc474fad8a0 pending>,
#     :divide=>#<Concurrent::Promises::Future:0x00007fc474fad008 pending>,
#     :add=>#<Concurrent::Promises::Future:0x00007fc474fa7d60 pending>,
#     :subtract=>#<Concurrent::Promises::Future:0x00007fc474fa6ca8 pending>}
Concurrent::Promises.zip(map_of_work).value!
# => {:multiply=>6,
#     :divide=>1,
#     :add=>5,
#     :subtract=>1}

Why? Because it's a common pattern in Javascript libraries[1][2], and thus my team.

Arrays are slightly more cumbersome where I have to remember that index 2 is always for the addition work. Should another developer prepend more Futures to the array, we must not forget to change all the indexes referenced thereafter. With hashes, an :add is always an :add.

f3ndot avatar Nov 07 '18 04:11 f3ndot

Forgot to mention that this pattern would apply to the errors section as well:

# ...
Concurrent::Promises.zip(map_of_work).result
# => [true,
#     {:multiply=>6,
#      :divide=>1,
#      :add=>5,
#      :subtract=>1},
#     {:multiply=>nil,
#      :divide=>nil,
#      :add=>nil,
#      :subtract=>nil}]

f3ndot avatar Nov 07 '18 06:11 f3ndot

That is definitely an interesting idea, thanks for sharing! I'll leave it as looking-for-contributor for the moment to see if anybody would be interested to implement it. If not I'll do it.

pitr-ch avatar Nov 08 '18 14:11 pitr-ch

Ok

OunmanNgampeerapong avatar Dec 20 '18 01:12 OunmanNgampeerapong

So I started working through a POC of this, trying to follow the documentation here for consistency as much as possible. Some notes:

  • Hard to tell the difference between a legit hash and an options hash, resulting alternatives for syntax are all ugly. I think the best is an explicit options hash, but I still hate it: Concurrent::Promises.zip(promise_hash, {})
  • For an initial implementation, I would handle a hash of only promises. No sub-hashes or arrays for values.
  • @f3ndot what do you mean by your second comment? At least as of 1.1.4, .result doesn't exist on Concurrent::Promise, where are you getting yours from?

I'll see about poking at it tomorrow, will post some code once I have a few good specs passing.

jamie avatar Jan 30 '19 06:01 jamie

@jamie remember ::Promise and ::Future are deprecated in favour of ::Promises::Future:

Promises is a new framework unifying former tools Concurrent::Future, Concurrent::Promise, Concurrent::IVar, Concurrent::Event, Concurrent.dataflow, Delay, and TimerTask of concurrent-ruby.

It's a gotcha that I've been bit by a few times.

C:\Users\Justin>gem install concurrent-ruby
Fetching: concurrent-ruby-1.1.4.gem (100%)
Successfully installed concurrent-ruby-1.1.4
Parsing documentation for concurrent-ruby-1.1.4
Installing ri documentation for concurrent-ruby-1.1.4
Done installing documentation for concurrent-ruby after 9 seconds
1 gem installed

C:\Users\Justin>irb
irb(main):001:0> require 'concurrent-ruby'
=> true
irb(main):002:0> Concurrent::Promises.future { 3*2 }
=> #<Concurrent::Promises::Future:0x00000000034356b8 pending>
irb(main):003:0> Concurrent::Promises.future { 3*2 }.result
=> [true, 6, nil]
irb(main):004:0> Concurrent::Promises.future { raise 'oh no' }.result
=> [false, nil, #<RuntimeError: oh no>]
irb(main):005:0>

f3ndot avatar Jan 30 '19 12:01 f3ndot

Ah, that explains it. I was grepping the code for def self.zip and only found the one in Promise. Concurrent::Promises::FactoryMethods defines zip_futures, and aliases that to zip. That code might be a bit more interesting to refactor hash support into...

jamie avatar Jan 30 '19 17:01 jamie

Thanks @jamie for looking into this. Please accept collaboration invitation, then I'll be able to assign the issue to you.

Concurrent::Promises is definitely the right place to add this. The deprecated implementation is maintained but it's not getting any new features.

I am trying to think about how do this flexibly and without too much added complexity to the existing implementation. Something as follows could work

{ a: future { 1 },
  b: future { 2 }
}.reduce(fulfilled_future({})) do |hf, (k, vf)|
  (vf & hf).then { |v, h| h.update k => v }
end.value! #=> { a: 1, b: 2 }

pitr-ch avatar Feb 02 '19 15:02 pitr-ch