apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.88k stars 725 forks source link

Performance issues - OperationQueue not performing async #1807

Open victormihaita opened 3 years ago

victormihaita commented 3 years ago

Bug report

The network performance of the app is very bad. All the queries are performed correctly, but it looks like there is a queue for the operations and they are performed one by one (in serial) while I expect them to be done in parallel.

First I've tried to use the default init for the ApolloClient and after I've started to use the background thread for the operationQueue hoping that I will achieve the parallel execution of the queries and an improvement in the performance. I've attached the main part of the code used for the NEtworking so maybe someone can find out if I miss something.. It looks correct to be as everything works... but is just slooooow :(

Versions

Steps to reproduce

Here is the code I use to init the ApolloClient

private(set) lazy var store: ApolloStore = {
        guard let documentsPath = NSSearchPathForDirectoriesInDomains(
                .documentDirectory,
                .userDomainMask,
                true).first else {
            return ApolloStore(cache: InMemoryNormalizedCache())
        }
        let documentsURL = URL(fileURLWithPath: documentsPath)
        let fileURL = documentsURL.appendingPathComponent(AppConfiguration.current.sqliteURL)

        let cache = try? SQLiteNormalizedCache(fileURL: fileURL)
        return ApolloStore(cache: cache ?? InMemoryNormalizedCache())
    }()

    private lazy var sessionClient: URLSessionClient = {
        let config = URLSessionConfiguration.default
        config.timeoutIntervalForResource = 60
        config.timeoutIntervalForRequest = 60

        let queue = OperationQueue()
        queue.maxConcurrentOperationCount = 64
        queue.underlyingQueue = .global(qos: .userInteractive)

        return URLSessionClient(sessionConfiguration: config, callbackQueue: queue)
    }()

    private lazy var interceptor: NetworkInterceptorProvider = {
        return NetworkInterceptorProvider(client: sessionClient, store: store)
    }()

    private lazy var networkTransport: RequestChainNetworkTransport = {
        return RequestChainNetworkTransport(
            interceptorProvider: interceptor,
            endpointURL: AppConfiguration.current.apiURL
        )
      }()

    private(set) lazy var apollo = ApolloClient(networkTransport: networkTransport, store: store)

Here is the reactive implementation for fetch

public func fetch<T: GraphQLQuery>(_ query: T, cachePolicy: CachePolicy) -> Maybe<T.Data> {
        return Maybe.create { maybe in
            let cancellable = self.apollo.fetch(query: query, cachePolicy: cachePolicy) {
                switch $0 {
                case .success(let queryResult):
                    if let data = queryResult.data {
                        maybe(.success(data))
                    } else if let errors = queryResult.errors {
                        maybe(.error(RxApolloError.graphQLErrors(errors)))
                    } else {
                        maybe(.completed)
                    }
                case .failure(let error):
                    maybe(.error(error))
                }
            }
            return Disposables.create {
                cancellable.cancel()
            }
        }
    }

Here is my NetworkInterceptor implementation

open class NetworkInterceptorProvider: InterceptorProvider {

    private let client: URLSessionClient
    private let store: ApolloStore

    public init(client: URLSessionClient, store: ApolloStore) {
        self.client = client
        self.store = store
    }

    open func interceptors<Operation: GraphQLOperation>(for operation: Operation) -> [ApolloInterceptor] {
        return [
            MaxRetryInterceptor(),
            UserManagementInterceptor(),
            LegacyCacheReadInterceptor(store: store),
            NetworkFetchInterceptor(client: client),
            ResponseCodeInterceptor(),
            LegacyParsingInterceptor(cacheKeyForObject: store.cacheKeyForObject),
            AutomaticPersistedQueryInterceptor(),
            LegacyCacheWriteInterceptor(store: store),
            TokenErrorInterceptor()
        ]
    }

    open func additionalErrorInterceptor<Operation: GraphQLOperation>(
        for operation: Operation
    ) -> ApolloErrorInterceptor? {
        return nil
    }
}

Here is the interceptor used for setting the authentication token

class UserManagementInterceptor: ApolloInterceptor {
    public enum NetworkError: Error, LocalizedError {
        case offline

        public var errorDescription: String? {
            switch self {
            case .offline:
                return "Make sure your device is connected to the internet"
            }
        }
    }

    func interceptAsync<Operation: GraphQLOperation>(
        chain: RequestChain,
        request: HTTPRequest<Operation>,
        response: HTTPResponse<Operation>?,
        completion: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) {
        request.addHeader(name: "x-api-key", value: AppConfiguration.current.apiKey)
        if Connectivity().currentReachabilityStatus == .notReachable {
            chain.handleErrorAsync(NetworkError.offline, request: request, response: response, completion: completion)
        } else if ApiClient.shared.performUnauthenticatedRequest == true {
            chain.proceedAsync(request: request, response: response, completion: completion)
        } else if let accessToken = KeychainManager().get(from: .accessToken) {
            request.addHeader(name: "Authorization", value: "Bearer \(accessToken)")
            chain.proceedAsync(request: request, response: response, completion: completion)
        } else {
            chain.proceedAsync(request: request, response: response, completion: completion)
        }
    }
}

Further details

I also have a TokenErrorInterceptor which is used to catch errors and refresh the token but I didn't post the code as is not relevant for this issue...

designatednerd commented 3 years ago

@victormihaita There's a couple things I see in your setup of the operation queue: I see you've got the max concurrent operation count at 64 for the networking queue - that's a lot of operations to keep track of from a networking perspective. I would try setting that to maybe 4 or 8 and see if that helps.

You've also got that operation queue set up as using .userIneractive quality of service, which as those docs I linked say is for actual drawing to the screen, so it may be leveraging stuff from the main thread. I would at least recommend dropping that to .userInitiated. More frequently for networking either .utility or .background is what gets used.

If neither of those changes help, either a) A sample app that reproduces the slow-downs you're seeing and/or b) info from Instruments about where the slow-downs are occurring, and how slow things get.

victormihaita commented 3 years ago

Thanks for replying. I've tried your suggestions, but I still have the same results.. Here is a print screen with the network requests that are performed when launching the app.. As you can see the duration is really slow..

designatednerd commented 3 years ago

So if you're seeing these numbers within ProxyMan (or any other network proxying software), that's going to be the duration from when a request leaves the device/sim to when the server responds.

I agree some of these numbers look pretty hideous (13 seconds...ouch), but because these numbers are from after the request leaves the device/sim, that indicates the issue is at the server level rather than an issue with Apollo or even URLSession code.

victormihaita commented 3 years ago

I've improved the performance a bit by not triggering that many queries at the same time and by removing the duplicated queries. Is it a good approach to use the OperationQueue() on the background thread to perform the network requests and is it the right direction the implementation I am doing above?