Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

from-promise fails on JVM #32

Closed
jconti opened this issue Jul 21, 2017 · 9 comments
Closed

from-promise fails on JVM #32

jconti opened this issue Jul 21, 2017 · 9 comments
Assignees
Labels

Comments

@jconti
Copy link
Contributor

jconti commented Jul 21, 2017

Clojure 1.8, JRE 1.8, beicon 3.5.0

user> (def foo (promise))
#'user/foo
user> (do (future (do (Thread/sleep 20000) (deliver foo "foo!")))
          (rx/on-value (rx/from-promise foo) println))
ClassCastException clojure.core$promise$reify__7005 cannot be cast to java.util.concurrent.Future  beicon.core/from-promise (core.cljc:615)
@niwinz niwinz self-assigned this Jul 25, 2017
@niwinz niwinz added the bug label Jul 25, 2017
@jconti
Copy link
Contributor Author

jconti commented Jul 27, 2017

I was considering writing something for this, but after I read an unimplemented proposal for Clojure promises, I wondered if the disposable was very close to the INotify interface proposed. So while implementing java.util.concurrent.Future in a wrapper for the Clojure promise is ok, I wondered if turning await directly into a Flowable with a generate was a more direct path. In that way, when subscribed to, the generate function would block awaiting the promise, then deliver the result. Comments welcome, as I am not yet sure I understand all the cross-cutting issues.

@niwinz
Copy link
Member

niwinz commented Jul 27, 2017

The main problem of clojure promises, is that they are blocking and does not allow chain comuptations. This means that if you want to convert that promise to observable you will need a dedicated thread to block/wait until promise is resolved, and then emit that value to the observable.

This is because is completely notrecommeded work with clojure promises and convert them to observables.

@jconti
Copy link
Contributor Author

jconti commented Jul 27, 2017

One way to interface with promises would be to use the realized? call to determine if the promise had its value delivered, before dereferencing it.

In my particular case, http-kit provides an async client that delivers results via a promise. This allows http-kit to run the pool of threads which service the sockets, and provides a simple interface to the eventual delivery. This is a clean interface to an async library, so users of the library can either block or not block depending on their application. So I think the capability to directly use promises on the JVM would be a useful addition to beicon.

One way to make that work would be to have a background thread devoted to polling any promises that had turned into observables by using a call to realized?. When any of those promises becomes realized, the thread would deliver the realized value to the observable, and complete the observable. It could do this by calling the sink function provided by a call to create (with rx/end wrapping the value). The promise would then be removed from the list of promises being polled. All from-promise would do is add the promise to the polling list, along with a reference to the sink function provided by the call to the create call.

It may be that an object representing this "subscription" to a promise is then easily regarded as a disposable. Calling -cancel or dispose on it simply removes the promise from the background thread's polling list (before it is realized, it is a nop after results have been delivered).

Another implementation might be to just make a periodic timer observable that polls the promise and returns a single value when it is realized. That might be an easier implementation, but I am more uncertain how a lot of timers inside rx/java will perform.

@niwinz
Copy link
Member

niwinz commented Jul 28, 2017

All implementations will imply block a thread for some perior of time in order to deref the promise or polling it in a interval if it is realized. That is very inneficient; unfortunatelly if the library you are using return clojure promises you have no choice... If you want composable promises, look at promesa library that works in jvm and cljs.

Obviously I think we need to implement the from-promise and choice the better implementation for type of promise it can receive. The clojure native promise is not composable in an asynchronous way so converting it to observable will be very inneficient.

@jconti
Copy link
Contributor Author

jconti commented Jul 29, 2017

This is a good discussion for me. I already do handle this interface via callbacks and beicon.core/create. I was interested in doing a flowable that allowed "paging" requests, requests that get one page, then use the data in the page to create the very next request. I wanted to build this so I could handle large amounts of data slowly, without thrashing the GC.

The promise was the blocking interface that I would use inside the flowable's generate function. In that context, the blocking is desired, since that is what the generate function and the use case are about. The interesting discussion here distracted me from the original use apparently. But for the blocking case, http-kit's client returns a promise. This gives the caller the flexibility to choose when to block.

Looking at the original error message then, if the promise implemented the future interface, the correct behavior would actually occur. In fact, one could argue that would be a nice feature for http-kit to provide, to return something that implements the future interface. However, I think a helper that wrapped promises with the future interface would be all that is actually required. Just thinking out loud here:

(defn promise->future [p]
  (reify java.util.concurrent.Future
   (cancel [this _] false)
   (get [this] (deref p))
   (get [this tout unit] 
     (deref p (.convert  java.util.concurrent.TimeUnit/MILLISECONDS unit tout) :timeout))
   (isCancelled [this] false)
   (isDone [this] (realized? p)))

Would such a thing be sufficient for the clj implementation for beicon. On the JVM, promises and futures are inherently blocking. It may not be performant, but it might be exactly what a user is trying to achieve. Not sure if I believe my own argument here, but I am putting it out for further discussion.

@niwinz
Copy link
Member

niwinz commented Jul 29, 2017

The response is: yes, it should be enough with that.

But on the jvm, the futures are not inherently blocking, let see https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/CompletableFuture.html (that impl is used in promesa)

Blocking on beicon/create is not recommeded, if you want to handle that http requests efficiently, you should switch to use non blocking promise/future implementation or use http library that uses callbacks instead of blocking promises. This is because every time you make a http request, you will allocate a new thread or you will block the current one, any of that will ruin almost all the advantages of using Rx.

@jconti
Copy link
Contributor Author

jconti commented Jul 29, 2017

I think that is a reasonable perspective. Given that, should from-promise be removed from the JVM implementation of the library? Anyone coming from javascript or a language with true async behavior for their promises will be surprised.

@jconti
Copy link
Contributor Author

jconti commented Jul 29, 2017

I guess alternatively, it could be renamed from-future.

@niwinz
Copy link
Member

niwinz commented Aug 1, 2017

renamed to from-future

@niwinz niwinz closed this as completed Aug 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants