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

Allow IL scanner's dependency graph be GC'd #72430

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

MichalStrehovsky
Copy link
Member

We don't need to keep the graph in the memory once we're done scanning.

Verified it gets collected now by adding a finalizer to ILScanNodeFactory and having it print something. It didn't get collected previously. If I'm measuring it right, it was rooting about 17 MB of objects in memory on a hello world.

Cc @dotnet/ilc-contrib

We don't need to keep the graph in the memory once we're done scanning.

Verified it gets collected now by adding a finalizer to `ILScanNodeFactory` and having it print something. It didn't get collected previously. If I'm measuring it right, it was rooting about 17 MB of objects in memory on a hello world.
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Nice!

@VSadov
Copy link
Member

VSadov commented Jul 19, 2022

I like the improvement, but I wonder if there is something wrong with the liveness analysis and thus the user needs to do this.
Should we know that the scanner roots are all dead at some point and not report them (at least in release build)?

Is it the part that the containing method is 500-lines long that makes it difficult to track when the scanner is unreachable?

@MichalStrehovsky
Copy link
Member Author

Is it the part that the containing method is 500-lines long that makes it difficult to track when the scanner is unreachable?

I didn't test whether just shuffling things around (without moving scanning into a separate method frame) would be sufficient to get it GC'd. It might have been enough.

JIT is free to extend the lifetimes of things however it sees fit - an extra method frame makes sure the locals really become dead no matter what the JIT decides to do now or in the future.

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

Successfully merging this pull request may close these issues.

None yet

3 participants