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

Why do I see a different GC behavior in .NET5 and .NET Framework(4.7.2) on Windows 10? #24440

Closed
Vaskaran opened this issue May 28, 2021 · 22 comments
Labels
Pri3 product-question Product usage related questions [org][type][category]

Comments

@Vaskaran
Copy link

Vaskaran commented May 28, 2021

Hi,
Consider the following program:

class Program
{
class Sample
{
~Sample()
{
System.Diagnostics.Trace.WriteLine("Destructor is called..");
}
}

static void Main(string[] args)
{

 System.Diagnostics.Trace.WriteLine("Experimenting destructors.");
 Sample sample = new Sample();
 sample = null;
    
 System.Diagnostics.Trace.WriteLine("GC is about to start.");
 GC.Collect();
 GC.WaitForPendingFinalizers();
    
 System.Diagnostics.Trace.WriteLine("GC is completed.");
 Console.ReadKey();

}
}

In .NET Framework 4.7.2, I can see the destructor message. But the same program in .NET 5(or .NET 6 or .NET Core 3.1), does not show the destructor message. Is the backward compatibility broken?

Additional info:
i) I use Windows 10.

ii) To avoid the "timing explanation" I have forced GC and analyzed the output.

iii)I have checked that .NET 5/6 shows the destructor message if you compose the object into another object and the composed object is not inside the scope of Main(). I am adding this info -if it can provide any help...

I'd appreciate your thoughts on this.

Regards,
Vaskaran

@dotnet-bot dotnet-bot added the ⌚ Not Triaged Not triaged label May 28, 2021
@BillWagner
Copy link
Member

Thanks for asking this question @Vaskaran

There is a lot of discussion on #17463 about this change. Quick summary: finalizers could produce a deadlock that prevented a program from exiting. Therefore, the code to run finalizers on exit was relaxed further.

You can also see dotnet/csharpstandard#309 for the updated text in the C# standard (draft for version 6).

I'll close this as a duplicate of #17463

@dotnet-bot dotnet-bot removed the ⌚ Not Triaged Not triaged label May 28, 2021
@BillWagner BillWagner added the duplicate Indicates issues or PRs that are duplicate of existing ones [org][type][category] label May 28, 2021
@svick
Copy link
Contributor

svick commented May 28, 2021

@BillWagner I'm not sure that's the same issue. When I run the above code on .Net Framework, the output is:

GC is about to start.
Destructor is called..
GC is completed.

On .Net 5, the output is:

GC is about to start.
GC is completed.

I.e. on .Net Framework, the finalizer is called while the application is still running, so I think a change to executing finalizers on process shutdown alone doesn't explain this difference.

Though it's still possible this is an intentional change and I think it's consistent with the new spec.

@BillWagner
Copy link
Member

Thanks @svick
I did think it was related, but Maoni would know for sure.
Adding @Maoni0 in case this is something we need to address differently.

@Vaskaran
Copy link
Author

Vaskaran commented May 28, 2021

@svick,
I am very much thankful to you that you hit the right point which I wanted to bring to everyone's notice. As an author, I like to give my readers a better explanation by not saying "something like it is by design.. etc." I have encountered questions like the following:
1.Is the backward compatibility broken now?
2. Is the GC in .NET 5 is MORE intelligent than in the .NET framework? etc.

  • This is the true reason to bring this to everyone's notice. I appreciate everyone's help in this discussion and I like to see it open at this stage if we all agree.

@BillWagner BillWagner reopened this May 28, 2021
@MSDN-WhiteKnight
Copy link
Contributor

I think it's not only about finalizers, but about what is considered unreachable. If we have code like this that sets variable to null to make object unreachable:

class MyClass
{
    ~MyClass()
    {
        Trace.WriteLine("MyClass finalized");
    }
}

//....

    static void Main(string[] args)
    {
        MyClass x = new MyClass();
        WeakReference wr = new WeakReference(x);
        Trace.WriteLine("MyClass created");
        x = null;
        Trace.WriteLine("GC Started");
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Trace.WriteLine("GC Finished");
        Trace.WriteLine("IsAlive: "+wr.IsAlive);
        Console.ReadKey();
    }

The output on .NET Core 3.1 is:

MyClass created
GC Started
GC Finished
IsAlive: True

