Improve performance of cgroup access (dotnet/coreclr#21229)

Currently, we create a CGroup instance on each request for getting
used or total physical memory. This has an extra cost of finding
filesystem paths of the current cgroup files. I have found that
if we do the initialization just once, the performance of getting
the used or total memory in a tight loop improves 22 times.

Accidentally, I was also looking into a perf regression in the
ByteMark.BenchBitOps test that was observed in the past, seemingly
related to the recent change to the way we get the used memory.
And I've found that the benchmark results improve two fold with
the change in this commit.

This change was made both in PAL and in the standalone GC.

Commit migrated from https://github.com/dotnet/coreclr/commit/4a6753dcacf44df6a8e91b91029e4b7a4f12d917

diff --git a/src/coreclr/src/gc/unix/cgroup.cpp b/src/coreclr/src/gc/unix/cgroup.cpp
index 3b337f5..b66323a 100644 (file)
--- a/src/coreclr/src/gc/unix/cgroup.cpp
+++ b/src/coreclr/src/gc/unix/cgroup.cpp
@@ -25,6 +25,8 @@ Abstract:
 #include <sys/resource.h>
 #include <errno.h>
 
+#include "cgroup.h"
+
 #ifndef SIZE_T_MAX
 #define SIZE_T_MAX (~(size_t)0)
 #endif
@@ -39,64 +41,64 @@ Abstract:
 
 class CGroup
 {
-    char* m_memory_cgroup_path;
-    char* m_cpu_cgroup_path;
+    static char* s_memory_cgroup_path;
+    static char* s_cpu_cgroup_path;
 public:
-    CGroup()
+    static void Initialize()
     {
-        m_memory_cgroup_path = FindCgroupPath(&IsMemorySubsystem);
-        m_cpu_cgroup_path = FindCgroupPath(&IsCpuSubsystem);
+        s_memory_cgroup_path = FindCgroupPath(&IsMemorySubsystem);
+        s_cpu_cgroup_path = FindCgroupPath(&IsCpuSubsystem);
     }
 
-    ~CGroup()
+    static void Cleanup()
     {
-        free(m_memory_cgroup_path);
-        free(m_cpu_cgroup_path);
+        free(s_memory_cgroup_path);
+        free(s_cpu_cgroup_path);
     }
 
-    bool GetPhysicalMemoryLimit(size_t *val)
+    static bool GetPhysicalMemoryLimit(size_t *val)
     {
         char *mem_limit_filename = nullptr;
         bool result = false;
 
-        if (m_memory_cgroup_path == nullptr)
+        if (s_memory_cgroup_path == nullptr)
             return result;
 
-        size_t len = strlen(m_memory_cgroup_path);
+        size_t len = strlen(s_memory_cgroup_path);
         len += strlen(MEM_LIMIT_FILENAME);
         mem_limit_filename = (char*)malloc(len+1);
         if (mem_limit_filename == nullptr)
             return result;
 
-        strcpy(mem_limit_filename, m_memory_cgroup_path);
+        strcpy(mem_limit_filename, s_memory_cgroup_path);
         strcat(mem_limit_filename, MEM_LIMIT_FILENAME);
         result = ReadMemoryValueFromFile(mem_limit_filename, val);
         free(mem_limit_filename);
         return result;
     }
 
-    bool GetPhysicalMemoryUsage(size_t *val)
+    static bool GetPhysicalMemoryUsage(size_t *val)
     {
         char *mem_usage_filename = nullptr;
         bool result = false;
 
-        if (m_memory_cgroup_path == nullptr)
+        if (s_memory_cgroup_path == nullptr)
             return result;
 
-        size_t len = strlen(m_memory_cgroup_path);
+        size_t len = strlen(s_memory_cgroup_path);
         len += strlen(MEM_USAGE_FILENAME);
         mem_usage_filename = (char*)malloc(len+1);
         if (mem_usage_filename == nullptr)
             return result;
 
-        strcpy(mem_usage_filename, m_memory_cgroup_path);
+        strcpy(mem_usage_filename, s_memory_cgroup_path);
         strcat(mem_usage_filename, MEM_USAGE_FILENAME);
         result = ReadMemoryValueFromFile(mem_usage_filename, val);
         free(mem_usage_filename);
         return result;
     }
 
-    bool GetCpuLimit(uint32_t *val)
+    static bool GetCpuLimit(uint32_t *val)
     {
         long long quota;
         long long period;
@@ -317,7 +319,7 @@ private:
         return cgroup_path;
     }
     
-    bool ReadMemoryValueFromFile(const char* filename, size_t* val)
+    static bool ReadMemoryValueFromFile(const char* filename, size_t* val)
     {
         bool result = false;
         char *line = nullptr;
@@ -363,19 +365,19 @@ private:
         return result;
     }
 
-    long long ReadCpuCGroupValue(const char* subsystemFilename){
+    static long long ReadCpuCGroupValue(const char* subsystemFilename){
         char *filename = nullptr;
         bool result = false;
         long long val;
 
-        if (m_cpu_cgroup_path == nullptr)
+        if (s_cpu_cgroup_path == nullptr)
             return -1;
 
-        filename = (char*)malloc(strlen(m_cpu_cgroup_path) + strlen(subsystemFilename) + 1);
+        filename = (char*)malloc(strlen(s_cpu_cgroup_path) + strlen(subsystemFilename) + 1);
         if (filename == nullptr)
             return -1;
 
-        strcpy(filename, m_cpu_cgroup_path);
+        strcpy(filename, s_cpu_cgroup_path);
         strcat(filename, subsystemFilename);
         result = ReadLongLongValueFromFile(filename, &val);
         free(filename);
@@ -385,7 +387,7 @@ private:
         return val;
     }
 
-    bool ReadLongLongValueFromFile(const char* filename, long long* val)
+    static bool ReadLongLongValueFromFile(const char* filename, long long* val)
     {
         bool result = false;
         char *line = nullptr;
@@ -417,12 +419,24 @@ private:
     }
 };
    
+char *CGroup::s_memory_cgroup_path = nullptr;
+char *CGroup::s_cpu_cgroup_path = nullptr;
+
+void InitializeCGroup()
+{
+    CGroup::Initialize();
+}
+
+void CleanupCGroup()
+{
+    CGroup::Cleanup();
+}
+
 size_t GetRestrictedPhysicalMemoryLimit()
 {
-    CGroup cgroup;
     size_t physical_memory_limit;
  
-    if (!cgroup.GetPhysicalMemoryLimit(&physical_memory_limit))
+    if (!CGroup::GetPhysicalMemoryLimit(&physical_memory_limit))
          physical_memory_limit = SIZE_T_MAX;
 
     struct rlimit curr_rlimit;
@@ -454,13 +468,12 @@ bool GetPhysicalMemoryUsed(size_t* val)
     bool result = false;
     size_t linelen;
     char* line = nullptr;
-    CGroup cgroup;
 
     if (val == nullptr)
         return false;
 
     // Linux uses cgroup usage to trigger oom kills.
-    if (cgroup.GetPhysicalMemoryUsage(val))
+    if (CGroup::GetPhysicalMemoryUsage(val))
         return true;
 
     // process resident set size.
@@ -492,10 +505,8 @@ bool GetPhysicalMemoryUsed(size_t* val)
 
 bool GetCpuLimit(uint32_t* val)
 {
-    CGroup cgroup;
-
     if (val == nullptr)
         return false;
 
-    return cgroup.GetCpuLimit(val);
+    return CGroup::GetCpuLimit(val);
 }

diff --git a/src/coreclr/src/gc/unix/cgroup.h b/src/coreclr/src/gc/unix/cgroup.h
new file mode 100644 (file)
index 0000000..6e405ae
--- /dev/null
+++ b/src/coreclr/src/gc/unix/cgroup.h
@@ -0,0 +1,13 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+
+#ifndef __CGROUP_H__
+#define __CGROUP_H__
+
+void InitializeCGroup();
+void CleanupCGroup();
+
+#endif // __CGROUP_H__
+

diff --git a/src/coreclr/src/gc/unix/gcenv.unix.cpp b/src/coreclr/src/gc/unix/gcenv.unix.cpp
index 23a4935..b0877db 100644 (file)
--- a/src/coreclr/src/gc/unix/gcenv.unix.cpp
+++ b/src/coreclr/src/gc/unix/gcenv.unix.cpp
@@ -39,6 +39,7 @@
 #include <errno.h>
 #include <unistd.h> // sysconf
 #include "globals.h"
+#include "cgroup.h"
 
 #if defined(_ARM_) || defined(_ARM64_)
 #define SYSCONF_GET_NUMPROCS _SC_NPROCESSORS_CONF
@@ -118,6 +119,8 @@ bool GCToOSInterface::Initialize()
     }
 #endif // HAVE_MACH_ABSOLUTE_TIME
 
+    InitializeCGroup();
+
     return true;
 }
 
@@ -130,6 +133,8 @@ void GCToOSInterface::Shutdown()
     assert(ret == 0);
 
     munmap(g_helperPage, OS_PAGE_SIZE);
+
+    CleanupCGroup();
 }
 
 // Get numeric id of the current thread if possible on the

