JS-friendly Deferred.t

[mitschabaude]

This PR introduces a wrapper library called Promise which has a native and a JS implementation: The native version is just an alias for Async_kernel.Deferred, while the JS version is implemented using JS promises. The mli file common to both versions is designed to expose all the functionality we need from Async_kernel.Deferred.

This strategy bypasses the need for running a Scheduler in JS applications and exposes async operations in a way that is very natural to JS. It fixes the problem that currently, in JS, Deferreds never return.

Within kimchi_bindings and pickles, we use the new Promise.t instead of Deferred.t, so e.g. pickles' async provers can be used from JS. Within libraries not used in JS that depend on pickles, like transaction_snark, we just convert back to a Deferred.t, to keep the diff small.

TODO: So far, this strategy is only applied to the provers returned by pickles. It still has to be done for verify.

[mitschabaude]

Not ready to review yet - there is still an issue with promise handling

[bkase]

Are we breaking any contracts of Deferred to have things automatically start executing without starting a scheduler? I think @nholland94 is familiar with those contracts.

Perhaps we want the JS implementation to be unit -> Js.Promise so that we can explicitly start it in the same way that Deferreds are started.

[mitschabaude]

Are we breaking any contracts of Deferred to have things automatically start executing without starting a scheduler? I think @nholland94 is familiar with those contracts.

Perhaps we want the JS implementation to be unit -> Js.Promise so that we can explicitly start it in the same way that Deferreds are started.

@bkase, AFAIU, once a scheduler is up and running, Deferreds don't have to be started explicitly. They are just values stored in cells that are empty at the beginning and get "filled" eventually, and the scheduler has the task of running the operations that are associated with that value getting filled, i.e. call the listeners. In JavaScript, the built-in JS event loop plays that role of the scheduler. So in my view, in JS the contract is now equivalent to the native side when it starts a scheduler right at the beginning, which I think is fine. (We could potentially implement it in a way so there has to be an explicit global "start_scheduler" operation before Deferreds run.. I don't see the value of that, IMO that makes it harder to use)

[mitschabaude]

This is ready for review now!
I fixed a remaining problem (Promise.bind wasn't implemented correctly). On the snarkyjs branch, which also includes these changes, I'm now able to obtain the pickles proof inside a promise in JS!

[bkase]

This looks good to me pending the few questions I asked, and you're explanation makes sense, but would love someone familiar with the inner workings of Deferreds also on the review for this PR

[nholland94]

Won't this change the concurrency control flow quite a bit? I fear this could lead to a lot of bugs. IIRC, promise chains in javascript run as much code synchronously as possible until an I/O operation is hit (as javascripts builtin threading model is to only switch tasks for I/O and event handlers). If this understanding of javascript promises is correct, then you are essentially swapping out our concurrency model for something that works very differently, which will lead to hard to debug race conditions that only occur in the javascript environment.

[mrmr1993]

I know I suggested this, but I don't think it'll handle exceptions properly, since upon will have swallowed the exception. We probably need an upon_exn, and to call this to_deferred_exn.

[mitschabaude]

@mrmr1993 As currently implemented, when the promise rejects, this will throw an error, which cannot be caught in any way (because upon creates a new promise, which will reject when it's parent promise rejects, but the new promise doesn't get exposed so it can't be caught)

[mitschabaude]

So this is not ideal obiously, but I'm wondering: How are exceptions in deferreds usually handled? I'd want to emulate that

[mrmr1993]

They get caught by the scheduler, so the point at which we attach the promise to the scheduler feels like the right time to raise. The current implementation is really upon_exn (which I missed in review), and the real upon can be

function deferred_upon(deferred, func) {
  deferred.promise.then(function () {
    func(deferred.value);
  }).catch(function(){});
}

[mitschabaude]

I changed the upon_exn / upon thing but I'm hesitant to call this function to_deferred_exn, since to_deferred can't throw an exception in native ocaml and this JS version was actually just supposed to be a dummy function (we don't have a way of using Deferred in JS after all). So I'd say this function, with upon called upon_exn, is "good enough" for now

[mrmr1993]

Won't this change the concurrency control flow quite a bit? I fear this could lead to a lot of bugs. IIRC, promise chains in javascript run as much code synchronously as possible until an I/O operation is hit (as javascripts builtin threading model is to only switch tasks for I/O and event handlers). If this understanding of javascript promises is correct, then you are essentially swapping out our concurrency model for something that works very differently, which will lead to hard to debug race conditions that only occur in the javascript environment.

@nholland94 I think this is important for us to be careful around in general, but I don't think we need to worry here. Concretely, the only Deferred.ts that we end up interacting with are created out of foreign functions (which will be implemented in javascript for SnarkyJS, designed to interact with the JS promise model), and so they will function as intended.

The other option would be to more-closely simulate the async scheduler, by changing all of the .then(foo) calls to

.then(function(input){
    return new Promise(function(resolve) {
        setTimeout(function() { resolve(foo(input)); }, 0);
    });
})

although this feels like overkill to me.

[mitschabaude]

@mrmr1993 Yeah, that version would lift Promise execution out of the the current microtask queue, into the macrotask queue. But as I discussed with Nathan in Slack, the current version already doesn't execute them synchronously, but queues them as a microtask