Make Bazel's RAM estimate container aware

fmeum · copybara-github · commit 2f3cdc530db1 · 2023-12-07T02:42:36.000-08:00
As of JDK 14, `OperatingSystemMXBean` provides information about system memory that is container-aware. Outside containers, it uses the same mechanisms as Bazel to determine available RAM (`/proc/meminfo` on Linux, `hw.memsize` on macOS) and can thus be used as a drop-in replacement for the custom implementation. A small caveat is that Bazel's macOS RAM estimate was based on converting bytes to "MB" via a divisor of `1000^2` instead of `1024^2`, resulting in a consistent overestimate compared to an identical Linux machine that is now corrected. This opportunity was missed in #16512 since `OperatingSystemMXBean` is based on a complete Java implementation of cgroups handling and doesn't go through the `os::total_memory` or `os::physical_memory` Hotspot functions. RELNOTES[INC]: * On Linux, Bazel's RAM estimate for the host machine is now aware of container resource limits. * On macOS, Bazel no longer consistently overestimates the total RAM by ~5% (`1024^2/1000^2`). * On Windows, Bazel's RAM estimate is now generally more accurate as it is no longer influenced by JVM heuristics. Fixes #3886 Closes #20435. PiperOrigin-RevId: 588718034 Change-Id: I2daafa0567740a1b149ca8756ec27f102129283c
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -427,18 +427,13 @@ java_library(
     name = "localhost_capacity",
     srcs = [
         "LocalHostCapacity.java",
-        "LocalHostResourceFallback.java",
-        "LocalHostResourceManagerDarwin.java",
-        "LocalHostResourceManagerLinux.java",
+        "LocalHostResource.java",
         "ResourceSet.java",
         "ResourceSetOrBuilder.java",
     ],
     deps = [
         ":exec_exception",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
-        "//src/main/java/com/google/devtools/build/lib/jni",
-        "//src/main/java/com/google/devtools/build/lib/unix",
-        "//src/main/java/com/google/devtools/build/lib/unix:procmeminfo_parser",
         "//src/main/java/com/google/devtools/build/lib/util:os",
         "//src/main/java/com/google/devtools/build/lib/worker:worker_key",
         "//src/main/java/com/google/devtools/common/options",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LocalHostCapacity.java b/src/main/java/com/google/devtools/build/lib/actions/LocalHostCapacity.java
@@ -17,7 +17,6 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.flogger.GoogleLogger;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
-import com.google.devtools.build.lib.util.OS;
 
 /**
  * This class estimates the local host's resource capacity.
@@ -26,7 +25,6 @@
 public final class LocalHostCapacity {
 
   private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
-  private static final OS currentOS = OS.getCurrent();
   private static ResourceSet localHostCapacity;
 
   private LocalHostCapacity() {}
@@ -39,21 +37,7 @@ public static ResourceSet getLocalHostCapacity() {
   }
 
   private static ResourceSet getNewLocalHostCapacity() {
-    ResourceSet localResources = null;
-    switch (currentOS) {
-      case DARWIN:
-        localResources = LocalHostResourceManagerDarwin.getLocalHostResources();
-        break;
-      case LINUX:
-        localResources = LocalHostResourceManagerLinux.getLocalHostResources();
-        break;
-      default:
-        break;
-    }
-    if (localResources == null) {
-      localResources = LocalHostResourceFallback.getLocalHostResources();
-    }
-
+    ResourceSet localResources = LocalHostResource.get();
     logger.atInfo().log(
         "Determined local resources: RAM=%dMB, CPU=%.1f",
         (int) localResources.getMemoryMb(), localResources.getCpuUsage());
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LocalHostResource.java b/src/main/java/com/google/devtools/build/lib/actions/LocalHostResource.java
@@ -14,22 +14,28 @@
 
 package com.google.devtools.build.lib.actions;
 
-/**
- * This class provide a fallback of the local host's resource capacity.
- */
-public class LocalHostResourceFallback {
+import com.sun.management.OperatingSystemMXBean;
+import java.lang.management.ManagementFactory;
+
+/** This class computes the local host's resource capacity. */
+final class LocalHostResource {
 
-  /* If /proc/* information is not available, guess based on what the JVM thinks.  Anecdotally,
-   * the JVM picks 0.22 the total available memory as maxMemory (tested on a standard Mac), so
-   * multiply by 3, and divide by 2^20 because we want megabytes.
-   */
   private static final ResourceSet DEFAULT_RESOURCES =
       ResourceSet.create(
-          3.0 * (Runtime.getRuntime().maxMemory() >> 20),
+          // Only com.sun.management.OperatingSystemMXBean provides the total physical memory size.
+          // This bean is container-aware as of JDK 14.
+          // https://github.com/openjdk/jdk/commit/7b82266a159ce363708e347aba7e1b0d38206b48
+          ((OperatingSystemMXBean) ManagementFactory.getOperatingSystemMXBean())
+                  .getTotalPhysicalMemorySize()
+              / (1024.0 * 1024.0),
+          // As of JDK 11, availableProcessors is aware of cgroups as commonly used by containers.
+          // https://hg.openjdk.java.net/jdk/hs/rev/7f22774a5f42#l6.178
           Runtime.getRuntime().availableProcessors(),
           Integer.MAX_VALUE);
 
-  public static ResourceSet getLocalHostResources() {
+  public static ResourceSet get() {
     return DEFAULT_RESOURCES;
   }
+
+  private LocalHostResource() {}
 }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerDarwin.java b/src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerDarwin.java
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinux.java b/src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinux.java
diff --git a/src/main/java/com/google/devtools/build/lib/unix/NativePosixSystem.java b/src/main/java/com/google/devtools/build/lib/unix/NativePosixSystem.java
diff --git a/src/main/native/darwin/file_jni.cc b/src/main/native/darwin/file_jni.cc
@@ -113,8 +113,4 @@ ssize_t portable_lgetxattr(const char *path, const char *name, void *value,
   return result;
 }
 
-int portable_sysctlbyname(const char *name_chars, void *mibp, size_t *sizep) {
-  return sysctlbyname(name_chars, mibp, sizep, nullptr, 0);
-}
-
 }  // namespace blaze_jni
diff --git a/src/main/native/unix_jni.cc b/src/main/native/unix_jni.cc
@@ -1282,19 +1282,6 @@ Java_com_google_devtools_build_lib_unix_NativePosixFiles_write(
   free(buf);
 }
 
-extern "C" JNIEXPORT jlong JNICALL
-Java_com_google_devtools_build_lib_unix_NativePosixSystem_sysctlbynameGetLong(
-    JNIEnv *env, jclass clazz, jstring name) {
-  const char *name_chars = GetStringLatin1Chars(env, name);
-  int64_t r;
-  size_t len = sizeof(r);
-  if (portable_sysctlbyname(name_chars, &r, &len) == -1) {
-    PostException(env, errno, std::string("sysctlbyname(") + name_chars + ")");
-  }
-  ReleaseStringLatin1Chars(name_chars);
-  return (jlong)r;
-}
-
 /*
  * Class:     com_google_devtools_build_lib_platform_SleepPreventionModule_SleepPrevention
  * Method:    pushDisableSleep
diff --git a/src/main/native/unix_jni.h b/src/main/native/unix_jni.h
@@ -83,9 +83,6 @@ ssize_t portable_getxattr(const char *path, const char *name, void *value,
 ssize_t portable_lgetxattr(const char *path, const char *name, void *value,
                            size_t size, bool *attr_not_found);
 
-// Run sysctlbyname(3), only available on darwin
-int portable_sysctlbyname(const char *name_chars, void *mibp, size_t *sizep);
-
 // Used to surround an region that we want sleep disabled for.
 // push_disable_sleep to start the area.
 // pop_disable_sleep to end the area.
diff --git a/src/main/native/unix_jni_bsd.cc b/src/main/native/unix_jni_bsd.cc
@@ -105,15 +105,6 @@ ssize_t portable_lgetxattr(const char *path, const char *name, void *value,
 #endif
 }
 
-int portable_sysctlbyname(const char *name_chars, void *mibp, size_t *sizep) {
-#if defined(HAVE_SYSCTLBYNAME)
-  return sysctlbyname(name_chars, mibp, sizep, nullptr, 0);
-#else
-  errno = ENOSYS;
-  return -1;
-#endif
-}
-
 int portable_push_disable_sleep() {
   // Currently not supported.
   // https://wiki.freebsd.org/SuspendResume
diff --git a/src/main/native/unix_jni_linux.cc b/src/main/native/unix_jni_linux.cc
@@ -85,11 +85,6 @@ ssize_t portable_lgetxattr(const char *path, const char *name, void *value,
   return result;
 }
 
-int portable_sysctlbyname(const char *name_chars, void *mibp, size_t *sizep) {
-  errno = ENOSYS;
-  return -1;
-}
-
 int portable_push_disable_sleep() {
   // Currently not supported.
   return -1;
diff --git a/src/test/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinuxTest.java b/src/test/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinuxTest.java

Original file line number	Diff line number	Diff line change
`@@ -113,8 +113,4 @@ ssize_t portable_lgetxattr(const char path, const char name, void *value,`
`113`	`113`	`return result;`
`114`	`114`	`}`
`115`	`115`
`116`		`-int portable_sysctlbyname(const char name_chars, void mibp, size_t *sizep) {`
`117`		`- return sysctlbyname(name_chars, mibp, sizep, nullptr, 0);`
`118`		`-}`
`119`		`-`
`120`	`116`	`} // namespace blaze_jni`