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

A mitigation for processors with memory leaks #1067

Merged
merged 2 commits into from Aug 16, 2022

Conversation

ting-yuan
Copy link
Collaborator

by keeping static variables as few as possible, and set them to null after each round.

See #1063 for details.

Copy link
Collaborator

@neetopia neetopia left a comment

Choose a reason for hiding this comment

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

Have you verified it with the reproduce project?

What I didn't get is that, in #1063 it says the the reference to the processors comes from instance object, however the processor loading happens in KotlinSymbolProcessingExtension.

Can you elaborate how un-assigning instance for ResolverImpl can help with this case when the actual reference to processors are in KotlinSymbolProcessingExtension?

@ting-yuan
Copy link
Collaborator Author

Yes it does help. Without this it'd OOM at ~30th iteration. Also verified with VisualVM and the leak is much slower now.

ResolverImpl.instance holds all the objects that are reachable from ResolverImpl. Cutting it makes most of the compiler data structures (indirectly) used by KSP unreachable.

There is no reference in static fields of KotlinSymbolProcessingExtension. Objects referenced by the instance of KotlinSymbolProcessingExtension should become collectible once the compilation finishes, because nothing holds the instance. Non-static fields are collectable.

The only thing left un-collectable are classes themselves (which are not trivial unfortunately), as well as objects reachable from static fields / companion objects.

Copy link
Collaborator

@neetopia neetopia left a comment

Choose a reason for hiding this comment

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

I get your point on the reference to compiler data structures. Just to confirm, does this PR solves the classloader issue caused by Room? My question was mainly on that part since the linked issue mentioned Room in particular.

@ting-yuan
Copy link
Collaborator Author

It mitigates it. This PR makes most of the objects in KSP and kotlinc collectable. There is still leak, but much smaller now. The thorough fix needs to be done in Room or more likely sqlite. This PR is the best we can do in KSP in case of leaking processors like this.

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.

None yet

2 participants