diff --git a/src/coreclr/src/pal/src/include/pal/cgroup.h b/src/coreclr/src/pal/src/include/pal/cgroup.h
new file mode 100644 (file)
index 0000000..1946692
--- /dev/null
+++ b/src/coreclr/src/pal/src/include/pal/cgroup.h
@@ -0,0 +1,37 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+/*++
+
+
+
+Module Name:
+
+    include/pal/cgroup.h
+
+Abstract:
+    
+    Header file for the CGroup related functions.
+    
+
+
+--*/
+
+#ifndef _PAL_CGROUP_H_
+#define _PAL_CGROUP_H_
+
+#ifdef __cplusplus
+extern "C"
+{
+#endif // __cplusplus
+
+void InitializeCGroup();
+void CleanupCGroup();
+
+#ifdef __cplusplus
+}
+#endif // __cplusplus
+
+#endif /* _PAL_CGROUP_H_ */
+

diff --git a/src/coreclr/src/pal/src/init/pal.cpp b/src/coreclr/src/pal/src/init/pal.cpp
index 8980082..119e611 100644 (file)
--- a/src/coreclr/src/pal/src/init/pal.cpp
+++ b/src/coreclr/src/pal/src/init/pal.cpp
@@ -44,6 +44,7 @@ SET_DEFAULT_DEBUG_CHANNEL(PAL); // some headers have code with asserts, so do th
 #include "pal/init.h"
 #include "pal/numa.h"
 #include "pal/stackstring.hpp"
+#include "pal/cgroup.h"
 
 #if HAVE_MACH_EXCEPTIONS
 #include "../exception/machexception.h"
@@ -387,6 +388,8 @@ Initialize(
             goto done;
         }
 
+        InitializeCGroup();
+
         // Initialize the environment.
         if (FALSE == EnvironInitialize())
         {
@@ -711,6 +714,7 @@ CLEANUP1a:
 CLEANUP1:
     SHMCleanup();
 CLEANUP0:
+    CleanupCGroup();
     TLSCleanup();
     ERROR("PAL_Initialize failed\n");
     SetLastError(palError);

diff --git a/src/coreclr/src/pal/src/misc/cgroup.cpp b/src/coreclr/src/pal/src/misc/cgroup.cpp
index 145586a..7fe5e59 100644 (file)
--- a/src/coreclr/src/pal/src/misc/cgroup.cpp
+++ b/src/coreclr/src/pal/src/misc/cgroup.cpp
@@ -17,6 +17,7 @@ SET_DEFAULT_DEBUG_CHANNEL(MISC);
 #include "pal/palinternal.h"
 #include <sys/resource.h>
 #include "pal/virtual.h"
+#include "pal/cgroup.h"
 #include <algorithm>
 
 #define PROC_MOUNTINFO_FILENAME "/proc/self/mountinfo"
@@ -28,64 +29,64 @@ SET_DEFAULT_DEBUG_CHANNEL(MISC);
 #define CFS_PERIOD_FILENAME "/cpu.cfs_period_us"
 class CGroup
 {
-    char *m_memory_cgroup_path;
-    char *m_cpu_cgroup_path;
+    static char *s_memory_cgroup_path;
+    static char *s_cpu_cgroup_path;
 public:
-    CGroup()
+    static void Initialize()
     {
-       m_memory_cgroup_path = FindCgroupPath(&IsMemorySubsystem);
-       m_cpu_cgroup_path = FindCgroupPath(&IsCpuSubsystem);
+        s_memory_cgroup_path = FindCgroupPath(&IsMemorySubsystem);
+        s_cpu_cgroup_path = FindCgroupPath(&IsCpuSubsystem);
     }
 
-    ~CGroup()
+    static void Cleanup()
     {
-        PAL_free(m_memory_cgroup_path);
-        PAL_free(m_cpu_cgroup_path);
+        PAL_free(s_memory_cgroup_path);
+        PAL_free(s_cpu_cgroup_path);
     }
     
-    bool GetPhysicalMemoryLimit(size_t *val)
+    static bool GetPhysicalMemoryLimit(size_t *val)
     {
         char *mem_limit_filename = nullptr;
         bool result = false;
 
-        if (m_memory_cgroup_path == nullptr)
+        if (s_memory_cgroup_path == nullptr)
             return result;
 
-        size_t len = strlen(m_memory_cgroup_path);
+        size_t len = strlen(s_memory_cgroup_path);
         len += strlen(MEM_LIMIT_FILENAME);
         mem_limit_filename = (char*)PAL_malloc(len+1);
         if (mem_limit_filename == nullptr)
             return result;
 
-        strcpy_s(mem_limit_filename, len+1, m_memory_cgroup_path);
+        strcpy_s(mem_limit_filename, len+1, s_memory_cgroup_path);
         strcat_s(mem_limit_filename, len+1, MEM_LIMIT_FILENAME);
         result = ReadMemoryValueFromFile(mem_limit_filename, val);
         PAL_free(mem_limit_filename);
         return result;
     }
 
-    bool GetPhysicalMemoryUsage(size_t *val)
+    static bool GetPhysicalMemoryUsage(size_t *val)
     {
         char *mem_usage_filename = nullptr;
         bool result = false;
 
-        if (m_memory_cgroup_path == nullptr)
+        if (s_memory_cgroup_path == nullptr)
             return result;
 
-        size_t len = strlen(m_memory_cgroup_path);
+        size_t len = strlen(s_memory_cgroup_path);
         len += strlen(MEM_USAGE_FILENAME);
         mem_usage_filename = (char*)malloc(len+1);
         if (mem_usage_filename == nullptr)
             return result;
 
-        strcpy(mem_usage_filename, m_memory_cgroup_path);
+        strcpy(mem_usage_filename, s_memory_cgroup_path);
         strcat(mem_usage_filename, MEM_USAGE_FILENAME);
         result = ReadMemoryValueFromFile(mem_usage_filename, val);
         free(mem_usage_filename);
         return result;
     }
 
-    bool GetCpuLimit(UINT *val)
+    static bool GetCpuLimit(UINT *val)
     {
         long long quota;
         long long period;
@@ -308,27 +309,27 @@ private:
         return cgroup_path;
     }
 
-    bool ReadMemoryValueFromFile(const char* filename, size_t* val)
+    static bool ReadMemoryValueFromFile(const char* filename, size_t* val)
     {
         return ::ReadMemoryValueFromFile(filename, val);
     }
 
-    long long ReadCpuCGroupValue(const char* subsystemFilename){
+    static long long ReadCpuCGroupValue(const char* subsystemFilename){
         char *filename = nullptr;
         bool result = false;
         long long val;
         size_t len;
 
-        if (m_cpu_cgroup_path == nullptr)
+        if (s_cpu_cgroup_path == nullptr)
             return -1;
 
-        len = strlen(m_cpu_cgroup_path);
+        len = strlen(s_cpu_cgroup_path);
         len += strlen(subsystemFilename);
         filename = (char*)PAL_malloc(len + 1);
         if (filename == nullptr)
             return -1;
 
-        strcpy_s(filename, len+1, m_cpu_cgroup_path);
+        strcpy_s(filename, len+1, s_cpu_cgroup_path);
         strcat_s(filename, len+1, subsystemFilename);
         result = ReadLongLongValueFromFile(filename, &val);
         PAL_free(filename);
@@ -338,7 +339,7 @@ private:
         return val;
     }
 
-    bool ReadLongLongValueFromFile(const char* filename, long long* val)
+    static bool ReadLongLongValueFromFile(const char* filename, long long* val)
     {
         bool result = false;
         char *line = nullptr;
@@ -368,15 +369,27 @@ private:
     }
 };
 
+char *CGroup::s_memory_cgroup_path = nullptr;
+char *CGroup::s_cpu_cgroup_path = nullptr;
+
+void InitializeCGroup()
+{
+    CGroup::Initialize();
+}
+
+void CleanupCGroup()
+{
+    CGroup::Cleanup();
+}
+
 
 size_t
 PALAPI
 PAL_GetRestrictedPhysicalMemoryLimit()
 {
-    CGroup cgroup;
     size_t physical_memory_limit;
 
-    if (!cgroup.GetPhysicalMemoryLimit(&physical_memory_limit))
+    if (!CGroup::GetPhysicalMemoryLimit(&physical_memory_limit))
          physical_memory_limit = SIZE_T_MAX;
 
     struct rlimit curr_rlimit;
@@ -411,13 +424,12 @@ PAL_GetPhysicalMemoryUsed(size_t* val)
     BOOL result = false;
     size_t linelen;
     char* line = nullptr;
-    CGroup cgroup;
 
     if (val == nullptr)
         return FALSE;
 
     // Linux uses cgroup usage to trigger oom kills.
-    if (cgroup.GetPhysicalMemoryUsage(val))
+    if (CGroup::GetPhysicalMemoryUsage(val))
         return TRUE;
 
     // process resident set size.
@@ -447,10 +459,8 @@ BOOL
 PALAPI
 PAL_GetCpuLimit(UINT* val)
 {
-    CGroup cgroup;
-
     if (val == nullptr)
         return FALSE;
 
-    return cgroup.GetCpuLimit(val);
+    return CGroup::GetCpuLimit(val);
 }