Is `Future.map(on: worker)` intentionally synchronous?

[bmhatfield]

When working with Vapor, I recently was surprised to learn that Future.map was a synchronous method that returns a future. I had been under the mistaken impression that it executed the closure on the event loop.

Compare: https://github.com/vapor/core/blob/master/Sources/Async/Future%2BGlobal.swift#L11-L21

with .submit: https://github.com/apple/swift-nio/blob/ae13d135f79e7b60281f9bf4c1cf1b3341a7339f/Sources/NIO/EventLoop.swift#L268-L280

Just checking that this wasn't an oversight, because the two methods appear to be very close in implementation.

[bmhatfield]

PS: I'm happy to put up a PR to execute the promise on the event loop, but I wasn't sure what the intended behavior here was, hence this discussion issue first. If there's a better venue for this question, I am happy to redirect!

[MrMage]

Normally the block inside `map` only gets executed once the previous future is fulfilled, so in that case the block will be run on the event loop anyway, I guess? (Not sure hat happens if the previous future has already been fulfilled at the time when `map` is called, though. Also, I might be misunderstanding something.)

…

On 31. Jul 2018, at 15:28, Brian Hatfield ***@***.***> wrote: PS: I'm happy to put up a PR to execute the promise on the event loop, but I wasn't sure what the intended behavior here was, hence this discussion issue first. If there's a better venue for this question, I am happy to redirect! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[vzsg]

@MrMage There's no previous Future here, the static Future.map method creates a new Future from the result of a synchronous function call.

The question is valid. The only difference between the NIO and the Async functions is which thread the synchronous function is executed on: as noted, Async's map runs it on the caller thread (and returns a Future which is resolved on the specified event loop); NIO's submit pushes the invocation to the event loop thread as well.

[bmhatfield]

@MrMage: @vzsg has understood it right - I am asking about the static Future.map function, which I was using to create (simple) new futures until I learned that it doesn't actually execute that closure on the eventloop.

It's of course possible to use Promises directly to create new futures, but I found the static Future.map to be very convenient.

As mentioned - if it's an oversight, the PR here is very straightforward, should be backwards-compatible, and I am happy to make it.

[MrMage]

Sorry for jumping in, I should have looked at the code 😅 Didn't mean to detract, just wanted to be helpful :-)

[tanner0101]

@bmhatfield Future.map is meant as an easy way to wrap synchronous, throwing code into a Future. It's a tool to combat methods that return futures and throw. It is expected to be called when you are already on the event loop. Generally you should never not be on the event loop unless you are needing to interact with blocking code. In those special cases, you should create your own promises and background threads.

That said I don't think there would be a huge detriment to putting these calls onto the event loop. The only thing I would say is that we should only call EventLoop.submit if we are not already on the event loop. NIO has a static call for checking this on the EventLoop type.