Looks like the object was not touched by GC at all

But if we transform it like this:

    static WeakReference wr;

    static void Test()
    {
        MyClass x = new MyClass();
        wr = new WeakReference(x);
        Trace.WriteLine("MyClass created");
    }

    static void Main(string[] args)
    {
        Test();
        Trace.WriteLine("GC Started");
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Trace.WriteLine("GC Finished");
        Trace.WriteLine("IsAlive: " + wr.IsAlive);
        Console.ReadKey();
    }

The output is:

MyClass created
GC Started
MyClass finalized
GC Finished
IsAlive: False

So if the local variable actually goes out of scope, it is properly collected and finalized.

The spec however tells that setting variable to null should make it eligible for garbage collection, so it's either a bug in GC, or a good reason to revise the spec more.

@Vaskaran
Copy link
Author

@MSDN-WhiteKnight,
Exactly. This is the way my ex-college Harilal (harilalpr@gmail.com) also analyzed the same.
I just want to add that even WeakReference is not needed. You can introduce another class and use something like:

class A : IDisposable
{
public A()
{
Console.WriteLine("Inside A's constructor.");
using (Sample sample = new Sample())
{
//Some code
}
}
public void Dispose()
{
//GC.SuppressFinalize(this);
Console.WriteLine("A's Dispose() is called.");
//Release any other resource(s)
}
~A()
{
Console.WriteLine("A's Destructor is Called.");
}
}
where Sample class is same:

class Sample : IDisposable
{
// Other code
~Sample().
{
Console.WriteLine("Sample's Destructor is called.");

    }
}

And inside Main(), if you write:
A obA = new A();
obA = null;
Console.WriteLine("GC is about to start.");
GC.Collect();
GC.WaitForPendingFinalizers();
Console.WriteLine("GC is completed.");

You can see the destructor message from Sample class object.

So, the bottom line is: How GC works is in question? You summarised and explained it nicely in last few lines.I'll echo the same. And this issue is NOT the same as 17463.

Regards,
Vaskaran

@Maoni0
Copy link
Member

Maoni0 commented May 29, 2021

doesn't have anything to do with the GC. it's the lifetime tracking in JIT.

@Vaskaran
Copy link
Author

Vaskaran commented Jun 1, 2021

@ Maoni:
"doesn't have anything to do with the GC. it's the lifetime tracking in JIT." -Does it mean that we should expect a fix in the tracking mechanism in JIT?

@MSDN-WhiteKnight
Copy link
Contributor

Before we can expect the JIT fix, we must file an issue properly. Or, at first, figure out is it even a real bug. If i create many objects in a loop, they actually start to get collected at some point, even if inside the same scope. So maybe we are hitting an edge case that only matters in small test programs.

The interesting detail, however, that in .NET Core 2.1 the issue vanishes in release mode (when optimizations are on). And .NET Framework x64 JIT is also affected, and again, only in debug mode. Might be related to the fact that debug-mode JIT extends lifetime of some objects to the end of function to make debugging experience better, but what went wrong starting from .NET Core 3.x so release mode is also affected?

@BillWagner
Copy link
Member

Adding @JulieLeeMSFT

Should this be a doc change, or is it an issue for the JIT lifetime tracking?

@gewarren gewarren added product-question Product usage related questions [org][type][category] and removed duplicate Indicates issues or PRs that are duplicate of existing ones [org][type][category] labels Jun 1, 2021
@Maoni0
Copy link
Member

Maoni0 commented Jun 1, 2021

so far I haven't seen anything that says there is an issue. JIT is free to extend the lifetime of a local var till the end of the scope and it can change how it reports the lifetime from release to release.

Julie is OOF right now. please see discussion on this issue: dotnet/runtime#36265

@MSDN-WhiteKnight
Copy link
Contributor

There is issue in that C# spec contains example that sets variable to null to demonstrate how it becomes eligible for garbage collection: https://github.com/dotnet/csharpstandard/blob/draft-v6/standard/basic-concepts.md#89-automatic-memory-management

static void Main() {
    B b = new B(new A());
    b = null;
    GC.Collect();
    GC.WaitForPendingFinalizers();
}

... These objects become eligible for garbage collection when the variable b is assigned the value null, since after this time it is impossible for any user-written code to access them.

On .NET Core 3.1+ finalizers don't run and instead of expected output it just prints nothing. While the spec does warn that implementations can have different behaviour, the value of this example is pretty low if the latest implementation with default both debug & release configs does not match.

@Vaskaran
Copy link
Author

Vaskaran commented Jun 2, 2021

@Whiteknight,
Unfortunately, you and svick are bringing the right issue to everyone's notice, but in between the discussion it is being lost. If anyone sees the original program and svick's reply, the problem is crystal clear. Bill also asks the right question: Either we need to fix the issue or we need to update the spec. But unfortunately, the problem is trying to be attached with some old/existing CR's where the issues are not exactly the same, yes I repeat: there are differences.
Anyways, it is up to you -how to accept the fact or not. But let me add few more points that can help you analyze more :
1.As suggested in 36265, I made TieredCompilation false, but the problem persists.
2.I enforced generation-wise garbage collection: first, gen(0), then gen(1), and then gen(2); but what I can see that the sample object (which is made null inside Main() already) is never collected at all. Instead, it is promoting to generation 2 at the end and stays there always.

@Maoni0
Copy link
Member

Maoni0 commented Jun 2, 2021

@MSDN-WhiteKnight please work with jit folks to update the C# doc. looping in @AndyAyersMS.

@MSDN-WhiteKnight
Copy link
Contributor

Anyways, it is up to you -how to accept the fact or not.

Why is it up to me? It's the .NET team who can decide whether it's issue or not. I'm not even an active contributor neither for JIT nor for C# spec.

As suggested in 36265, I made TieredCompilation false, but the problem persists.

Note that you must build the release config, or in other way enable optimizations in JIT. And MSBuild property TieredCompilation seems to not work for some reason, but the env. variable works. I.e. you need to execute commands like this (on Windows) to see the issue vanishing:

dotnet build -c Release
SET COMPlus_TieredCompilation=0
.\bin\Release\netcoreapp3.1\NetCoreTest.exe

.I enforced generation-wise garbage collection: first, gen(0), then gen(1), and then gen(2); but what I can see that the sample object (which is made null inside Main() already) is never collected at all. Instead, it is promoting to generation 2 at the end and stays there always.

Generations clearly does not matter here. Like Maoni written, it is determined in JIT's gcinfo that knows nothing about generations, not in GC per se.

@Vaskaran
Copy link
Author

Vaskaran commented Jun 2, 2021

"Anyways, it is up to you -how to accept the fact or not.
Why is it up to me? It's the .NET team who can decide whether it's an issue or not. I'm not even an active contributor neither for JIT nor for C# spec."

[Vaskaran] I meant .NET teams. Probably I needed to make it clear. So, its my mistake. I acknowledge it.

"As suggested in 36265, I made TieredCompilation false, but the problem persists.
Note that you must build the release config, etc..."

[Vaskaran] Nice info. But as an end-user for a simple program, if we need to do so many things, I am not sure whether it's a good advertisement. But I can be wrong!

"I enforced generation-wise garbage collection: first, gen(0), then gen(1), and then gen(2); but what I can see that the sample object (which is made null inside Main() already) is never collected at all. Instead, it is promoting to generation 2 at the end and stays there always.
Generations clearly does not matter here. Like Maoni written,..etc."

[Vaskaran] I wanted to emphasize the fact that these local objects are staying. Analyzing true memory leaks can be problematic in certain situations like this. Is this a minor issue? I am NOT sure...

Additional:
"I'm not even an active contributor neither for JIT nor for C# spec."
-I do not know that. Our colleagues suggested that this is a place where you meet the experts. So, I brought this issue here. But this issue was tried to mix with others...Anyways, bringing this issue to everyone's notice may cause some unwanted things. And if my comments hurt anyone, I say a big SORRY to him...

---Whether it is treated as an issue or not that does not hamper my day to day work. It's OK whatever the .NET team decides.

@MSDN-WhiteKnight
Copy link
Contributor

But as an end-user for a simple program, if we need to do so many things, I am not sure whether it's a good advertisement. But I can be wrong!

It indeed feels somewhat wrong that release builds now get the same lifetime rules as debug by default. But the tiered JIT was introduced for a reason, it improves startup time, so it's a tradeoff. For small programs the improved startup time can be more useful then collecting some objects earlier.

