Skip to content
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

Optimize dumpCoroutinesInfoAsJsonAndReferences #4323

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mvicsokolova
Copy link
Contributor

I've removed trimIndent and toStringRepr calls, made during the composition of coroutine dump json.

The motivation: Intellij IDEA Debugger supports the Coroutine View, which can show the hierarchy of all the available coroutines. This View struggled of sever performance issues (IDEA-335303). There were reasons for that on the debugger side, like fetching additional information for each coroutine. But other than that, the invocation of dumpCoroutinesInfoAsJsonAndReferences took significant amount of time.

I made some dumb performance measurements on the dumps of ~5000 coroutines, the absolute time is irrelevant ofc, just % of time of the parent call are:

  1. Original version, most of time is taken by trimIndent:
Screenshot 2025-01-07 at 16 56 44
  1. Without trimIndent invocation:
Screenshot 2025-01-07 at 16 57 09
  1. Without toStringRepr invocation as well, it finally makes sense and shows dumpCoroutineInfos as the next significant call:
Screenshot 2025-01-07 at 16 50 38

It's not necessary to call trimIndent on every piece of this JSON, so that it could be parsed on the debugger side. So, I suggest to optimize this code.

…d toStringRepr calls.

`trimIndent` and `toStringRepr` calls cause a significant overhead for json dump for a large number of coroutines.
@mvicsokolova mvicsokolova requested review from dkhalanskyjb and removed request for dkhalanskyjb January 7, 2025 16:00
@fzhinkin
Copy link
Contributor

fzhinkin commented Jan 7, 2025

@mvicsokolova, is there a way to improve trimIndent performance? It might be fine to abstain from calling it here, but I believe that it will be beneficial for all other projects relying on that function.

@mvicsokolova
Copy link
Contributor Author

Yes, I'll create a ticket for trimIndent optimization in kotlin-stdlib 👍🏼

@mvicsokolova
Copy link
Contributor Author

I'm not sure for now though, what's the better way to deal with special symbols in the coroutine name, than the current toStringRepr.

@dkhalanskyjb
Copy link
Collaborator

Yes, repr() is needed to ensure that JSON is valid. Not all hope is lost, though. I'm not a performance optimization expert, but there are a few ideas I'd try.

  • Usually, there are very few dispatchers in the program. This presents an optimization opportunity. If you cache the string representations of dispatchers in an IdentityHashMap<CoroutineDispatcher, String>, the number of repr() calls should drastically decrease (at least by about a half for a large number of named coroutines, but realistically, even more).
  • If this doesn't help, maybe you could first check if the string contains any special symbols (99% of them won't), and if not, skip char-by-char string copying entirely.
  • Do we need a pretty JSON at all? If we're not trimming indents, why not write the JSON string in a single line? The size of the resulting string would be literally half of what it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants