Use kotlin coroutines instead of raw threads (+ refactorings) #6801
Conversation
…ch time it does something This theoretically makes it unnecessary for the OnlineMultiplayerGameSaver to be re-instantiated each time it is used
This probably also cuts down on our raw thread usage, improving performance, since we now use a cached thread pool (in addition to coroutines being able to reuse threads anyway)
Azzurite
changed the title
|
Just interested, what is this |
|
Also why do you do stuff = newStuff {
// .. moreStuff
return@newStuff
}in place of stuff {
// .. moreStuff
return
}does this change anything? |
kotlin's default way of handling asynchronous programming. https://kotlinlang.org/docs/coroutines-overview.html
yes |
Because that's just how you use coroutines. I don't really want to explain here how they work, it's all explained in the official documentation I think my implementation was as simple as possible and basically a 1:1 drop in to what we currently do. |
|
So, basically you believe that coroutines are a more lightweight form of handling parallel processing and than Java Threads and implementing those for better performance? But since that would take explaining first that these would actually optimize stuff, you are shipping these with your [one new feature]™? |
Yes
No, I'm adding coroutines with this PR. Re-read the second paragraph of the PR description. Apart from a few of the refactorings, this PR is completely standalone and does what the title says: "Use kotlin coroutines instead of raw threads". So no, I'm not "shipping coroutines with my new feature", I'm shipping them here, that's what this PR is about.
Not really massively. But it is making multiplayer updates asynchronous and separates them out into its own module, and coroutines are just better for asynchronous programming than anything based on standard java threads and whatever else java has. |
Also pardon the sarcasm but coroutines looks like fun stuff() = runBlocking {
launch { someStuff() }
launch { someMoreStuff() }
}looks a lot like async function stuff() {
await Promise.all([
someStuff(),
someMoreStuff()
]);
}Except based on docs, they are trickier and gives you more control over how your processor would actually handle them. |
|
Also since I am new to kotlin it would be if you had some docs about |
This: fun stuff() = runBlocking {
launch { someStuff() }
launch { someMoreStuff() }
}has no Javascript equivalent, since Javascript always runs on a single thread. Coroutines can run on a single thread like Javascript async/await, but it depends on what you use as your coroutine scope. The scope is basically "where does our coroutine run". And in this PR, the coroutine scope is always the cached thread pools, i.e. multiple threads. But also, what your example is actually doing is more alike to function stuff() {
someStuff()
someMoreStuff()
}where both functions are If you wanted to do something like async function stuff() {
await Promise.all([
someStuff(),
someMoreStuff()
]);
}in coroutines, it would look like this: fun stuff() = runBlocking {
listOf(
async {
someStuff()
},
async {
someMoreStuff()
}
).awaitAll()
}but you'll likely not write code like that with coroutines. In that case you'd rather use flows or channels. But again, I really don't want to do a tutorial on coroutines here... If there are any specific questions about how this PR is implemented, okay, but I'm not going to further explain coroutines in general (unless you pay me for my time ;) |
trailing lambda: https://kotlinlang.org/docs/lambdas.html#passing-trailing-lambdas |
🤣 👍 👍
Coroutines are not a Religious matter, they're fact. And they're the way to get Unciv's CPU load to >16% where it's currently stopping even on the heaviest huge-map late-game next-turn. If only I had some ideas how to de-couple e.g. the units so one could automate without knowing whether Schrödinger's cat is alive or dead... |
Sigh. That's why I sometime wished for Github to have a realtime chat feature to that we could have such silly chats. Forums are not that good for chatting. And I was bored that time reading docs. |
|
So you didn't dare tackle the MusicController. I know it's intimidating, but the threading elements are really only a way to update volume repeatedly (fade) or leave a pause between tracks. And the latest complication is simpler than it looks too - lwjgl3 became less thread-safe than 2 on desktop only so I hijacked the Gdx app loop instead of postRunnable everything. On desktop the only Thread is a recovery delay after a bad crash; but on Android that "fade" ticker is still a Timer. Come to think of it, Looks good, but I'm too tired right now to really switch on brain (hi brain where are you). Later. |
|
How much does this raise the size of the apk? |
What do you mean, "dare tackle"? There are no threads being used within |
@yairm210 it is < 0.1 MB from my build tests. |
This PR raises the size of the APK compared to the base commit of this branch by 20 kB, from 11,685 kB to 11,705 kB |
Just wondering why so little. Maybe because coroutines are built on top of Java Threads or something like that? |
|
Probably because this is the coroutine library:
|
|
😎👍 |
|
That was the major reason holding back coroutines however many years ago we tried it, of that's ok then in it goes |
|
"Jar-Datei" - And I would have guessed you to be a Tango-dancing, Suomi full of "Sisu" (ntbc w/ politics). Go prejudices go! 🥳 |
But a kotlin.concurrent.timer.Timer? Is that integrated well enough w/ the coroutine thread pool to just leave it? |
|
@Azzurite hey, we've had Music stutter on moderately large loads / nextturns for a while (on Mint at least), and now we're getting |
Also just wondering if I am the only one facing this error. Does this work on Windows or Linux amd64 devices? It isn't because I am I was testing on a arm device or is it? |
So for that [one new feature]™ that I'm currently developing I did all these refactorings & kotlin coroutine addition, but yeah, it makes major sense to split that up.
So here are a bunch of refactorings/smaller improvements and the main thing: inclusion of kotlin coroutines within the project, and switching all usages of raw threads to coroutines which run on a cached thread pool (and coroutines also reuse threads anyway). This alone should vastly reduce our number of threads and thus likely increase performance.
When reviewing this, I would strongly suggest to do it commit-by-commit. I have neatly separated each refactoring/improvement into its own commit and the commit messages should be self-explanatory. I hope each commit compiles by itself, but I might have made mistakes, I split this into commits post-partum.
I tested pretty much all aspects of the game on desktop & android, should all be fine.
Here are some additional comments:
ScreensFileNotFoundexception handling in the multiplayer logic, it just makes sense to use 404's in the custom server implementation.GameSaver.saveGamethat did not add asaveCompletionCallback. These swallow all exceptions that happen on saving the game, which are some of the most important exceptions to handle, or at least get reports about from users. So I simply made the default handler throw the exception.