From eac4b6b971c93b0ea119a8b5155fcdbd752ee227 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 7 Dec 2023 02:41:04 -0800 Subject: [PATCH] Make Bazel's RAM estimate container aware 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 https://github.com/bazelbuild/bazel/pull/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 --- .../google/devtools/build/lib/actions/BUILD | 7 +- .../build/lib/actions/LocalHostCapacity.java | 18 +- ...ceFallback.java => LocalHostResource.java} | 26 +-- .../LocalHostResourceManagerDarwin.java | 53 ------ .../LocalHostResourceManagerLinux.java | 58 ------- .../build/lib/unix/NativePosixSystem.java | 40 ----- src/main/native/darwin/file_jni.cc | 4 - src/main/native/unix_jni.cc | 13 -- src/main/native/unix_jni.h | 3 - src/main/native/unix_jni_bsd.cc | 9 - src/main/native/unix_jni_linux.cc | 5 - .../LocalHostResourceManagerLinuxTest.java | 154 ------------------ 12 files changed, 18 insertions(+), 372 deletions(-) rename src/main/java/com/google/devtools/build/lib/actions/{LocalHostResourceFallback.java => LocalHostResource.java} (50%) delete mode 100644 src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerDarwin.java delete mode 100644 src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinux.java delete mode 100644 src/main/java/com/google/devtools/build/lib/unix/NativePosixSystem.java delete mode 100644 src/test/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinuxTest.java diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index ecfe45aefff70c..c8c6e19ef72c22 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -422,18 +422,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 index 026199db7b3b1d..a0946ec35cc4fe 100644 --- 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/LocalHostResourceFallback.java b/src/main/java/com/google/devtools/build/lib/actions/LocalHostResource.java similarity index 50% rename from src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceFallback.java rename to src/main/java/com/google/devtools/build/lib/actions/LocalHostResource.java index 77cd5a83f7f444..07edb8fcc365a5 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceFallback.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 deleted file mode 100644 index 95116cd88c7f12..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerDarwin.java +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.actions; - -import com.google.devtools.build.lib.jni.JniLoader; -import com.google.devtools.build.lib.unix.NativePosixSystem; -import java.io.IOException; -import javax.annotation.Nullable; - -/** - * This class estimates the local host's resource capacity for Darwin. - */ -public class LocalHostResourceManagerDarwin { - - private static int getLogicalCpuCount() throws IOException { - return (int) NativePosixSystem.sysctlbynameGetLong("hw.logicalcpu"); - } - - private static double getMemoryInMb() throws IOException { - return NativePosixSystem.sysctlbynameGetLong("hw.memsize") / 1E6; - } - - @Nullable - public static ResourceSet getLocalHostResources() { - if (!JniLoader.isJniAvailable()) { - return null; - } - - try { - int logicalCpuCount = getLogicalCpuCount(); - double ramMb = getMemoryInMb(); - - return ResourceSet.create( - ramMb, - logicalCpuCount, - Integer.MAX_VALUE); - } catch (IOException e) { - return null; - } - } -} 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 deleted file mode 100644 index fd3c7dfafd203c..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinux.java +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.actions; - -import com.google.devtools.build.lib.unix.ProcMeminfoParser; -import java.io.IOException; -import javax.annotation.Nullable; - -/** - * This class estimates the local host's resource capacity for Linux. - */ -public class LocalHostResourceManagerLinux { - - private static final String MEM_INFO_FILE = "/proc/meminfo"; - - private static int getLogicalCpuCount() throws IOException { - // 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 - return Runtime.getRuntime().availableProcessors(); - } - - private static double getMemoryInMb() throws IOException { - return getMemoryInMbHelper(MEM_INFO_FILE); - } - - @Nullable - public static ResourceSet getLocalHostResources() { - try { - int logicalCpuCount = getLogicalCpuCount(); - double ramMb = getMemoryInMb(); - - return ResourceSet.create( - ramMb, - logicalCpuCount, - Integer.MAX_VALUE); - } catch (IOException e) { - return null; - } - } - - public static double getMemoryInMbHelper(String memInfoFileName) throws IOException { - ProcMeminfoParser memInfo = new ProcMeminfoParser(memInfoFileName); - double ramMb = ProcMeminfoParser.kbToMb(memInfo.getTotalKb()); - return ramMb; - } -} 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 deleted file mode 100644 index 11f19b6c268ff5..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/unix/NativePosixSystem.java +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.unix; - -import com.google.devtools.build.lib.jni.JniLoader; -import java.io.IOException; - -/** - * Utility methods for access to UNIX system calls not exposed by the Java - * SDK. Exception messages are selected to be consistent with those generated - * by the java.io package where appropriate--see package javadoc for details. - */ -public class NativePosixSystem { - - static { - JniLoader.loadJni(); - } - - private NativePosixSystem() {} - - /** - * Native wrapper around POSIX sysctlbyname(3) syscall. - * - * @param name the name for value to get from sysctl - * @throws IOException iff the sysctlbyname() syscall failed. - */ - public static native long sysctlbynameGetLong(String name) throws IOException; -} diff --git a/src/main/native/darwin/file_jni.cc b/src/main/native/darwin/file_jni.cc index b076cd960d6dd5..2f9c910b57067c 100644 --- 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 index 5082c409d11e16..893507d385114c 100644 --- 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 index 3e231fcf65ae58..c7a1a73af38eaa 100644 --- 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 index 6dd506854dc7c6..2acda1cbc77355 100644 --- 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 index b32c1d33a19b83..1aa0f252f2d6f3 100644 --- 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 deleted file mode 100644 index b5d95a3355871d..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/actions/LocalHostResourceManagerLinuxTest.java +++ /dev/null @@ -1,154 +0,0 @@ -// Copyright 2015 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.actions; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.devtools.build.lib.util.StringUtilities; -import com.google.devtools.build.lib.vfs.util.FsApparatus; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** - * Test for LocalHostResourceManagerLinux. - */ -@RunWith(JUnit4.class) -public class LocalHostResourceManagerLinuxTest { - - private final FsApparatus scratch = FsApparatus.newNative(); - - @Test - public void testNonHyperthreadedMachine() throws Exception { - String meminfoContent = - StringUtilities.joinLines( - "MemTotal: 3091732 kB", - "MemFree: 2167344 kB", - "Buffers: 60644 kB", - "Cached: 509940 kB", - "SwapCached: 0 kB", - "Active: 636892 kB", - "Inactive: 212760 kB", - "HighTotal: 0 kB", - "HighFree: 0 kB", - "LowTotal: 3091732 kB", - "LowFree: 2167344 kB", - "SwapTotal: 9124880 kB", - "SwapFree: 9124880 kB", - "Dirty: 0 kB", - "Writeback: 0 kB", - "AnonPages: 279028 kB", - "Mapped: 54404 kB", - "Slab: 42820 kB", - "PageTables: 5184 kB", - "NFS_Unstable: 0 kB", - "Bounce: 0 kB", - "CommitLimit: 10670744 kB", - "Committed_AS: 665840 kB", - "VmallocTotal: 34359738367 kB", - "VmallocUsed: 300484 kB", - "VmallocChunk: 34359437307 kB", - "HugePages_Total: 0", - "HugePages_Free: 0", - "HugePages_Rsvd: 0", - "Hugepagesize: 2048 kB"); - String meminfoFile = scratch.file("test_meminfo_nonht", meminfoContent).getPathString(); - // +/- 0.1MB - assertThat(LocalHostResourceManagerLinux.getMemoryInMbHelper(meminfoFile)) - .isWithin(0.1) - .of(3091732 >> 10); // kibis to meibis - } - - @Test - public void testHyperthreadedMachine() throws Exception { - String meminfoContent = - StringUtilities.joinLines( - "MemTotal: 3092004 kB", - "MemFree: 26124 kB", - "Buffers: 3836 kB", - "Cached: 52400 kB", - "SwapCached: 68204 kB", - "Active: 2281464 kB", - "Inactive: 260908 kB", - "HighTotal: 0 kB", - "HighFree: 0 kB", - "LowTotal: 3092004 kB", - "LowFree: 26124 kB", - "SwapTotal: 9124880 kB", - "SwapFree: 8264920 kB", - "Dirty: 616 kB", - "Writeback: 0 kB", - "AnonPages: 2466336 kB", - "Mapped: 37576 kB", - "Slab: 483004 kB", - "PageTables: 11912 kB", - "NFS_Unstable: 0 kB", - "Bounce: 0 kB", - "CommitLimit: 10670880 kB", - "Committed_AS: 3627984 kB", - "VmallocTotal: 34359738367 kB", - "VmallocUsed: 300460 kB", - "VmallocChunk: 34359437307 kB", - "HugePages_Total: 0", - "HugePages_Free: 0", - "HugePages_Rsvd: 0", - "Hugepagesize: 2048 kB"); - String meminfoFile = scratch.file("test_meminfo_ht", meminfoContent).getPathString(); - // +/- 0.1MB - assertThat(LocalHostResourceManagerLinux.getMemoryInMbHelper(meminfoFile)) - .isWithin(0.1) - .of(3092004 >> 10); // kibis to meibis - } - - @Test - public void testAMDMachine() throws Exception { - String meminfoContent = - StringUtilities.joinLines( - "MemTotal: 8223956 kB", - "MemFree: 3670396 kB", - "Buffers: 374068 kB", - "Cached: 3366980 kB", - "SwapCached: 0 kB", - "Active: 3275860 kB", - "Inactive: 737816 kB", - "HighTotal: 0 kB", - "HighFree: 0 kB", - "LowTotal: 8223956 kB", - "LowFree: 3670396 kB", - "SwapTotal: 6024332 kB", - "SwapFree: 6024332 kB", - "Dirty: 84 kB", - "Writeback: 0 kB", - "AnonPages: 272308 kB", - "Mapped: 62604 kB", - "Slab: 506140 kB", - "PageTables: 4608 kB", - "NFS_Unstable: 0 kB", - "Bounce: 0 kB", - "CommitLimit: 10136308 kB", - "Committed_AS: 600672 kB", - "VmallocTotal: 34359738367 kB", - "VmallocUsed: 299068 kB", - "VmallocChunk: 34359438843 kB", - "HugePages_Total: 0", - "HugePages_Free: 0", - "HugePages_Rsvd: 0", - "Hugepagesize: 2048 kB"); - String meminfoFile = scratch.file("test_meminfo_amd", meminfoContent).getPathString(); - // +/- 0.1MB - assertThat(LocalHostResourceManagerLinux.getMemoryInMbHelper(meminfoFile)) - .isWithin(0.1) - .of(8223956 >> 10); // kibis to meibis - } -}