[mono] Fix sgen_gc_info.memory_load_bytes

[radekdoulik]

Set it to used memory size instead of available memory size

[ghost]

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Set it to used memory size instead of available memory size

Author:	radekdoulik
Assignees:	-
Labels:	area-GC-mono
Milestone:	-

[naricc]

Looks good, thanks for fixing this.

[radekdoulik]

Let see how the CI build will look. I think we might need to update also the mono_determine_physical_ram_available_size to not return 0 in case the information is not available. And also handle cases where sysconf returns -1.

[lateralusX]

Let me try this tomorrow together with EventPipe and RuntimeEventSource since it presents a bunch of GC data to tools like dotnet-counters.

[lateralusX]

Looks like RuntimeEventSource won't be affected since this metric is currently not published as an EventSource counter.

[lateralusX]

Did we check this property against coreclr GC? Just looking at the documentation won't tell what is actually returned, looking at coreclr GC implementation of the property it is calculated like this:

*lastRecordedMemLoadBytes = (uint64_t) (((double)(last_gc_info->memory_load)) / 100 * gc_heap::total_physical_mem);

where last_gc_info->memory_load seems to either be memory load on last GC enter/exit representing memory load on process/system, coming from GCToOSInterface::GetMemoryStatus that has the following comment,

// Get memory status
// Parameters:
// restricted_limit - The amount of physical memory in bytes that the current process is being restricted to. If non-zero, it used to calculate
// memory_load and available_physical. If zero, memory_load and available_physical is calculate based on all available memory.
// memory_load - A number between 0 and 100 that specifies the approximate percentage of physical memory
// that is in use (0 indicates no memory use and 100 indicates full memory use).
// available_physical - The amount of physical memory currently available, in bytes.
// available_page_file - The maximum amount of memory the current process can commit, in bytes.

That calculation take into account if there are restrictions on amount of memory that can be used by GC.

and gc_heap::total_physical_mem is set to:

gc_heap::total_physical_mem = (size_t)GCConfig::GetGCTotalPhysicalMemory();

and that seems to return either total amount of physical memory or any restricted limitation setup.

[lateralusX]

On Windows it looks like at least one coreclr GC code path (ignoring memory restrictions and limitations) will just fall through using dwMemoryLoad here, https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-memorystatusex, and total physical is ullTotalPhys from that same struct, so then it will just calculate percent used out of total physical memory and that should be total physical bytes used, similar to our mono_determine_physical_ram_size() - mono_determine_physical_ram_available_size (), but we don't go over using the memory load (in percent) reducing some complexity in the calculations. Not sure how well this works with heap size restrictions on Mono though.

[lateralusX]

Setting a explicit max heap size will probably cause issues the way the calculation is currently done, since sgen_gc_info.total_available_memory_bytes is then set by user, but mono_determine_physical_ram_available_size will still reflect the amount of physical memory available on the system.

[radekdoulik]

Setting a explicit max heap size will probably cause issues the way the calculation is currently done, since sgen_gc_info.total_available_memory_bytes is then set by user, but mono_determine_physical_ram_available_size will still reflect the amount of physical memory available on the system.

Makes sense. I will change it to use mono_determine_physical_ram_size() in the calculation. I didn't notice the total_available_memory_bytes can be overriden.

[radekdoulik]

That might not work very well either. Let me think more about it.

[lateralusX]

Looks like CoreCLR uses different calculations when having heap size limitations, using process working set size in calculation, see GCToOSInterface::GetMemoryStatus.

[radekdoulik]

I made it to scale the memory load by total_available_memory_bytes/physical_ram_size. I think that might work for now and we can improve it later, if it leads to some issues?

The PhysicalMemoryMonitor should work ok with it.

[lateralusX]

At some point in time we should probably do calculation against process working set when having a heap max limit instead of using system global memory state, but if current fix solves memory monitor issue I believe it will give us a good enough representation of this metric and way better than what we had (that was doing the opposite to what it should have done).