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

ArrayPool trimming does not respect GCHardLimit #853

Open
Tracked by #64598 ...
jkotas opened this issue Jul 3, 2019 · 11 comments
Open
Tracked by #64598 ...

ArrayPool trimming does not respect GCHardLimit #853

jkotas opened this issue Jul 3, 2019 · 11 comments

Comments

@jkotas
Copy link
Member

jkotas commented Jul 3, 2019

From dotnet/coreclr#25437 (comment) :

Let's say I have 32GB physical memory with 16GB of free memory, set COMPlus_GCHeapHardLimit to 200MB, and start using array pool heavily. The array pool is going to see HighMemoryLoadThresholdBytes=~30GB and MemoryLoadBytes=~16GB, so it will not make any attempt to trim and it can cache way over 200MB

@Maoni0
Copy link
Member

Maoni0 commented Jul 4, 2019

actively setting the hardlimit config isn't a main scenario - hardlimit's main usage occurs in the container scenario. and we are very close to end of 3.0 so realistically this will not make it. we can try to get this in for 3.1 though (if the policy allows it).

@jkotas
Copy link
Member Author

jkotas commented Jul 5, 2019

Repro:

using System;
using System.Buffers;

class Program
{
   static void UsePool<T>(int blocks)
   {
       var pool = ArrayPool<T>.Shared;
       T[][] a = new T[blocks][];
       for (int size = 128; size <= 1024*1024; size *= 2)
       {
            for (int i = 0; i < a.Length; i++) (a[i] = pool.Rent(size)).AsSpan().Clear();
            for (int i = 0; i < a.Length; i++) pool.Return(a[i]);
            GC.Collect();
            System.Threading.Thread.Sleep(10);
       }
       var info = GC.GetGCMemoryInfo();
       Console.WriteLine($"Heap size: {info.HeapSizeBytes} Available Memory: {info.TotalAvailableMemoryBytes}");
   }

   static void Main(string[] args)
   {
        UsePool<byte>(40);
        UsePool<sbyte>(40);
        UsePool<char>(20);
        UsePool<int>(10);
        UsePool<uint>(10);
        UsePool<long>(5);
        UsePool<ulong>(5);        
   }
} 

This program is keeping no more than ~40MB alive at any given point. Run it with COMPlus_GCHeapHardLimit=0x10000000 (ie 256MB). It will fail half-way through with OutOfMemoryException because of the ArrayPool caches are keeping too much memory alive.

@Maoni0
Copy link
Member

Maoni0 commented Oct 31, 2019

CC @sergiy-k

@sergiy-k sergiy-k assigned ghost Nov 1, 2019
@jkotas jkotas transferred this issue from dotnet/coreclr Dec 13, 2019
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Buffers untriaged New issue has not been triaged by the area owner labels Dec 13, 2019
@GrabYourPitchforks GrabYourPitchforks added this to the 5.0 milestone Feb 21, 2020
@GrabYourPitchforks GrabYourPitchforks removed the untriaged New issue has not been triaged by the area owner label Feb 21, 2020
@adamsitnik adamsitnik modified the milestones: 5.0.0, Future Jul 6, 2020
@adamsitnik adamsitnik unassigned ghost Jul 6, 2020
@adamsitnik adamsitnik modified the milestones: Future, 6.0.0 Jul 6, 2020
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Apr 9, 2021
@tannergooding
Copy link
Member

Is this work going to be completed for 6.0.0? If not, we should adjust the milestone.

@deeprobin
Copy link
Contributor

Interesting Issue. @jkotas I saw that you merged already a PR.
What would be the further procedure now? Are you open to a community contribution here or does this change need a lot of GC experience?

@jkotas
Copy link
Member Author

jkotas commented May 29, 2022

I am not sure what PR you are talking about.

Fixing this likely requires a lot of experience. The first step is to propose a design for the fix (in English, no code).

@deeprobin
Copy link
Contributor

I am not sure what PR you are talking about.

dotnet/coreclr#25437

Fixing this likely requires a lot of experience. The first step is to propose a design for the fix (in English, no code).

Okay. Then this is the wrong issue for me :D

@davidfowl
Copy link
Member

Isn’t this a big problem when container limits are specified?

@deeprobin
Copy link
Contributor

Isn’t this a big problem when container limits are specified?

Is there any way to access the limits via the Docker API or somehow more directly via containerd?

@jkotas
Copy link
Member Author

jkotas commented May 29, 2022

Isn’t this a big problem when container limits are specified?

If your workload uses different ArrayPools a lot, you may need to reserve more memory for the container that what would be strictly required. Is this specific issue a big problem? I am not sure.

Is there any way to access the limits via the Docker API or somehow more directly via containerd?

The runtime reads the limits and configures the GC based on them today. The problem is not in reading the limits. The problem is in how the limits are applied across the system.

@jeffhandley jeffhandley removed this from the 7.0.0 milestone Jul 9, 2022
@jeffhandley jeffhandley added this to the 8.0.0 milestone Jul 9, 2022
@tannergooding tannergooding modified the milestones: 8.0.0, Future Aug 14, 2023
@tannergooding
Copy link
Member

Putting this as future. This doesn't seem to be a trivial problem to resolve and likely needs to happen early in a release when it does happen.

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

No branches or pull requests

10 participants