awslabs / aws-mobile-appsync-sdk-android

Android SDK for AWS AppSync.
https://docs.amplify.aws/sdk/api/graphql/q/platform/android/
Apache License 2.0
105 stars 58 forks source link

Multiple queries in one go performance issue #302

Closed l-lemesev closed 2 years ago

l-lemesev commented 4 years ago

I'm having a performance issue that I'm trying to solve. I'm making a query and when I get results I have to run two additional queries per result. I make sure I get everything downloaded in the background once the user signs in and then run the queries with CACHE_ONLY policy. If I understand correctly the AWSAppSyncClient runs the queries one by one which results in long waiting times before I get the data and can show it to the user.

AppSync

I'm using Koin as DI and have the following classes :

AWS Koin module:

const val APP_SYNC_SESSION = "APP_SYNC_SESSION"
val awsModule = module {
    scope(APP_SYNC_SESSION) { AppSync(androidContext(), get(name = APP_SYNC), get<CustomCredentialsProvider>()) }
    scope(APP_SYNC_SESSION) { CustomCredentialsProvider(get())}

    single { AppSyncSessionManager() }
    single<AwsTimeStampProvider> { AwsTimeStampProvider() }
}

Query/Mutation abstract class:

abstract class AppSyncAbstractOperation<D : Operation.Data, T, V : Operation.Variables> : KoinComponent {
    protected val userRepository: UserRepository by inject()
    protected val appSync: AppSync by inject()
}

Query abstract class:

abstract class AppSyncAbstractQuery<D : Operation.Data, T, V : Operation.Variables, Result> : AppSyncAbstractOperation<D, T, V>() {
    protected lateinit var continuation: CancellableContinuation<AppSyncResult<Result>>
    abstract fun getQuery(): Query<D, T, V>?
    private val queryCallback = object : GraphQLCall.Callback<T>() {
        override fun onResponse(response: Response<T>) {
            Timber.d("query [${this@AppSyncAbstractQuery.javaClass.simpleName}] ended ,  error number = ${response.errors().size}")
            val parsedResult = parseSpecificResult(response)
            parsedResult.let {
                if (!continuation.isCompleted) {
                    continuation.resume(it)
                }
            }
        }

        override fun onFailure(e: ApolloException) {
            Timber.e(e, "Exception during [${this@AppSyncAbstractQuery.javaClass.simpleName}] query")
            if (!continuation.isCompleted) {
                continuation.resumeWithException(AppSyncQueryApolloException())
            }
        }
    }

    protected abstract fun queryResponseFetcher(): ResponseFetcher
    protected abstract fun parseSpecificResult(response: Response<T>): AppSyncResult<Result>

    suspend fun runQuery(): AppSyncResult<Result> = suspendCancellableCoroutine { cont ->
        continuation = cont
        getQuery()?.let {
            appSync.aWSAppSyncClient.query(it)
                .responseFetcher(queryResponseFetcher())
                .enqueue(queryCallback)
        }
    }
}

This is how I'm executing the request

val result = AppSyncGetDiaryRangeDataQueryWrapper(start, end).runQuery()
                val sessions = arrayListOf<DiaryTreatmentSession>()

                async {
                    result.data.treatments?.values?.forEach {
                        it.forEach {
                            async {
                                val res = AppSyncGetSessionDataQuery(it, AppSyncResponseFetchers.CACHE_ONLY).runQuery()
                                val questionnaire = AppSyncExtendedQuestionnaireDataQuery(res.data).runQuery()
                                sessions.add(DiaryTreatmentSession(questionnaire.data.treatmentData, questionnaire.data))
                            }.start()
                        }
                    }
                }.wait()

Android API 29 AppSync v3.1.0

CompileConnected commented 4 years ago

I kinda interested, in your code but i never actually use continuation in wrapping graphql result. So For the problem, i might guess.

