Improve EventLoopFuture code

[Lukasa]

A lot of the EventLoopFuture code is quite prone to subtle correctness bugs. This happens for a bunch of reasons, most of them relating to the way the code has grown over time. While we've patched over them a bit, we should really take a step to clean this code up more profoundly, ideally one method at a time.

Here are some things we should aim to do:

• [ ] Make map faster.

  Right now map performs worse than many users would expect. This is because map is basically then with a call to eventLoop.newSucceededFuture. This means that a call to map forces allocation of a new EventLoopPromise and a new EventLoopFuture. While this is appealing from the perspective of correctness ("well, map is really just the combination of monadic uplift and bind, blah blah blah haskell") it discourages our internal code from using it for performance reasons.

  This allocation is fundamentally unnecessary, and a better internal API could draw a distinction between these two behaviours to improve the performance of map.

• [ ] Define a better internal API.

  Quite a few methods try to be fast by skipping the external API, not least because map is slow. This is problematic, because the internal API exposes all the warty insides of these types ("what's a CallbackList? Why do I need to return one? How is then implemented, and how do I approximate it?"). We should clean these interfaces up to make it easier to be confident that the code that uses them is correct. In particular, the way they interact with the thread-hopping behaviour of EventLoopFuture needs to be made extremely clear, so that we can validate that code that uses them is actually safe.

• [ ] More utility functions.

  While we have and<T, U> -> EventLoopFuture<(T, U)> and andAll<Void> -> EventLoopFuture<Void>, there are a few other primitives we should probably provide. In particular, it's easy enough to extend andAll<Void> to andAll<T> -> EventLoopFuture<[T]>, as well as provide a variant of andAll that fails slow instead of failing fast (e.g. collate<T> -> EventLoopFuture<[Result<T>]>).

Any other suggestions?

[karim-elngr]

About part 3 - More utility functions - extending andAll

Having both andAll<Void> -> EventLoopFuture<Void> and andAll<T> -> EventLoopFuture<[T]> would introduce ambiguity to the compiler causing compile errors. For example the compiler would complain about this code try EventLoopFuture<Void>.andAll(writeFutures, eventLoop: channel.eventLoop).wait() unless we do the following let someFuture: EventLoopFuture<Void> = EventLoopFuture<Void>.andAll(writeFutures, eventLoop: channel.eventLoop) and try someFuture.wait()

I would like to work on it, but I am not sure how to handle this as it breaks many test cases.

[Lukasa]

Yeah, that's a rough one. We could do EventLoopFuture.reduce<T>(futures:eventLoop:_ reducer:) -> EventLoopFuture<T> instead, I suppose, which allows us to recompose the Void implementation. The easier thing to do, though, is to just give it a different name, which fixes the compiler's misery. :wink:

[karim-elngr]

I think we also need a eventLoopFuture.thenCombine(other:bifunction) as currently the only way to combine the values of 2 futures and map it into a new value is through and followed by a map and that is really not good in terms of performance due to allocation of 2 new promises and a succeeded future.

[karim-elngr]

@Lukasa Would be ok if we add a new protocol Future to allow us to define protocol extensions on sequences of EventLoopFuture like this example:

protocol Monad {
    associatedtype valueType
    var value: valueType {get set}
}

struct AnyMonad<T> : Monad {
    typealias valueType = T
    var value: T

    init(_ value: T) {
        self.value = value
    }
}

extension Sequence where Element: Monad {
    func reduce<T>(_ initialResult: T, reducer: @escaping (T, Element.valueType) -> T) -> AnyMonad<T> {
        var value = reduce(initialResult) { (partialResult: T, nextElement: Element) -> T in
            return reducer(partialResult, nextElement.value)
        }
        return AnyMonad<T>(value)
    }
}

let monads = [AnyMonad<Int>(1), AnyMonad<Int>(2)]
monads.reduce(0, reducer: +)

Which would allow the caller to omit the prefix EventLoopFuture<SomeType>.

[Lukasa]

We certainly could, yeah. I’m a bit reluctant to, though, on the grounds that we can’t really prevent the user from implementing that protocol themselves, even though doing so would be useless.

That said, maybe that’s just ok. @weissi?

[weissi]

@karim-elngr your Monad protocol doesn't define a monad. In fact you can't define a very useful Monad (or Applicative) protocol in Swift as that requires higher kinded types.

[Mordil]

Has any more thought been put into how to support both a Void returning and [Value] returning overload set?

After doing #773 I've been thinking about this and have thought up some things.

Either: 1) No overload set, and users users add their own .map { _ in () } to all calls to get the old Void behavior 2) Providing an overload that returns ELF<Void> that almost guarantees all usages need additional type information to remove ambiguity such as ELF<Void>.whenAllSucceed(...)

But I'm thinking this is the better approach:

// fail fast
public static func afterAllSucceed(_ futures: [EventLoopFuture<Value>], on eventLoop: EventLoop) -> EventLoopFuture<Void>
public static func whenAllSucceed(_ futures: [EventLoopFuture<Value>], on eventLoop: eventLoop) -> EventLoopFuture<[Value]>

// fail slow
public static func afterAllComplete(_ futures: [EventLoopFuture<Value>], on eventLoop: EventLoop) -> EventLoopFuture<Void>
public static func whenAllComplete(_ futures: [EventLoopFuture<Value>], on eventLoop: EventLoop) -> EventLoopFuture<[Result<Value, Error>]>

There's some bikeshedding to do on whether to use whenAll or afterAll or andAll to be consistent with other methods like and that already exist.

I think we could revisit the entire names as a collection to be consistent - but I'd also understand of wanting to discuss just the current new methods.

As it stands, the prior art is:

• and combines two ELFs and returns a tuple of the results
• andAll "folds" an array of ELF<Void> into a single ELF that acts as a completion notification
• whenComplete returns a Result<T, Error>
  • a user can "split" the Result into just either the T or Error value with whenSuccess or whenFailure

[tanner0101]

@Mordil what do you think about something like this?

extension ELFuture where Value == Void {
  static func andAll(_ futures: [ELFuture<Void>], on: ...) -> ELFuture<Void>
  static func andAllComplete(_ futures: [ELFuture<Void>], on: ...) -> ELFuture<[Error?]>
}
extension ELFuture {
  static func andAll(_ futures: [ELFuture<Value>], on: ...) -> ELFuture<[Value]>
  static func andAllComplete(_ futures: [ELFuture<Value>], on: ...) -> ELFuture<[Result<Value, Error>]>
}

[Mordil]

@tanner0101 Thanks, that definitely helps and I think I hit an OK solution with all of NIO's prior art.

// fail fast
static func andAllSucceed(...) -> EventLoopFuture<Void>
static func whenAllSucceed(...) -> EventLoopFuture<[Value]>

// fail slow
static func andAllComplete(...) -> EventLoopFuture<Void>
static func whenAllComplete(...) -> EventLoopFuture<[Result<Value, Error>]>

These are implemented in #804 and #803