EventLoop preference overhaul

[weissi]

Motivation:

With #96 we changed HTTPClient.Task's eventLoop property to be currentEventLoop because we anticipated that it might change because of the connection pool work that's ongoing.
That however made the programming model rather hard because the user didn't get any guarantee anymore on which EventLoop the delegate would run.
With this patch, we're bringing eventLoop back which is the EventLoop the delegate will run on. Note that it's not necessarily the EventLoop the Channel will run on. If the user requires those to be the same, the can request that with EventLoopPreference.delegateAndChannel(on: eventLoop) rather than .indifferent or .delegate(on: eventLoop)

Modification:

Let the user select between:

• .indifferent: I don't care about the EventLoop
• .delegate(on: eventLoop): I want the delegate to be called on eventLoop and I would prefer the Channel to be on eventLoop too but I'm happy if not
• .delegateAndChannel(on: eventLoop): I want Channel and delegate to live on eventLoop

Result:

• Happier users because the programming model is easier
• fixes programming model questions #100

[weissi]

CC @adtrevor

[PopFlamingo]

Maybe there should also be a public accessible property to the channel's EventLoop?

[weissi]

What would the use-case be?

[weissi]

Happy to add now or later if we feel there’s a compelling-enough reason

[PopFlamingo]

I was thinking about the backpressure futures of the delegate: performance wise, it's best to return one from the same event loop as the channel, which wouldn't be possible anymore if there is no access to the channel event loop.

[weissi]

I'm inclined to say that if that really matters much, the user should've chosen .delegateAndChannel(on: ...) and therefore the channel's EL == delegate's EL. @Lukasa what do you think?

[Lukasa]

I think it’s reasonable to be able to ask for the delegate loop.

[PopFlamingo]

@Lukasa The delegate EventLoop should already be accessible with the eventLoop property, but I was thinking about the channel's EventLoop.

To give a more concrete example of what worries me, if you look at what happens in the ResponseAccumulator, the newest changes are doing this:

public func didReceiveHead(task: HTTPClient.Task<Response>, _: HTTPResponseHead) -> EventLoopFuture<Void> {
    return task.eventLoop.makeSucceededFuture(())
}

If no event loop preference is set, task.eventLoop can be completely different from the channel event loop.

But this means that in this part:

self.delegate.didReceiveBodyPart(task: self.task, body)
                    .hop(to: context.eventLoop)

The cost of hopping will be quite big especially something that's going to be called so much often.

It's true that for some uses .delegateAndChannel(on: ...) would probably be the best options, but for the delegates like ResponseAccumulator this isn't needed and yet being unable to access the channel el would loose performance.

[PopFlamingo]

On the other hand if letting public access to the channel event loop risks introducing too many errors on lib user side, maybe we could actually allow returning nil for the delegates that don't need backpressure? This would allow the client to choose the right event loop to get the best performance.

[Lukasa]

Yeah, I don't think it's worth exposing the channel event loop at this time. I understand the concern, and I think we can address it in the future, but for now I don't think I'd really bother. The performance concern is not really large enough (IMO) to justify it right now.

[weissi]

@adtrevor if you really need that kind of performance, I'd say just use .delegateAndChannel(on: ...) and then the delegate and the Channel's EL are guaranteed to be the same. If we truly find that there are important use-cases to get the Channel's El we can, as Cory points out, add that at any point without breaking API.

[weissi]

@artemredkin I fixed the merge conflicts now.

[artemredkin]

Do you want to merge of wait for @Lukasa's approval?