Re-Implementation + Provisional dataloader

[syrusakbary]

This branch updates promises with async and ultra-performant resolution and including a provisional dataloader.

Things to do before merging:

• [x] Have all the tests running successfully
• [x] Port Promise.all
• [ ] Add promise error tracking
• [x] Improve Dataloader testing
• [ ] Create a list of breaking changes
• [ ] Investigate into Cython performance
• [x] Update Readme's with the new API usage

[coveralls]

Coverage decreased (-6.9%) to 92.745% when pulling 1f103fdda286b077d95af2a626a4ac898dcd45b4 on async-dataloader into 9539d3bbdd280d0c4f57f89797858121eeceb996 on master.

[coveralls]

Coverage decreased (-6.8%) to 92.766% when pulling f98004e13ede45e3e462b3957e198ae63998273e on async-dataloader into 9539d3bbdd280d0c4f57f89797858121eeceb996 on master.

[LivInTheLookingGlass]

Pulled master after this merge and it looks like promise creation is still calling things synchronously. Am I missing something here?

[syrusakbary]

Hi @gappleto97!

I just added a feature so you can change the scheduler quite easily:

from promise import set_default_scheduler, ThreadScheduler

set_default_scheduler(ThreadScheduler())

Hope this helps! :)

[LivInTheLookingGlass]

Unfortunately not. It looks like the problem is stemming from promise.Promise._resolve_from_executor()

That method appears to ignore the scheduler logic entirely, and just make a direct call.

[syrusakbary]

Hi @gappleto97, that is the desired behavior.

The Promise executor should always be called in the same thread where you are creating the promise, is the then (onFulfilled, onRejected) method the one that should be resolved off the thread.

This is also reflected in the spec: https://promisesaplus.com/#point-34 https://promisesaplus.com/#point-67

For clarifying, here are some JS examples that demonstrate this behavior:

promise = new Promise(function (resolve, reject) {
    // This is executed immediately
    console.log("RESOLVED"));
    resolve(true);
})
console.log("AFTER PROMISE")

(this should output "RESOLVED" and then "AFTER PROMISE")

However, is the then execution is the one that is asynchronous:

p = new Promise(function (resolve, reject) {
    // This is executed immediately
    console.log("RESOLVED");
    resolve(true);
}).then(function(resolved) {
    // This is executed in a new thread after the call stack is empty
    console.log("CALLED THEN", resolved);
});
console.log("AFTER PROMISE");

(this should output ("RESOLVED", "AFTER PROMISE", "CALLED THEN true")).

[syrusakbary]

Some other projects, for achieve an "async" promise execution (not resolution), use the trick of creating a resolved promise and deferring the execution on the promise resolution:

resolvedPromise = Promise.resolve();
resolvedPromise.then(function() {
// This is called async
});

https://github.com/facebook/dataloader/blob/master/src/index.js#L210 https://github.com/apollographql/graphql-server/blob/master/packages/graphql-server-core/src/runQuery.ts#L55

[LivInTheLookingGlass]

Huh. That's surprising to me. I guess I never noticed because you can't sleep in JavaScript. On the other hand, now I don't know quite how to proceed.

Would you be willing to add a keyword for using the scheduler in this case? Another option would be to have a single modifier inheritor (ScheduledPromise) where that one method is replaced.

[LivInTheLookingGlass]

A motivation to do this in that making it asynchronous in JavaScript is truly easy with a setTimeout, but there is no equivalent tool here

[LivInTheLookingGlass]

So I guess what I'm asking for is the ability to do:

new Promise((accept, reject)=>{
    setTimeout(()=>{
        accept(1);
    }, 2000);
});

In your library this is currently not possible.

[syrusakbary]

Hi @gappleto97,

Something similar could be implemented with python promise :)

from threading import Timer
def set_timeout(fn, time):
    timer = Timer(time, fn)
    timer.start()

p = Promise(lambda accept, reject: set_timeout(lambda: accept(1), 2.0))

Does that work for your case?

[LivInTheLookingGlass]

It works, but it adds exactly the sort of overhead you were looking to avoid with this. At that point you might as well just start a thread in the first place.

[syrusakbary]

I understand your frustration here, but I think is worth to keep the current python closer to the current Promise specifications.

I think, for your case, might be worth for you to extend the current promise implementation enabling this behavior:

from threading import Thread

class ThreadedPromise(Promise):
    def __init__(self, executor):
        def threaded_executor(resolve, reject):
            thread = Thread(target=executor, args=(resolve, reject))
            thread.start()
        super(ThreadedPromise, self).__init__(threaded_executor)

t = ThreadedPromise(lambda resolve, reject: resolve(1))