How to use EventLoopFuture in Swift properly?
Asked Answered
I

1

8

I'm new to EventLoop futures and promises. My software stack:

  • Backend in Go + gRPC
  • iOS client in Swift + SwiftUI + GRPC + NIO

I've got something to work and looking for suggestions on how to improve it since I'm a little lost in docs around .map, .flatMap, .always, etc.

Here's a relevant function from my gRPC data singleton within iOS app:

import Foundation
import NIO
import GRPC

class DataRepository {
    static let shared = DataRepository()
    // skip ...

    func readItem(id: Int64, eventLoop: EventLoop) -> EventLoopFuture<V1_ReadResponse> {
        // TODO: Is this the right place?
        defer {
            try? eventLoop.syncShutdownGracefully()
        }

        let promise = eventLoop.makePromise(of: V1_ReadResponse.self)

        var request = V1_ReadRequest()
        request.api = "v1"
        request.id = id

        let call = client.read(request, callOptions: callOptions) // client - GRPCClient initialized in the singleton

        call.response.whenSuccess{ response in
            return promise.succeed(response)
        }

        call.response.whenFailure{ error in
            return(promise.fail(error))
        }

        return promise.futureResult
    }

My code in SwiftUI View:

import SwiftUI
import NIO

struct MyView : View {
    @State private var itemTitle = "None"

    var body: some View {
        Text(itemTitle)
    }

    func getItem() {
        let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
        let result = DataRepository.shared.readItem(id: 1, eventLoop: eventLoopGroup.next())

        _ = result.always { (response: Result<V1_ReadResponse, Error>) in

            let res = try? response.get()                
            if let resExist = res {
                self.itemTitle = resExist.item.title
            }

            _ = response.mapError{ (err: Error) -> Error in
                print("[Error] Connection error or item not found: \(err)")
                return err
            }
        }
    }

Should I refactor getItem and/or readItem? Any specific suggestions?

Illailladvised answered 30/11, 2019 at 18:26 Comment(0)
W
19

I have one very concrete suggestion, followed by a few general ones. The first, most concrete suggestion is that if you aren't writing NIO code, you probably don't need to create EventLoops at all. grpc-swift will create its own event loops, and you can simply use those for callback management.

This lets you refactor your readItem code to simply be:

func readItem(id: Int64, eventLoop: EventLoop) -> EventLoopFuture<V1_ReadResponse> {
    var request = V1_ReadRequest()
    request.api = "v1"
    request.id = id

    let call = client.read(request, callOptions: callOptions) // client - GRPCClient initialized in the singleton
    return call.response
}

This is a great simplification of your code, and lets you defer essentially all of the complicated work of managing event loops to grpc-swift, so you can worry about your application code.

Otherwise, here are a few general suggestions:

Don't shut down event loops you don't own

At the top of readItem you have this code:

// TODO: Is this the right place?
defer {
    try? eventLoop.syncShutdownGracefully()
}

You can see I deleted it in my example above. That's because this is not the right place for this. Event loops are long-lived objects: they are expensive to construct and shut down, so you want to do that very rarely. In general, you want your event loops to be constructed and owned by a fairly high level object (like an AppDelegate, or some high-level view controller or View). They will then be "borrowed" by other components in the system. For your specific app, as I've noted, you probably want your event loops to be owned by grpc-swift, and so you shouldn't be shutting down any event loops, but in general if you take this policy, then a clear rule applies: if you didn't create the EventLoop, don't shut it down. It's not yours, you're just borrowing it.

In fact, in NIO 2.5.0 the NIO team made it an error to shut down an event loop in this way: if you replaced try? with try! you'd be seeing crashes in your application.

EventLoopGroups are top-level objects

In your MyView.getItem function you create a MultiThreadedEventLoopGroup. My advice above is for you to not create your own event loops at all, but if you're going to, this isn't a great place to do so.

For SwiftUI, the best thing to do is to have your EventLoopGroup be an EnvironmentObject that is injected into the view hierarchy by the AppDelegate. Every view that needs an EventLoopGroup can then arrange to extract one from the environment, which allows you to share the event loops across all views in your application.

Thread safety

EventLoopGroups use a private pool of their own threads to execute callbacks and application work on. In getItem you modify the view state from within one of these future callbacks, instead of from the main queue.

You should use DispatchQueue.main.async { } to rejoin the main queue before modifying your state. You may want to wrap this in a helper function like this:

extension EventLoopFuture {
    func always<T>(queue: DispatchQueue, _ body: (Result<T>) -> Void) {
        return self.always { result in
            queue.async { body(result) }
        }
    }
}

Callback refactoring

As a minor quality of life thing, this block of code could be refactored from:

let res = try? response.get()                
if let resExist = res {
    self.itemTitle = resExist.item.title
}

_ = response.mapError{ (err: Error) -> Error in
    print("[Error] Connection error or item not found: \(err)")
    return err
}

to this:

switch response {
case .success(let response):
    self.itemTitle = response.item.title
case .failure(let err):
    print("[Error] Connection error or item not found: \(err)")
}
Widow answered 9/12, 2019 at 9:20 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.