I wanted to emphasize the fact that these local objects are staying. Analyzing true memory leaks can be problematic in certain situations like this. Is this a minor issue? I am NOT sure...

That's a good point. At first when i discovered this issue i thought it would cause massive memory leaks. I tried a program that creates many instances of the object in the same function, i.e. while(true) x=new MyClass();, where the value of x is immediatly discarded. And after there were ~128K objects created, finalizers actually started executing, and memory consumption wasn't growing infinitely. So probably JIT has some magic that prevents accumulating a lot of uncollected objects with extended lifetime.

And if my comments hurt anyone, I say a big SORRY to him...

No problem, i wasn't hurt by your comments.

@Vaskaran
Copy link
Author

Vaskaran commented Jun 2, 2021

Thanks for your thoughts.
"And after there were ~128K objects created, finalizers actually started executing, and memory consumption wasn't growing infinitely. So probably JIT has some magic that prevents accumulating a lot of uncollected objects with extended lifetime."
[Vaskaran]Let me present you with a true case study. I worked in an organization that has 20+ printer devices. The memory capacity of these devices vary. You can surely assume that those memory spaces are precious. Now let us assume they migrate from .NET Framework to .NET Core/.NET5/6. I can tell you that the memory leak suite will fail for many of these devices if we need to wait for finalizers to start executing until ~128K objects are there. So, unless there is a clear guideline/doc, I am wondering how to deal with such scenarios.[I know there can be some code changes/workarounds, but I want to say that a clean migration seems to be tough for them.]

@AndyAyersMS
Copy link
Member

The behavior of the GC reporting in the JIT can be a bit confusing to understand. The only guarantee (and this is true going back to the very early days of .NET) is that there will not be any "hidden" references to objects allocated by a method once the method returns. With in a method, exactly which objects might end up with hidden references is undefined, and (as noted above) can and does change based on compilation option and the version of .NET used.

So if you are doing some allocation and want to guarantee the items allocated are not kept alive by hidden JIT references, you must mark the allocating method with [MethodImpl(MethodImplOptions.NoInlining)] and then actually return from the method.

In particular, this means simple allocation tests using Main like the ones earlier in this issue are problematic.

@AndyAyersMS
Copy link
Member

This behavior of the JIT almost never leads to problems in realistic applications. Typically methods return with enough frequency that the extra GC references the JIT creates have no real impact.

@Vaskaran I assume your case study example above is hypothetical and you haven't actually observed this being a problem?

@Vaskaran
Copy link
Author

Vaskaran commented Jun 3, 2021

Hi All,
I showed you a .NET issue a few days back and I wanted to understand this behavior from the experts like you.
Then we have seen many valuable insights. I am thankful to all of you for all these valuable inputs.
In between, I discussed this with my ex-colleagues and gurus about the same and showed them this discussion.
Here is the summary:
1.Some of us believe that it is an issue, but we can live with it. They believe that JIT is smart enough to handle the case.
2.Some of us believe that it is not an issue at all. For them: everything is working as expected.
3.Some of us believe that it is surely an issue, and it could be better if it is documented at least.

One last point: The example I gave you was NOT hypothetical, but as per my company policy, I doubt whether I should dig this in further detail.

--- So, as a creator of this issue, I can say that I do not have anything more to add at this moment. If I face any questions regarding this behavior, I can refer them to this link; so that they can reach their own conclusion. I also believe that in the future, if you believe that you will work on this issue, surely you'll do that. In between, I thank everybody one more time.

@BillWagner
Copy link
Member

Thanks for the thoughtful discussions from everyone. I'm going to close this without action. However, I'll add a bit on my reasoning.

The updated standard (see dotnet/csharpstandard#309 for the changes) accurately reflects the behavior.

@Vaskaran I'll point to a couple resources that you should find valuable in any updates needed based on the scenario you describe:

  • The Garbage collection section of the .NET Guide describes the implementation of the garbage collector, and how to tune its behavior. This section contains a lot of detail on the questions raised in this thread.
  • @Maoni0 has written a detailed guide for .NET Memory Performance analysis.

Between those resources, you should have all the tools needed to explore, diagnose, and make any updates to your applications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pri3 product-question Product usage related questions [org][type][category]
Projects
None yet
Development

No branches or pull requests

9 participants