-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Memory usage regression #73254
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to this area: @dotnet/gc Issue DetailsI am noticing an important regression in GC heap sizes and working set measurement on ASP.NET benchmarks (most of them). These are the charts for Json MVC.
One of these changes impacted some GC work: #72923
|
does this use Server GC? If so it's not affected by #72923. |
The benchmarks use Server GC. Is there a slight chance that it's still affecting it, because I can repro consistently with just this changeset, and all the other commits have nothing to do with the GC. Is there a trace I can provide that would confirm it? |
Could you provide instructions on how to repro? I can try to see if I see the same error locally with and without the change. |
Can confirm it's #72923 (I sent patched binary to crank where I reverted that PR)
|
I took another look at 72923 and I do see a problem with it for WKS GC. it needs to initialize the saved_allocated in the else case when |
Confirming it's running with Server GC, we do output the value during the run to be certain it is doing so: http://asp-citrine-win:5001/jobs/245/output:
And the code that displays this line: https://github.com/aspnet/Benchmarks/blob/main/src/Benchmarks/Program.cs#L197 Console.WriteLine($"Server GC is currently {(GCSettings.IsServerGC ? "ENABLED" : "DISABLED")}"); |
do you know why I'm getting this?
|
Maybe you have installed nightly bits (preview7) but not the full SDK (with aligned runtime/aspnet). |
I see, that makes sense. next Q is where do I install the full SDK from? I installed preview7 from https://github.com/dotnet/installer. |
If you install nightly SDK builds, you need to manually add nightly nuget feed to your nuget config: https://github.com/dotnet/runtime/blob/main/docs/project/dogfooding.md#obtaining-daily-builds-of-nuget-packages |
@Maoni0: I was able to repro the issue using (thanks @EgorBo):
and found that reverting the following on line number 28726 in
back to:
Fixes the memory issue. This is one of the cases that isn't in WKS. Continuing to investigate why this is the case. |
oh, I see the problem......... you missed an 'S' in the #ifdef
is supposed to be
this change is supposed to not have any effect on Server GC because
for Server GC. |
I am noticing an important regression in GC heap sizes and working set measurement on ASP.NET benchmarks (most of them).
These are the charts for Json MVC.
An early range of changes points to these commits: cfe26ec...68bd58eSmallest set of changes reproducing the regression: cfe26ec...e415018
One of these changes impacted some GC work: #72923
The text was updated successfully, but these errors were encountered: