Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Add property HardLimitBytes to GCMemoryInfo #25437

Merged
merged 7 commits into from Jul 8, 2019
Merged

Add property HardLimitBytes to GCMemoryInfo #25437

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9035562
Add property HardLimitBytes to GCMemoryInfo
Jun 25, 2019
b095f24
Remove HardLimitBytes; have TotalAvailableMemoryBytes take on its beh…
Jun 28, 2019
a9c48ae
Fix typos
Jun 28, 2019
6447db8
Separate total_physical_mem and heap_hard_limit
Jul 2, 2019
c485038
Do more work in gc.cpp instead of Gc.cs
Jul 2, 2019
cb8f824
Consistently end names in "Bytes"
Jul 2, 2019
a92324c
Merge remote-tracking branch 'upstream/master' into just_hardlimit_ch…
Jul 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/System.Private.CoreLib/shared/System/GCMemoryInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ public readonly struct GCMemoryInfo
public long MemoryLoadBytes { get; }

/// <summary>
/// Total available memory for the GC to use when the last GC ocurred. By default this is the physical memory on the machine, but it may be customized by specifying a HardLimit.
/// Total available memory for the GC to use when the last GC ocurred.
///
/// If the environment variable COMPlus_GCHeapHardLimit is set,
jkotas marked this conversation as resolved.
Show resolved Hide resolved
/// or "Server.GC.HeapHardLimit" is in runtimeconfig.json, this will come from that.
/// If the program is run in a container, this will be an implementation-defined fraction of the container's size.
/// Else, this is the physical memory on the machine that was available for the GC to use when the last GC occurred.
/// </summary>
public long TotalAvailableMemoryBytes { get; }

Expand Down
39 changes: 21 additions & 18 deletions src/System.Private.CoreLib/src/System/GC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,26 +54,28 @@ public enum GCNotificationStatus
public static class GC
{
[MethodImplAttribute(MethodImplOptions.InternalCall)]
internal static extern void GetMemoryInfo(out uint highMemLoadThreshold,
out ulong totalPhysicalMem,
out uint lastRecordedMemLoad,
internal static extern void GetMemoryInfo(out ulong highMemLoadThresholdBytes,
out ulong totalAvailableMemoryBytes,
out ulong lastRecordedMemLoadBytes,
out uint lastRecordedMemLoadPct,
// The next two are size_t
out UIntPtr lastRecordedHeapSize,
out UIntPtr lastRecordedFragmentation);
out UIntPtr lastRecordedHeapSizeBytes,
out UIntPtr lastRecordedFragmentationBytes);

public static GCMemoryInfo GetGCMemoryInfo()
{
GetMemoryInfo(out uint highMemLoadThreshold,
out ulong totalPhysicalMem,
out uint lastRecordedMemLoad,
out UIntPtr lastRecordedHeapSize,
out UIntPtr lastRecordedFragmentation);
GetMemoryInfo(out ulong highMemLoadThresholdBytes,
out ulong totalAvailableMemoryBytes,
out ulong lastRecordedMemLoadBytes,
out uint _,
out UIntPtr lastRecordedHeapSizeBytes,
out UIntPtr lastRecordedFragmentationBytes);

return new GCMemoryInfo((long)((double)highMemLoadThreshold / 100 * totalPhysicalMem),
(long)((double)lastRecordedMemLoad / 100 * totalPhysicalMem),
(long)totalPhysicalMem,
(long)(ulong)lastRecordedHeapSize,
(long)(ulong)lastRecordedFragmentation);
return new GCMemoryInfo(highMemoryLoadThresholdBytes: (long)highMemLoadThresholdBytes,
memoryLoadBytes: (long)lastRecordedMemLoadBytes,
totalAvailableMemoryBytes: (long)totalAvailableMemoryBytes,
heapSizeBytes: (long)(ulong)lastRecordedHeapSizeBytes,
fragmentedBytes: (long)(ulong)lastRecordedFragmentationBytes);
}

[DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
Expand Down Expand Up @@ -541,13 +543,14 @@ public MemoryLoadChangeNotification(float lowMemoryPercent, float highMemoryPerc

private static float GetMemoryLoad()
{
GetMemoryInfo(out uint _,
GetMemoryInfo(out ulong _,
out ulong _,
out uint lastRecordedMemLoad,
out ulong _,
out uint lastRecordedMemLoadPct,
out UIntPtr _,
out UIntPtr _);

return (float)lastRecordedMemLoad / 100;
return (float)lastRecordedMemLoadPct;
}

private static bool InvokeMemoryLoadChangeNotifications()
Expand Down
24 changes: 13 additions & 11 deletions src/gc/gc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36150,17 +36150,19 @@ unsigned int GCHeap::GetCondemnedGeneration()
return gc_heap::settings.condemned_generation;
}

void GCHeap::GetMemoryInfo(uint32_t* highMemLoadThreshold,
uint64_t* totalPhysicalMem,
uint32_t* lastRecordedMemLoad,
size_t* lastRecordedHeapSize,
size_t* lastRecordedFragmentation)
{
*highMemLoadThreshold = gc_heap::high_memory_load_th;
*totalPhysicalMem = gc_heap::total_physical_mem;
*lastRecordedMemLoad = gc_heap::last_gc_memory_load;
*lastRecordedHeapSize = gc_heap::last_gc_heap_size;
*lastRecordedFragmentation = gc_heap::last_gc_fragmentation;
void GCHeap::GetMemoryInfo(uint64_t* highMemLoadThresholdBytes,
uint64_t* totalAvailableMemoryBytes,
uint64_t* lastRecordedMemLoadBytes,
uint32_t* lastRecordedMemLoadPct,
size_t* lastRecordedHeapSizeBytes,
size_t* lastRecordedFragmentationBytes)
{
*highMemLoadThresholdBytes = (uint64_t) (((double)gc_heap::high_memory_load_th) / 100 * gc_heap::total_physical_mem);
*totalAvailableMemoryBytes = gc_heap::heap_hard_limit != 0 ? gc_heap::heap_hard_limit : gc_heap::total_physical_mem;
*lastRecordedMemLoadBytes = (uint64_t) (((double)gc_heap::last_gc_memory_load) / 100 * gc_heap::total_physical_mem);
*lastRecordedMemLoadPct = gc_heap::last_gc_memory_load;
Copy link
Member

@jkotas jkotas Jul 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Is the last_gc_memory_load relative to total_physical_mem; or relative to *totalAvailableMemoryBytes ?

Either this or the previous line looks wrong. When memory load is 100%, I think loadBytes should be equal totalAvailableMemoryBytes.

It may be best to get rid of the *lastRecordedMemLoadPct argument, and just divide lastRecordedMemLoadBytes and totalAvailableMemoryBytes on the CoreLib side.

Copy link
Member

Choose a reason for hiding this comment

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

last_gc_memory_load is relative to total_physical_mem. the problem is we do have a GetMemoryLoad that needs the actual memory load.

I would really prefer our original API proposal that had both the total physical memory and the hardlimit for the GC heap 'cause we do care about both. and I don't think it's unreasonable for the users to know about both. either that or we have another API (internal) that gets the actual physical memory load.

Copy link
Author

Choose a reason for hiding this comment

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

last_gc_memory_load is relative to total_physical_mem. If totalAvailableMemoryBytes is less than total_physical_mem, then it will be impossible for last_gc_memory_load to ever reach 100% -- it would be at most totalAvailableMemoryBytes / total_physical_mem * 100.

Copy link
Member

@jkotas jkotas Jul 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The primary reason why we have introduced the GCMemoryInfo API is to allow folks to build self-tuning caches.

Could you please review the places where this API is used currently (our TlsOverPerCoreLockedStacksArrayPool in particular - not sure whether there is anything else) and validate that it works correctly in all cases (regular VMs, containers, hardlimit)?

The current code for this is here:

coreclr/src/System.Private.CoreLib/shared/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs

Line 303 in 4ca032d

GCMemoryInfo memoryInfo = GC.GetGCMemoryInfo();
. Is this code correct for all situations? If not, what is the code that we want people to write to implement this logic? Once we know what code we want people to write here, we should be clear about what these APIs needs to do.

GetMemoryLoad that needs the actual memory load.

This is used by the internal experimental MemoryLoadChangeNotification APIs. Is this API used by anything?

Copy link
Member

Choose a reason for hiding this comment

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

the impl in TlsOverPerCoreLockedStacksArrayPool.cs looks correct - it could be improved (for example it could say only trim if the size it trims could actually make a difference in memory pressure) but I don't think we are doing that for 3.0.

it seems awkward to have to fit the 2 numbers (actual physical memory and hardlimit) into 1. and if we don't want to have to call the diagnostics API to retrieve the physical memory (which also seems awkward for GC) our original proposal seems more sensible to me.

Copy link
Member

Choose a reason for hiding this comment

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

What logic do we think would need to look at physical memory rather than the hard limit?

Copy link
Member

@jkotas jkotas Jul 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We have received a lot of feedback in 2.x about ArrayPool keeping too much memory over time. We have seen people taking extreme measures like poking the ArrayPool via private reflection to make it release the memory. We have implemented the trimming at the last minute to mitigate the situation.

If the ArrayPool trimming does not take the hardlimit into account, I expect that we are going to hear the same feedback again from folks who try to use the hardlimit.

Copy link
Member

Choose a reason for hiding this comment

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

Right I'm agreeing with your assessment. I'm mostly wondering when you'd need the actual physical memory vs the hard limit (the other should probably be exposed where we have the other process specific APIs Process.*)

Copy link
Member

Choose a reason for hiding this comment

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

(My reply was for Maoni's comment.)

I agree that the actual physical memory does not seem to be necessary to build decent trimming heuristic.

I think that there are 4 numbers to feed into a decent basic trimming heuristic:

  • The GC heap size: current value and limit that it should stay under. I believe it is HeapSizeBytes and TotalAvailableMemoryBytes in the current API.
  • The memory load (ie the GC heap size plus everything else): current value and limit that it should stay under. I believe it is MemoryLoadBytes and HighMemoryLoadThresholdBytes in the current API.

When either of these gets too close to its limit, it is a signal to trim caches.

Copy link
Member

Choose a reason for hiding this comment

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

I have opened https://github.com/dotnet/coreclr/issues/25552 on ArrayPool not respecting GCHardLimit.

*lastRecordedHeapSizeBytes = gc_heap::last_gc_heap_size;
*lastRecordedFragmentationBytes = gc_heap::last_gc_fragmentation;
}

int GCHeap::GetGcLatencyMode()
Expand Down
11 changes: 6 additions & 5 deletions src/gc/gcimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,12 @@ class GCHeap : public IGCHeapInternal

unsigned GetCondemnedGeneration();

void GetMemoryInfo(uint32_t* highMemLoadThreshold,
uint64_t* totalPhysicalMem,
uint32_t* lastRecordedMemLoad,
size_t* lastRecordedHeapSize,
size_t* lastRecordedFragmentation);
void GetMemoryInfo(uint64_t* highMemLoadThresholdBytes,
uint64_t* totalAvailableMemoryBytes,
uint64_t* lastRecordedMemLoadBytes,
uint32_t* lastRecordedMemLoadPct,
size_t* lastRecordedHeapSizeBytes,
size_t* lastRecordedFragmentationBytes);

int GetGcLatencyMode();
int SetGcLatencyMode(int newLatencyMode);
Expand Down
11 changes: 6 additions & 5 deletions src/gc/gcinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,12 @@ class IGCHeap {
// lastRecordedMemLoad - physical memory load in percentage recorded in the last GC
// lastRecordedHeapSize - total managed heap size recorded in the last GC
// lastRecordedFragmentation - total fragmentation in the managed heap recorded in the last GC
virtual void GetMemoryInfo(uint32_t* highMemLoadThreshold,
uint64_t* totalPhysicalMem,
uint32_t* lastRecordedMemLoad,
size_t* lastRecordedHeapSize,
size_t* lastRecordedFragmentation) = 0;
virtual void GetMemoryInfo(uint64_t* highMemLoadThresholdBytes,
uint64_t* totalPhysicalMemoryBytes,
uint64_t* lastRecordedMemLoadBytes,
uint32_t* lastRecordedMemLoadPct,
size_t* lastRecordedHeapSizeBytes,
size_t* lastRecordedFragmentationBytes) = 0;

// Gets the current GC latency mode.
virtual int GetGcLatencyMode() = 0;
Expand Down
8 changes: 4 additions & 4 deletions src/vm/comutilnative.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,15 +886,15 @@ UINT64 GCInterface::m_remPressure[NEW_PRESSURE_COUNT] = {0, 0, 0, 0}; // his
// (m_iteration % NEW_PRESSURE_COUNT) is used as an index into m_addPressure and m_remPressure
UINT GCInterface::m_iteration = 0;

FCIMPL5(void, GCInterface::GetMemoryInfo, UINT32* highMemLoadThreshold, UINT64* totalPhysicalMem, UINT32* lastRecordedMemLoad, size_t* lastRecordedHeapSize, size_t* lastRecordedFragmentation)
FCIMPL6(void, GCInterface::GetMemoryInfo, UINT64* highMemLoadThreshold, UINT64* totalAvailableMemoryBytes, UINT64* lastRecordedMemLoadBytes, UINT32* lastRecordedMemLoadPct, size_t* lastRecordedHeapSizeBytes, size_t* lastRecordedFragmentationBytes)
{
FCALL_CONTRACT;

FC_GC_POLL_NOT_NEEDED();

return GCHeapUtilities::GetGCHeap()->GetMemoryInfo(highMemLoadThreshold, totalPhysicalMem,
lastRecordedMemLoad,
lastRecordedHeapSize, lastRecordedFragmentation);
return GCHeapUtilities::GetGCHeap()->GetMemoryInfo(highMemLoadThreshold, totalAvailableMemoryBytes,
lastRecordedMemLoadBytes, lastRecordedMemLoadPct,
lastRecordedHeapSizeBytes, lastRecordedFragmentationBytes);
}
FCIMPLEND

Expand Down
2 changes: 1 addition & 1 deletion src/vm/comutilnative.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class GCInterface {
static FORCEINLINE UINT64 InterlockedAdd(UINT64 *pAugend, UINT64 addend);
static FORCEINLINE UINT64 InterlockedSub(UINT64 *pMinuend, UINT64 subtrahend);

static FCDECL5(void, GetMemoryInfo, UINT32* highMemLoadThreshold, UINT64* totalPhysicalMem, UINT32* lastRecordedMemLoad, size_t* lastRecordedHeapSize, size_t* lastRecordedFragmentation);
static FCDECL6(void, GetMemoryInfo, UINT64* highMemLoadThresholdBytes, UINT64* totalAvailableMemoryBytes, UINT64* lastRecordedMemLoadBytes, UINT32* lastRecordedMemLoadPct, size_t* lastRecordedHeapSizBytes, size_t* lastRecordedFragmentationBytes);
static FCDECL0(int, GetGcLatencyMode);
static FCDECL1(int, SetGcLatencyMode, int newLatencyMode);
static FCDECL0(int, GetLOHCompactionMode);
Expand Down