val result = AppSyncGetDiaryRangeDataQueryWrapper(start, end).runQuery() val sessions = arrayListOf()

            async {
                //first problem.
                result.data.treatments?.values?.forEach {
                    it.forEach {
                        async {
                            val res = AppSyncGetSessionDataQuery(it, AppSyncResponseFetchers.CACHE_ONLY).runQuery()
                            val questionnaire = AppSyncExtendedQuestionnaireDataQuery(res.data).runQuery()
                            sessions.add(DiaryTreatmentSession(questionnaire.data.treatmentData, questionnaire.data))
                        }.start() 
                    }
                }
            }.wait() <---- second problem (main problem)

first problem so we double loop and then one by one it start api, from my experience calling Appsync is pretty much heavy duty task, i would recommend to minimize the call

second problem (main problem) the .wait means that you actually wait all the api call in synchronized manner. it would be performance impact

l-lemesev commented 4 years ago

The whole point of this operation is to download all data and show it to the user. It doesn't really matter what the implementation would be so I'm down to try other possible solutions. The double loop here assures the queries are executed in parallel instead of consequently (which I tried before and it works even slower). And the await on the outer async assures we download everything before continuing the coroutine execution. I know our GraphQL schema can use some optimization, but it's a difficult task and won't happen in the next couple of months so I have to work with that. Basically I'm running a query that gives me list of ID's and then for each ID I need to run two queries and gather data from both of them to create some object which I then show to the user after all objects are downloaded. How would you approach such a use case without using the continuation?

CompileConnected commented 4 years ago

How would you approach such a use case without using the continuation?

I did create gist https://gist.github.com/CompileConnected/1701801ac8f14265c47c2cffac192d5d But here the code

suspend fun <T> GraphQLCall<T>.toDeferred(): Response<T> {
    val deferred = CompletableDeferred<Response<T>>()

    deferred.invokeOnCompletion {
        if (deferred.isCancelled) {
            cancel()
        }
    }
    enqueue(object : GraphQLCall.Callback<T>() {
        override fun onResponse(response: Response<T>) {
            if (deferred.isActive) {
                deferred.complete(response)
            }
        }

        override fun onFailure(e: ApolloException) {
            if (deferred.isActive) {
                deferred.completeExceptionally(e)
            }
        }
    })

    return deferred.await()
}

then usually i run query or mutation like this

withContext(Dispatchers.IO) {
            try {
                val response = AWSAppSyncClient.query(query)
                    .responseFetcher(AppSyncResponseFetchers.NETWORK_FIRST)
                    .toDeferred()
                    .data()
                // handle response
            } catch (e: Exception) {
               // handle error
            }
        }

But you can use with CoroutineScope or async or viewModelScope.launch, your AppSyncAbstractQuery did overcomplicated to wrap graphqlcallback.

I don't know it this useful or not, this is how i handle query builder from AppSync //before extensions https://gist.github.com/CompileConnected/bf8f2a8b6a337451065280027a94cc56 //extensions https://gist.github.com/CompileConnected/0677f61494516bed48cf930f7a734c44 //after use extensions https://gist.github.com/CompileConnected/9a45b803e96c45ed0a54168cf5a1324f

Basically I'm running a query that gives me list of ID's and then for each ID I need to run two queries and gather data from both of them to create some object which I then show to the user after all objects are downloaded.

Is your case really need to download all object to finish before show to the user? I would suggest 2 solutions

We can show list of questioner one by one instead all of them wait to finish. for example that questioner one and three is finish then show it but other still in loading state until they finish download questioner

Change the UI UX, instead show the questioner in list, change it to pager like. for example User go to questioner, show loading, call api question [N], wait until it's done, show the one page with only question [N]. After it show the question [N] we use that time to load next question[N+1] or load other two next question. so user would not realize the slowness.

I would recommended you to use the second solution, because even coroutine is light weight bursting calling api still exhausting for both frontend and backend. but with second solution you would minimize the call and can be improve by how much you need to call the next questioner api and the most important user doesn't realize the process is slow

Hope this helpful @l-lemesev

CompileConnected commented 4 years ago

btw I'm not any part of maintainer in this AppSync Library. I'm sorry this is not the answer for Multiple queries in one go performance issue.

eeatonaws commented 2 years ago

Closing this issue since the original question was answered and this issue has been inactive for some time. Please create a new issue if you have additional questions.