Add Asynchronouts/Deferred implementation to Firestore/Database/etc

[Daeda88]

Wanted to start a discussion on this, so writing a ticket instead of a full on PR for a change. A suggestion for an implementation of this was originally in #448 but was removed because I couldn't properly clarify it at the time (someone else within our company had written it) and I felt it was best to take it out of the PR for clarity.

Consider the following usecase:

• We have a (multiplatform) viewmodel with a coroutineScope that lives while the corresponding view is on screen
• It receives frequent data from some service that needs to be stored to firestore, but we don't actually need to display whether that operation succeeded
• Speed is important cause it's a lot of data

Then

1. If I want to write the data fast, I can't use a collect on the flow of data to then send each data to firestore. Because it is a suspend method, I'll have to wait until the previous data was written before I can write the next data
2. Alternatively, within the collect I can launch and write from there, but launch takes some time to spin up and may slow me down.
3. Since I'm launching with the VM scope, the collect and therefore the write will stop when I leave the view (assuming I'm not able to put it in a service that survives the screen for whatever reason).

To solve this, it could be considered to add a non-suspended (deferred) approach to the library. Where the methods are not suspended at all, but just return a Deferred of the result. Since the platform APIs do nothing with coroutines, this is very straightforward.

It may be argued that you can just call the suspended methods within scope.async {} but that means you need to have a coroutineScope, which adds a lot of overhead for high volumes of data and has some form of a lifecycle. Or, to look at it from another way: even though the firebase implementation is suspended, since the internal implementation is not, actions like cancelling the scope have little effect on the interaction with the internal firebase sdk. So from both perspectives, coroutineScopes are not strictly required nor necessarily even beneficial.

My suggestion boils down to this.

Current code:

@PublishedApi
internal expect class NativeDocumentReference(nativeValue: NativeDocumentReferenceType) {
    suspend fun setEncoded(encodedData: EncodedObject, setOptions: SetOptions)
}

data class DocumentReference internal constructor(@PublishedApi internal val native: NativeDocumentReference) {
    suspend inline fun <reified T : Any> set(data: T, merge: Boolean = false, buildSettings: EncodeSettings.Builder.() -> Unit = {}) = native.setEncoded(
        encodeAsObject(data, buildSettings), if (merge) SetOptions.Merge else SetOptions.Overwrite)

}

New code:

@PublishedApi
internal expect class NativeDocumentReference(nativeValue: NativeDocumentReferenceType) {
    fun setEncoded(encodedData: EncodedObject, setOptions: SetOptions): Deferred<Unit>
}

data class DocumentReference internal constructor(@PublishedApi internal val native: NativeDocumentReference) {

    data class Async(@PublishedApi internal val native: NativeDocumentReference) {
         inline fun <reified T : Any> set(data: T, merge: Boolean = false, buildSettings: EncodeSettings.Builder.() -> Unit = {}) = native.setEncoded(
            encodeAsObject(data, buildSettings), if (merge) SetOptions.Merge else SetOptions.Overwrite)
        }
    }
    
    val async = Async(native)

    suspend inline fun <reified T : Any> set(data: T, merge: Boolean = false, buildSettings: EncodeSettings.Builder.() -> Unit = {}) = async.set(data, merge, buildSettings).await()
}

If others see the benefits of adding an async implementation as well, Im happy to make a PR for it.

[nbransby]

• Alternatively, within the collect I can launch and write from there, but launch takes some time to spin up and may slow me down.

What gives you the impression launch takes time to spin up?

It may be argued that you can just call the suspended methods within scope.async {} but that means you need to have a coroutineScope, which adds a lot of overhead for high volumes of data and has some form of a lifecycle.

What scope would you await the Deferred returned from the proposed api? Doesn't that give you the same problem?

3. Since I'm launching with the VM scope, the collect and therefore the write will stop when I leave the view (assuming I'm not able to put it in a service that survives the screen for whatever reason).

Just launch it in the GlobalScope instead then? That's essentially what's happening if the API was to return Deferred.

Also since (at least for android) we are using Task<T>.await() then according to the docs scope cancellation just means it stops waiting for the task instead of stopping the task (we should probably switch to using Task<T>.await(cancellationTokenSource: CancellationTokenSource))

[Daeda88]

1. I mean there is computational overhead in spinning up a coroutine. It may not be much, but it is there and for large amounts of data, it may not be beneficial.
2. It could be none (if you just want to fire and forget) or maybe you pass a collection of deferreds to a different viewModel and await them all in its scope.
3. Well, returning deferred is not launching anything, just running on whatever internal threading mechanism the official SDK uses. Its essentially this right:

inline fun <reifeid T> getDeferred(): Deferred<T> {
    val result = CompletableDeferred<T>()
    firestoreSDK.doStuff(object : FirestoreCallback {
          override fun onSucces(value: T) { 
               result.complete(value)
          }

          override fun onError(error: Exception) {
             result.completeExceptionally(error)
          }
    })
    return result
}

Launching on Globalscope is a big no-no in my book, and its entirely unnecessary to launch anything if you want a deferred.

In the end, users wrapping the suspend methods in async as would currently be the recommended strategy, instead of having the suspend methods wrap a deferred as this ticket proposes, adds unnecessary overhead that also requires either lifecycle management or launching in an non-recommended scope.

[nbransby]

1. sounds like premature optimization, coroutines are super lightweight as they are effectively just a run loop. I wouldn't worry about spinning them up unless you have benchmarks showing a bottleneck (tbh your problem with the huge number of writes needed for it to be an issue would not be the speed but the cost on firebase!!)
2. fire and forget is a bad idea as you don't have any error handling if the write fails, suspending functions encourage you to deal with the result by implicitly waiting for it, I think that's preferable.
3. yes it's just running on whatever internal threading mechanism the official SDK uses which is effectively a globalscope because they aren't scoped to anything and execution can continue past the scope of your application that it makes sense for them to execute.

In the end, users wrapping the suspend methods in async as would currently be the recommended strategy, instead of having the suspend methods wrap a deferred as this ticket proposes, adds unnecessary overhead that also requires either lifecycle management or launching in an non-recommended scope.

I believe the unnecessary overhead is negligible and not worth the api bloat proposed here. Requiring lifecycle management is good thing - it forces developers to consider the lifetime of the async operation and is the reason for the whole coroutines scope concept.

[Reedyuk]

Since I'm launching with the VM scope, the collect and therefore the write will stop when I leave the view (assuming I'm not able to put it in a service that survives the screen for whatever reason).

Just want to understand your usecase, it sounds like your job shouldn't be bound to the VM scope because you don't want it cancelling when the viewmodel scope is closed.
For your scenario why cant you use do something like:

CoroutineScope(Dispatchers.IO).launch {
        // stuff to do
 }

I don't think its a no no, especially when you don't want it to be bound to a particular scope.

Assuming it is android, If you want to be super safe, you could create an application scope that is associated to your Application and use this instead?

[Qw4z1]

I tend to agree with @nbransby here.

My experience is that writing to Firestore already is really fast, because afaik it defaults to writing to the local cache first and the synchronizes async. Write functions returns almost immediately, even when writing fails due to Rules or what not.

@Daeda88 how much data are we talking about here? 10 doc/s? 100 docs/s? More?