Do I need to rewrite every interface to migrate to `suspend`?

[mjgroth]

I’m working on migrating my thread-based code to coroutines.

I previously had a List called RedisList which preformed all of its operations against a redis database over a network connection. Now that I’m migrating to the Lettuce library which supports couroutines, I’d like all of my RedisList functions to suspend. However, the List interface does not support suspend functions.

Maybe I’m still struggling a little with understanding this migration. Since I can’t override List methods with the suspend keyword, now I feel like I need to create my own SuspendableList interface from scratch:

interface SuspendCollection<E> {
  suspend fun size(): Int

  suspend fun contains(element: E): Boolean

  suspend fun containsAll(elements: SuspendCollection<E>)
  suspend fun isEmpty(): Boolean
  suspend fun iterator(): SuspendIterator<E>
}


interface SuspendList<E>: SuspendCollection<E> {

  suspend fun get(index: Int): E

  suspend fun indexOf(element: E): Int

  suspend fun lastIndexOf(element: E): Int

  suspend fun listIterator(): SuspendListIterator<E>

  suspend fun listIterator(index: Int): SuspendListIterator<E>

  suspend fun subList(fromIndex: Int, toIndex: Int): SuspendList<E>
}


interface SuspendIterator<E>
interface SuspendListIterator<E>: SuspendIterator<E>

This of course feels crazy, because then I can no longer use any functions for List and need to create versions that are for SuspendList.

What am I missing?

[mjgroth]

Please let me try to frame my question better.

I am asking for general wisdom. I’m struggling to wrap my head around how it is intended that we incorporate suspending functions into our code. Specifically, with regard to external interfaces that do not have suspending functions.

The central problem I’m having is that if a function in an external interface is not suspending, I can not override it with a suspending one.

So that interface becomes useless if I need the implementation to have suspending functions.

The main example is MutableList. This is a critically important interface that I use throughout my code. I have so many functions that take it as a parameter or use it. And I also use the Kotlin Standard Library extension functions for collections extensively.

But then if I need an implementation of a MutableList that is suspending, it seems impossible.

I am currently trying to write a new SuspendingCollection interface hierarchy from scratch. It is designed to be the same as the Collection hierarchy, only everything is suspending.

This is feeling like a huge endeavor and I find myself wondering: Am I making a mistake? Does a library like this not already exist? Am I not understanding something? Is there not any technique to continue using the classic Collection interface hierarchy in a suspending context?

[broot]

Well, you are basically correct and I don’t think you miss anything. One problem related to coroutines is that it is hard to use them e.g. from callbacks of existing libraries or sometimes even with some stdlib components. In Kotlin stdlib this problem is partially solved by inline functions which could work with both regular and suspending code. But reusing existing interfaces/classes for suspending code isn’t trivial/possible, so it is tricky to have e.g. a suspending Comparable.

Speaking about your specific case I would say this is a rather rare need. Collections usually don’t involve IO underneath and this is probably why you didn’t find such examples or existing libraries in the internet.

Please note this problem partially exists even in Java. Usually, methods that do any IO should be marked to throw IOException. But that means your implementation of RedisList is not entirely correct, because it performs IO, but it doesn’t say so in the method signature. I guess you rethrow IOException as some runtime exception. And sometimes we face an opposite problem where we know our implementation doesn’t do any IO, but we still have to handle IOException, because we use interfaces related to IO. Of course, IOException isn’t as bad as suspend, because the first isn’t required while the latter is.

[mjgroth]

Thank you @broot . Your answer is giving me a lot of peace of mind that I’m not missing something major.

You make a great point that my RedisList is not a perfect implementation of List since it can involve IOExceptions. This didn’t occur to me. Since my application is using ktor, any exception inside of an endpoint will just respond a status code 500 without crashing the server.

I have a related follow up question.

I have a Map implementation LazyMap. I use it like so:

 private val expUIDToUserMap = lazyMap<ExperimentUID, User> { expUID ->
	println("searching for user for expUID = $expUID")
	allUsers().first { u -> u.allExperiments().any { it.uid == expUID } }
  }

It’s just a cache using the Map interface. It lazily evaluates the value for each key in the map when it is requested, and then is stores the result of the calculation.

But now, functions like allUsers() and allExperiments() are suspending. So the code above will not compile.

So, given what I know about coroutines, I think that I have to create a whole new class SuspendingLazyMap, which would have to implement my SuspendMap.

I am curious if there is any alternative. For example, I am curious about the following:

  class ExpUIDGetter(
	val uid: ExperimentUID,
	val context: CoroutineContext
  ) {
	override fun equals(other: Any?): Boolean {
	  return other is ExpUIDGetter && other.uid == uid
	}

	/*generated by IntelliJ*/
	override fun hashCode(): Int {
	  var result = uid.hashCode()
	  result = 31*result + context.hashCode()
	  return result
	}
  }


  private val expUIDToUserMap = lazyMap<ExpUIDGetter, User> { expUID ->
	runBlocking(expUID.context) {
	  println("searching for user for expUID = $expUID")
	  allUsers().first { u -> u.allExperiments().any { it.uid == expUID.uid } }
	}
  }

  suspend fun findUser(
	expUID: ExperimentUID
  ): User {
	return expUIDToUserMap[ExpUIDGetter(expUID, coroutineContext)]
  }

My example above is probably totally wrong, because everywhere I am reading that runBlocking is not supposed to be used in regular code. But my understanding is that what would happen here is that although the thread would block, the coroutineContext that is carried over will allow the ktor application to continue running inside the new coroutine created inside of runBlocking. Although, I’m afraid I’m still making some sort of big mistake here because I don’t have enough of an in-depth understanding.

[broot]

@mjgroth

But my understanding is that what would happen here is that although the thread would block, the coroutineContext that is carried over will allow the ktor application to continue running inside the new coroutine created inside of runBlocking

No, Ktor won’t be able to use this thread. If you invoke runBlocking() directly or indirectly from the default dispatcher of Ktor, in most cases that will mean that if you have e.g. 8 CPU cores, you will loose 1/8 of the performance until the IO in your lazy map finishes. You can partially mitigate this problem by switching to Dispatchers.IO or your own thread pool whenever you are going to call a function that you know will call runBlocking() underneath. It won’t solve the problem of blocking threads, but at least we will block a thread from a bigger pool.

But if we are going to do this frequently, then we pretty much kill the idea of coroutines. We can use a classic blocking web server, because this is what we do here. If we can make most of our code suspending, but we have this single, tricky component that can’t be easily ported, then I guess it makes sense to workaround it this way.

[mtimmerm]

The thing you are missing is that the methods of the List interface are not appropriate for execution directly against a database or Redis. They generally return just a little bit of information at a time or perform an operation on just one element.

That would make programs that use this interface very slow, because they have to wait for a complete round-trip to Redis for every little operation, and probably not thread-safe if other clients might be working on the same list.

For operating on a list in Redis, you should design an appropriate interface with suspending methods that perform operations of the appropriate granularity.

This is actually a very common kind of X/Y problem – you are right that you don’t get suspending versions of common interfaces for free, but you are wrong in wanting those interfaces in the first place.

[thumannw]

Since it has not yet been mentioned in this thread: The appropriate way to handle multiple values in the “suspending” world is a Flow (hence no need for a SuspendCollection).

[broot]

Flow and SuspendCollection are two much different things. Flow could be compared to Iterable, but not really to a collection, list or anything else.

[mjgroth]

@mtimmerm

The thing you are missing is that the methods of the List interface are not appropriate for execution directly against a database or Redis. They generally return just a little bit of information at a time or perform an operation on just one element.

That would make programs that use this interface very slow, because they have to wait for a complete round-trip to Redis for every little operation, and probably not thread-safe if other clients might be working on the same list.

I’ve actually considered this in great detail over the last few days. I’ve created a special iterator class that uses a custom distributed lock system to ensure that the underlying redis list is not modified… its been a heavy undertaking.

In any case, after working through this for the last few days I’m finding myself mostly agreeing with @mtimmerm that I’ve made some mistakes and that I’ve fallen for an XY problem.

I’ve created a hierarchy of SuspendCollection interfaces. They mimic the regular non-suspending Collection interfaces.

They work, but in some places I’m finding my design to be very flawed.

A good example is my SuspendMutableMap. In order to to mimic MutableMap, the interface contains the function:

    suspend fun entries(): SuspendMutableSet<SuspendMutableEntry<K, V>>

It works… but underneath the hood the Lettuce library for redis uses a Flow. Flows can be collected into a collection. But why should my SuspendCollection interfaces force the user to get a SuspendCollection? It would be better to just give them the Flow and allow them to collect the flow if they desire. I will consider replacing this with something like:

    suspend fun entries(): Flow<Map.Entry<K, V>>

However, my initial intention was good. My intention was to create a high level abstraction so that I could work with a Redis list as if it was just like a regular collection. By using distributed locking, Redis transactions and Redis LUA scripting when required, I intended to ensure these classes would be safe to iterate and modify from multiple clients.

But I am starting to feel like I get why the regular Collection class shouldn’t be used, and why it shouldn’t necessarily be mimicked function-for-function either. Maybe, at best, I should think of the SuspendCollection hierarchy as being inspired by, but not a mirror image of the Collection hierarchy. I should design its functions to respect that the underlying data is in a database over a network connection and not in memory. But I still want it to attempt to be as convenient as a Collection when possible.

[Skater901]

This issue of existing interfaces not playing nice with coroutines is something I noticed a while back… I’m curious if the JetBrains team have any ideas on how that could be solved. I’m almost tempted to say every interface should be marked with suspend, because you can work around blocking code in a suspend function using IO dispatchers or your own threadpool, but if you have a non-suspending interface you are implementing and you want to use coroutines, you’re out of luck. There’s literally nothing you can do. I’m partly replying to this thread just to follow along and see if any other geniuses have any ideas about how to get third-party interfaces to play nice with coroutines.

Regarding having a SuspendingCollection returning a Flow… I think that’s kind of dangerous. A Flow isn’t analogous to a Collection in the sense that it just contains all these items and you can retrieve whatever items you want. When you collect a Flow, you are triggering the initial Flow builder code to run and compute the values for the Flow. Instead of it being like a Collection, where it’s the same group of values each time, with a Flow, it’s a new group of values each time. Or in other words, a Collection is like a cache, a Flow is like going to your database every time you want your data. Nothing wrong if that’s what you want, but if you’re treating a Flow like a List, you’re probably going to have problems.

[mjgroth]

@Skater901

I had pretty much the same exact initial thoughts. Why can’t regular interfaces be suspending? I think JetBrains is trying to enforce a separation between suspending and non-suspending code. One way I’m trying to rationalize after reading some of the responses in this thread is that when you are given a Collection, it is nice that you can assume it is held in memory and not in a network database. If Collection was so abstract that it could also include data held in network databases, then functions that operate on collections may all need to take this into consideration. Having separate interfaces for suspend functions (in other words, separate interfaces for data not held in memory) is starting to seem more appealing to me as I’m realizing that they often need to be operated on in different ways. I’m not saying I disagree with you, part of me still wants to have the freedom to use any interface in a suspending fashion. I’m just playing devils advocate.

@Skater901

A Flow isn’t analogous to a Collection in the sense that it just contains all these items and you can retrieve whatever items you want.

Maybe I wasn’t writing my thoughts clearly enough, but this is actually exactly my point. I’m wondering if my SuspendingMutableMap.entries() should return a Flow and instead of a SuspendCollection because that is exactly how the underlying data will come in. I won’t get the entries all at once, I’ll get them in a Flow. See the function of the Redis library I’m using here; it returns a Flow. So for me to return a Collection of some sort would mean I would be taking the flow and collecting it first. I’m just thinking returning the flow itself might make more sense, and then code that uses this class could decide if they want to collect the flow.

[broot]

@mjgroth

Why can’t regular interfaces be suspending? I think JetBrains is trying to enforce a separation between suspending and non-suspending code.

One reason is compatibility with the existing code. If Kotlin would not assume any code interop with Java or JavaScript, then they could skip suspend at all and simply make all Kotlin functions suspendable. But that would mean we couldn’t use most of the existing stdlib of Java/JS, so Kotlin’s stdlib would be much more heavy, and more importantly, we couldn’t use Kotlin code with any existing Java/JS code, because it is not suspendable.

[Skater901]

I wonder if it’d be possible for the Kotlin compiler to recompile library bytecode to make interfaces suspending… though I guess that’d only work when the interfaces are only called from suspending functions. Also only works if you’re packaging your code up into a shaded jar.

@mjgroth

Maybe I wasn’t writing my thoughts clearly enough, but this is actually exactly my point. I’m wondering if my SuspendingMutableMap.entries() should return a Flow and instead of a SuspendCollection because that is exactly how the underlying data will come in. I won’t get the entries all at once, I’ll get them in a Flow. See the function of the Redis library I’m using here; it returns a Flow. So for me to return a Collection of some sort would mean I would be taking the flow and collecting it first. I’m just thinking returning the flow itself might make more sense, and then code that uses this class could decide if they want to collect the flow.

Thinking about it more, I think my issue is my expectations for what a Collection does. I expect a Collection to store data in memory. A Map that’s backed by an underlying database means the contents of the Map can change without my code changing them. Whether that’s “wrong” or just different to my personal expectations for how a Collection/Map should function… idk. I certainly wouldn’t want to use this kind of Map in my code, though.