Consolidate IVar childen into one object

[pitr-ch]

IVar, Future, Promise, Probe have a lot in common. I think we could explore an way how to merge some or more into one class.

[chrisseaton]

Future and Promise possibly. Although the problem then is that people come to the library looking for promises and they only find futures, or the other way around. That goes against our policy of giving you the primitives you want without an opinion on which is best.

IVar and Future, I'm not so sure. IVar is a dataflow variable. Future is a computation that is also an IVar. One is data, the other is code. It seems like a very good thing to separate.

I don't know about Probe - never seen that one!

[dsisnero]

Right now Future and Promise are pretty much the same. Is there anything,
perhaps named differently, that you can use to complete a future like
scala's Promise. I think Future and Promise should be combined into
something but we need something else that works like Scala's promise.
http://danielwestheide.com/blog/2013/01/16/the-neophytes-guide-to-scala-part-9-promises-and-futures-in-practice.html
& http://docs.scala-lang.org/overviews/core/futures.html

On Wed, Jul 30, 2014 at 6:48 AM, Chris Seaton notifications@github.com
wrote:

Future and Promise possibly. Although the problem then is that people come
to the library looking for promises and they only find futures, or the
other way around. That goes against our policy of giving you the primitives
you want without an opinion on which is best.

IVar and Future, I'm not so sure. IVar is a dataflow variable. Future is a
computation that is also an IVar. One is data, the other is code. It seems
like a very good thing to separate.

I don't know about Probe - never seen that one!

—
Reply to this email directly or view it on GitHub
#139 (comment)
.

[jdantonio]

Promise was one of the first things I wrote. It was based heavily on JavaScript's Promise/A and Promise/A+ specifications. As this library has evolved it has definitely strayed from the original influence and taken on a life of its own--a life that looks increasingly like our Future. I can get behind the idea of finding another inspiration (such as Scala's promise, as @dsisnero suggested) and evolving our Promise in that direction.

[mighe]

A Probe is a small component used by channels to implement the #select method. It is not meant to be part of the public API, but - of course - it can be refactored if necessary

[jdantonio]

I've started a new issue to discuss the next release and have suggested a new Promise implementation in that thread.

[pitr-ch]

Hi, sorry for me being late in the discussion after starting it. I quickly created the issue in a hurry without sharing my thoughts right-away, sorry.

Probe - adds single method set_unless_assigned which completes the IVar only when not completed returning true/false if success/failure. My thought was to add that to IVar and remove Probe.

But mainly I was thinking about what @dsisnero already said, to separate future and promise in Scala terms allowing to have only single object representing future value (no matter the source) keeping the computation separate as @chrisseaton said.

I should have time soonish to look into it.

[pitr-ch]

Hi, so I finally had few evenings and I've put together a draft how it could look like. Please review and comment in #176, let me know what do you think about the new API.

[jdantonio]

I suggest we close this discussion and the associated PR #176 and possibly revisit in the future when we begin planning a 2.0 release.

These discussions have been a very interesting thought exercise, but they are all speculative. These changes would break many existing APIs, many of which have been in place since the earliest days of this gem (long before I invited additional maintainers). There is a great deal of API consistency because of Dereferenceable, Obligation, and Observable. The existing APIs are in use in production and seem to make our users happy. No one who is using this gem in production has requested these changes, but they have requested a stable 1.0 release.

I believe the best path forward for our users is to stabilize and optimize the gem with the current APIs and release 1.0.

[jdantonio]

I've been traveling and otherwise very busy lately so I hadn't seen this comment from @pitr-ch when I made my previous comment. So I wasn't aware that some of these ideas came from work being done in another project. We should definitely take that into account.

[iNecas]

Yes, as @pitr-ch already mentioned, the lack of reasonable chaining (the dataflows would seem to workaround the issue, but the price of a thread needed to be allocated is too high) is quite painful for us. In the previous implementation, we had this https://github.com/Dynflow/dynflow/blob/master/lib/dynflow/future.rb#L85-L92, and it was quite nice to work with that API.

In the Dynflow/dynflow#139, which is taking us from our custom implementations of futures and actors to concurrent-ruby, we hit several places where the original approach was needed, but IVar (which we choose as replacement for our custom-made future) were lacking the feature. For this, we needed either to rework some of our implementations to workaround this, or came with suboptimal alternatives with using dataflows iNecas/dynflow@6327a83. This commit is there just to make our tests green, but the whole PRs is not going to be merged until we have proper solution, that doesn't require a thread for every such a operation.

So we are considering two approaches now: either some alternative appears in #268 and we will use it as an experimental feature, or we will need to build it again in our repo. Of course we prefer the first option, for all the benefits of sharing the code with others.

Thanks for taking our needs into account!

[jdantonio]

@iNecas Thanks for the explanation! It seems to me that we should be able to add a chaining feature without completely throwing out and rebuilding the core of the library. PR #176 feels too much like nuking the entire site from orbit.

A couple of questions:

• Why does Promise not meet your need? It provides chaining, runs the entire tree/chain on a single executor/thread pool, and supports the same Obligation API that IVar does.
• A suggestion was made above to add set_unless_assigned from Probe to IVar. Would that address your need, or is that something completely different?
• Can you please describe the specific details of the feature you would like to see added? PR #176 completely rewrites everything and the discussion is a jumble of many great but unrelated ideas. I can't tell which part of that PR was intended to address your particular need and which is the general rewrite.
• Could we simply extend/update Promise to meet your chaining need?

I'm hopeful that we can extend the library to meet this particular use case since it's driven by production use. But I'm very leery of rewriting the core of the library just to support a single feature. The rewrite suggested in PR #176 seems like a reasonable discussion to have (preferably for a future 2.0 release) but I think we need to decouple that discussion from a discussion about chaining futures.

[pitr-ch]

Thanks @iNecas for detailed explanation.

• Problem with promise is that it cannot be constructed by one thread and then later fulfilled externally by another arbitrary thread as IVar can.
• This suggestion is to remove the Probe implementation since it adds only one method set_unless_assigned (maybe a better name try_set) which is useful on IVar too. And it's not related.
• The unification PR was not originally created to fix the issues which arise during porting of Dynflow. It just allowed me to realize that there are shortcomings to be improved and potential issues with it's usage (which we have later hit).
• Promise could be probably extended (and Future too), and it would work for us in Dynflow.
• Another issue we need to deal in concurrent-ruby (which we hit in Dynflow) is that Observer will fire only if is set before the event (IVar being set) which makes it hard to use.

So for Dynflow case, we are happy if we extend current implementations to support our use-case but we would be happier if we could have the new unified API (assuming that I'll get it done in April).

About the unification PR: The current objects are quite similar, some are almost just feature subsets of others. That did not seem to me like a good way to go so when I saw that it does not fit our needs It motivated me to create new unifying implementation which in the end can be used to replace Ivar, Future, Probe, Delay, Promise, ScheduledTask all under one "umbrella" implementation supporting all and allowing smooth interoperation. (Timer task could be also simplified with this dropping the allocated threads.) It will also work nicely with actors an_actor.ask(:message).then { |result| do_stuff } (or later maybe future { compute_message }.then_ask(actor).then {|result| p result}). Channel integration can be added just by implementing one Promise type, which would allow things like:

select(channel1).then { |v| puts v } 
future { 1 }.
    then_select(channel1, channel2).
    then {|number, channel_output| puts number + channel_output}

After I finish the current Actor updates I would like to try actors to on this framework too

# very roughly
def adding(actor, count)
  # using Algebrick pattern matching
  receive(actor, ~Symbol.to_m | ~String.to_m).receive(actor, ~Numeric).then do |symbol, number|
    puts result = count.send(symbol, number)
    adding actor, result
  end
end

which would make them even more Erlang-ish and add thing like parallel processing inside actor.

Anyway I think there is lot of benefits in this PR which are at first glance hidden. I need to finish it provide documentation and examples, then we should decide if it's worth the hassle to get it to 1.0 or if it goes to edge/?/? and waits for next major version. I'll be happy either way since it'll be released and available.

[iNecas]

@jdantonio I've put together more documentation on Dynflow side that might help understanding our use-case for the IVar here as well http://dynflow.github.io/documentation/#inner-world-communication.

As @pitr-ch mentioned, the current promise has the limitation of not being able to set the value outside the block passed at the beginning. If the limitation was not there, we would probably be just fine in this particular case.

The question now, when the limitation of the Promise was removed, what would be the difference between the IVar and the Promise. In general, I find it quite hard to understand when to use IVar, Future or Promise (they look almost the same to me). That was also one of the questions on this year's Fosdem after @pitr-ch talk. Therefore I support the move towards the unification, as I believe it would enhance the usability to concurrent-ruby.

[pitr-ch]

One more issue is in actors. I need to use IVar so I can set it but that removes the ability to chain other async events when the actor answers (since doing it with blocking is bad). Example: actor.ask(:message).then { |result| process result } which won't work since it's IVar not Promise.

[pitr-ch]

Sorry one more issue came to mind: the current objects share interface for value retrieval but the fulfillment differs so they are not easily interchangeable. If we use IVar and Promises in Dynflow we will have to keep checking the types to be able to set the value.

[jdantonio]

@iNecas Thank you for the link to the Dynflow documentation. I'll take a look at it.

@iNecas @pitr-ch I made a similar comment in #269, but have either of you experimented with having Promise extend IVar the way Future does? I haven't had a chance to work with the code, but I glanced at it the other day and it seemed like a possibility. If we updated Promise to extend IVar it might work much better for Dynflow, but also take a first step toward unification.

[iNecas]

Yes, the IVar as the Promise parent is the way to go for me and makes sense.