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.

Sign up for GitHub

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

Jump to bottom

Fix computing available memory on OSX for GC #97102

Merged
merged 2 commits into from
Jan 25, 2024
Merged

Fix computing available memory on OSX for GC #97102

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
449d29c
Fix computing available memory on OSX for GC
janvorli Jan 17, 2024
a5bf5df
Reflect PR feeback and move total memory computation to init
janvorli Jan 19, 2024
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
50 changes: 38 additions & 12 deletions src/coreclr/gc/unix/gcenv.unix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,17 @@ AffinitySet g_processAffinitySet;
extern "C" int g_highestNumaNode;
extern "C" bool g_numaAvailable;

#ifdef TARGET_APPLE
static int *g_kern_memorystatus_level_mib = NULL;
static size_t g_kern_memorystatus_level_mib_length = 0;
#endif

// Initialize the interface implementation
// Return:
// true if it has succeeded, false if it has failed
bool GCToOSInterface::Initialize()
{
bool success = true;
int pageSize = sysconf( _SC_PAGE_SIZE );

g_pageSizeUnixInl = uint32_t((pageSize > 0) ? pageSize : 0x1000);
Expand Down Expand Up @@ -317,7 +323,24 @@ bool GCToOSInterface::Initialize()

NUMASupportInitialize();

return true;
#ifdef TARGET_APPLE
const char* mem_free_name = "kern.memorystatus_level";
int rc = sysctlnametomib(mem_free_name, NULL, &g_kern_memorystatus_level_mib_length);
if (rc == 0)
{
g_kern_memorystatus_level_mib = (int*)malloc(g_kern_memorystatus_level_mib_length * sizeof(int));
Copy link
Member

Choose a reason for hiding this comment

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

presumably if this returns null then sysctlnametomib would return a failure?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for spotting the missing check, fixed.

rc = sysctlnametomib(mem_free_name, g_kern_memorystatus_level_mib, &g_kern_memorystatus_level_mib_length);
if (rc != 0)
{
free(g_kern_memorystatus_level_mib);
g_kern_memorystatus_level_mib = NULL;
g_kern_memorystatus_level_mib_length = 0;
success = false;
}
}
#endif

return success;
}

// Shutdown the interface implementation
Expand Down Expand Up @@ -1248,20 +1271,23 @@ uint64_t GetAvailablePhysicalMemory()

// Get the physical memory available.
#if defined(__APPLE__)
vm_size_t page_size;
mach_port_t mach_port;
mach_msg_type_number_t count;
vm_statistics_data_t vm_stats;
mach_port = mach_host_self();
count = sizeof(vm_stats) / sizeof(natural_t);
if (KERN_SUCCESS == host_page_size(mach_port, &page_size))
uint32_t mem_free = 0;
size_t mem_free_length = sizeof(uint32_t);
if (g_kern_memorystatus_level_mib != NULL)
Copy link
Member

Choose a reason for hiding this comment

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

if GCToOSInterface::Initialize succeeded, this should always be non null, correct? if so we can just assert.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed

{
if (KERN_SUCCESS == host_statistics(mach_port, HOST_VM_INFO, (host_info_t)&vm_stats, &count))
int rc = sysctl(g_kern_memorystatus_level_mib, g_kern_memorystatus_level_mib_length, &mem_free, &mem_free_length, NULL, 0);
if (rc == 0)
{
available = (int64_t)vm_stats.free_count * (int64_t)page_size;
int64_t mem_size = 0;
size_t mem_size_length = sizeof(int64_t);
int mib[] = { CTL_HW, HW_MEMSIZE };
rc = sysctl(mib, 2, &mem_size, &mem_size_length, NULL, 0);
Copy link
Member

Choose a reason for hiding this comment

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

do we need to get the total memory every time?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I've moved it to the init and also unified it with other total memory computations we had above.

if (rc == 0)
{
available = (int64_t)mem_free * mem_size / 100;
}
}
}
mach_port_deallocate(mach_task_self(), mach_port);
#elif defined(__FreeBSD__)
size_t inactive_count = 0, laundry_count = 0, free_count = 0;
size_t sz = sizeof(inactive_count);
Expand All @@ -1279,7 +1305,7 @@ uint64_t GetAvailablePhysicalMemory()

if (tryReadMemInfo)
{
// Ensure that we don't try to read the /proc/meminfo in successive calls to the GlobalMemoryStatusEx
// Ensure that we don't try to read the /proc/meminfo in successive calls to the GetAvailablePhysicalMemory
// if we have failed to access the file or the file didn't contain the MemAvailable value.
tryReadMemInfo = ReadMemAvailable(&available);
}
Expand Down
Loading