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

Cache packages field, which is used by DefaultExternalLocationProvider.resolve #4009

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

whyoleg
Copy link
Collaborator

@whyoleg whyoleg commented Jan 27, 2025

Found and tested on https://github.com/rjaros/kilua (results are reproducible even after several runs).
Reduces allocations inside DefaultExternalLocationProvider.resolve from 11 GB to 1 GB. Previously packages were recreated for each resolve.
The changes don't really affect kotlinx.io and kotlinx.coroutines.

For future: probably it might make sense to also cache results of DefaultExternalLocationProvider.resolve?
In this specific project there are a lot of references to @Composable annotation passing through this function call.

@whyoleg whyoleg requested a review from vmishenev January 27, 2025 14:14
@whyoleg whyoleg self-assigned this Jan 27, 2025
Copy link
Contributor

@vmishenev vmishenev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably it might make sense to also cache results of DefaultExternalLocationProvider.resolve?

It might, but I'm afraid it could increase maxHeapSize. Anyway, we should assess the potential performance.

@whyoleg whyoleg merged commit 961c677 into master Jan 30, 2025
14 checks passed
@whyoleg whyoleg deleted the whyoleg/perf-packageList branch January 30, 2025 13:25
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.

2 participants