-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Suspending version of lazy { ... }
#706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
You can use I don't think such shorthand worth its own primitive: we don't have |
I guess that my main problem is that I'm trying to find a way to confine the evaluation to the scope of the first invocation, which I guess is probably not the way I should be doing this. I'll think a bit more about this and then report back. |
So, I've been thinking about this a decent amount, and I think that my general conclusion is that it would be nice to have efficient versions of I've managed to reduce most of my code to using channels, but I have a few strange cases:
So, I guess that I can create separate issues for 1 and 2 and close this. |
Wow, |
|
Having this or memoization would be helpful for lazily connecting to services and databases. We have a command line app that sometimes needs to connect to everything and sometimes nothing. Something like |
@ConnorWGarvey You can already do something similar without delegation: Example usage: val heavyThing = myScope.suspendLazy { // Or GlobalScope if app global
val stuff = withContext(Dispatchers.IO) { getStuffFromStorage() }
initThatHeavyThingWithASuspendFunction(stuff)
}
heavingThing().doStuff()
...
heavingThing().doMoreStuff() |
Precisely, so why would it block a thread? It makes zero sense. |
@alamothe Because allocation always blocks a thread, it's actually the CPU looking for space in the memory, and moving stuff if needed. So, it makes complete sense to me that Kotlin stdlib's If it still doesn't make sense to you, I encourage you to learn more about what happens when you want to allocate memory for some objects, and also, learn how coroutines work under the hood. And again, I linked an implementation + example of a |
Hi @LouisCAD, Hi @alamothe, |
@fvasco I disagree, allocation is always a blocking operation, and the time the thread requesting the allocation is blocked can vary depending on multiple factors like size requested and state of the memory. I'm talking low-level here. Now, I'm tired of this discussion, there's two solutions posted, nothing more to say, so I'm withdrawing. |
Hi @LouisCAD,
"Thread" and "blocked thread" is an abstraction of Operating System, not a CPU's one, but please correct me if I wrong. Allocation can be -generally- performed by running thread, not by a blocked one. Thank you. |
I get your point now @fvasco and thought about that after writing my message: Allocation is indeed performed by the calling thread, so, unless the object hierarchy being allocated is very big and you need the thread quickly (e.g. a UI thread), the most efficient way is to let it be run by the calling thread, not involving any coroutines that'd not improve performance at all. I think @alamothe has a misunderstanding of the purpose of If the code in the |
|
Same as Kotlin doesn't support suspending properties. It's trivial to implement it on the compiler end, it's very painful for us using the language. @LouisCAD You're getting too technical for something that's trivial to do. By your logic, suspending functions can't exist either, yet they do. |
@alamothe I don't see evidence that it's "trivial to do". If it really is, then you know better, which means you can submit a KEEP.
My logic, when stretched by you, but then it's no longer my logic.
This is off topic, and there are valid reasons (API design related) to have |
By that logic 🙂 we don't need properties at all. It's just a pair of parens. |
@alamothe You are completely ignoring the fact that when you read code, you expect a property to return immediately, while a suspending function is the opposite. But then, again, this is off-topic, so go on Kotlin's Slack if you want to debate that. |
What does "immediately" mean? Does How about if it spends 10s doing CPU work vs 0.1s I/O work? Which one is immediate? This is best left to code owners to decide. It's like saying we will forbid you to name variables with uppercase letters because you deemed that whoever reads the code expects lowercase letters. Not for you to make that judgement. |
@LouisCAD we have been using your implementation of |
Actually I have a question regarding the implementation. What if it's never called? Looks like it will hang @Test
fun testSuspendLazy() = runBlocking {
coroutineScope {
val l = suspendLazy {
println("hello")
}
// Hangs here
}
} |
Yes, use |
It's definitely a gotcha. I changed it not to call private class SuspendLazySuspendingImpl<out T>(
val coroutineScope: CoroutineScope,
val context: CoroutineContext,
val initializer: suspend CoroutineScope.() -> T
) : SuspendLazy<T> {
private var deferred: Deferred<T>? = null
override suspend operator fun invoke(): T {
if (deferred == null) {
deferred = coroutineScope.async(context, block = initializer)
}
return deferred!!.await()
}
} Do you see any problems here? (our code is single-threaded) I think the problem with a scope that lives forever is that they will never get garbage collected i.e. it is a memory leak. |
My use case is that there's no known/given
class SomeClass {
private val someConfig = GlobalScope.async { configLoader.readSomeConfigFromFile() }
suspend fun doSomething() {
val c = someConfig.await()
// ...
}
}
class SomeClass {
private val someConfig = SuspendingLazy { configLoader.readSomeConfigFromFile() }
suspend fun doSomething() {
val c = someConfig.value()
// ...
}
}
The custom implementation of lazy// Please criticize.
class SuspendingLazy<T>(initializer: suspend () -> T) {
private var initializer: (suspend () -> T)? = initializer
private var mutex: Mutex? = Mutex()
private var _value: T? = null
suspend fun value(): T {
val m = mutex ?: return _value as T // Warning: unchecked cast.
m.withLock {
val i = initializer ?: return _value as T // Warning: unchecked cast.
val v = i()
_value = v
initializer = null
mutex = null
return v
}
}
} |
try this on for size: /**
* Use this like a lazy, but because operator getValue cannot
* be suspend, you'll have to invoke this object instead in a
* suspend context to receive the value.
*
* Use when you want a lazy that is loaded via a suspend fun
* and you use it in a suspend fun which can tolerate loading
* the value on a miss.
*/
class LazySuspend<T>(
private val block: suspend () -> T,
) {
private val value = AtomicReference<Deferred<T>>()
suspend operator fun invoke(): T = (
value.get()
?: coroutineScope {
value.updateAndGet { actual ->
actual ?: async { block() }
}
}
).await()
} In case usage is unclear: private val lazyValue = LazySuspend {
delay(100)
"hello"
}
private suspend fun usesLazy() {
val hello = this.lazyValue()
print(hello)
} This strategy maps more faithfully to |
@fikr4n Your implementation is definitely appealing, because dealing with coroutine scopes is painful to say the least. The problem is that it is cancellable, which is undesirable for lazy value. Consider: val x = suspendLazy {
println("Calculating x")
delay(1000)
10
}
try {
coroutineScope {
val pAsync = async {
x()
}
val qAsync = async {
delay(100)
throw NumberFormatException()
}
pAsync.await()
qAsync.await()
}
} catch (e: NumberFormatException) {
println("Caught")
}
println(x()) This will start the calculation two times, because the exception will cancel the first calculation. EDIT: perhaps this is something that can be fixed simply with @kvcache Your implementation is very problematic, because if the lazy is shared among coroutine scopes where one is canceled due to an exception (unrelated to lazy), then others will be canceled as well. Consider: val x = suspendLazy {
println("Calculating x")
delay(1000)
10
}
coroutineScope {
List(10, { it }).map {
async {
try {
coroutineScope {
val pAsync = async {
x()
}
val qAsync = async {
delay(100)
throw NumberFormatException()
}
pAsync.await()
qAsync.await()
}
} catch (e: NumberFormatException) {
println("Caught")
}
}
}.map { it.await() }
} It would have been expected here that all exceptions are caught and nothing major happens, however with your implementation JobCancellationException will be propagated outside. So I absolutely wouldn't recommend it. |
Yes, in our case it is expected, because the lazy we need is only for optimization: changing "something repeated" become "something cached". Because it is originally repeated, in our case, repeating incomplete operation is not a problem. On the other hand, making it non-cancellable would lead to allowing a never-ending coroutine, furthermore the coroutine scope is unknown. Thanks |
@fikr4n Sure! There is also a small bug if initializer returns null: val x = suspendLazy {
delay(1000)
null
}
println(x())
println(x()) The second call will throw NPE because of the bang usage. It is trivial to fix by replacing |
My bad, I must have copy pasted it wrong and created the bug myself 🤦♂️ |
Kotlin newbie here! I don't understand why everyone messes with coroutine scopes. Something like this: fun <T> deferredLazy(initializer: suspend () -> T): Deferred<T> {
val delegated = CompletableDeferred<T>()
val isStarted = AtomicBoolean(false)
return object : Deferred<T> by delegated {
override suspend fun await(): T {
if (!isStarted.getAndSet(true))
delegated.complete(initializer.invoke())
return delegated.await()
}
}
} Warning: not tested, just asked out of curiousity |
Right now, I implement this with:
Although this kind of primitive seems reasonable enough to add by default.
The text was updated successfully, but these errors were encountered: