From f05c0a48dc298ad3e90f3acb399cc53c2944a801 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 8 Aug 2018 05:29:34 -0700 Subject: Add a option to disable idle gc. If a Bazel server is idle for 10 seconds, it unconditionally triggers a full-scale Java GC via System.gc(). This behavior doesn't have clear benefits and causes Bazel to steal resources from whatever the user does after invoking Bazel. This CL adds a startup option, --idle_server_tasks, to toggle the idle GC behavior. Also, add some logging for when idle GC is enabled, so it's easier to evaluate its effects. Example of logging: ``` 180718 17:43:04.609:I 247 [com.google.devtools.build.lib.server.IdleServerTasks.lambda$idle$0] [Idle GC] used: 157MB -> 15MB, committed: 421MB -> 422MB ``` Fixes https://github.com/bazelbuild/bazel/issues/5589. Closes #5628. PiperOrigin-RevId: 207869996 --- src/main/cpp/blaze.cc | 5 +++ src/main/cpp/startup_options.cc | 8 ++++ src/main/cpp/startup_options.h | 2 + .../devtools/build/lib/runtime/BlazeRuntime.java | 14 ++++--- .../lib/runtime/BlazeServerStartupOptions.java | 11 ++++++ .../devtools/build/lib/server/GrpcServerImpl.java | 43 ++++++++++++++++------ .../devtools/build/lib/server/IdleServerTasks.java | 19 ++++++++-- .../devtools/build/lib/server/RPCServer.java | 11 +++++- 8 files changed, 90 insertions(+), 23 deletions(-) diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 9e7d979855..801a9314c0 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -541,6 +541,11 @@ static vector GetArgumentArray( // being "null" to set the programmatic default in the server. result.push_back("--digest_function=" + globals->options->digest_function); } + if (globals->options->idle_server_tasks) { + result.push_back("--idle_server_tasks"); + } else { + result.push_back("--noidle_server_tasks"); + } if (globals->options->oom_more_eagerly) { result.push_back("--experimental_oom_more_eagerly"); } else { diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index a2d26b74d3..c54728f92e 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -93,6 +93,7 @@ StartupOptions::StartupOptions(const string &product_name, "com.google.devtools.build.lib.util.SingleLineFormatter"), expand_configs_in_place(true), digest_function(), + idle_server_tasks(true), original_startup_options_(std::vector()) { bool testing = !blaze::GetEnv("TEST_TMPDIR").empty(); if (testing) { @@ -133,6 +134,7 @@ StartupOptions::StartupOptions(const string &product_name, RegisterNullaryStartupFlag("experimental_oom_more_eagerly"); RegisterNullaryStartupFlag("fatal_event_bus_exceptions"); RegisterNullaryStartupFlag("host_jvm_debug"); + RegisterNullaryStartupFlag("idle_server_tasks"); RegisterNullaryStartupFlag("ignore_all_rc_files"); RegisterNullaryStartupFlag("watchfs"); RegisterNullaryStartupFlag("write_command_log"); @@ -334,6 +336,12 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( } else if (GetNullaryOption(arg, "--noexpand_configs_in_place")) { expand_configs_in_place = false; option_sources["expand_configs_in_place"] = rcfile; + } else if (GetNullaryOption(arg, "--idle_server_tasks")) { + idle_server_tasks = true; + option_sources["idle_server_tasks"] = rcfile; + } else if (GetNullaryOption(arg, "--noidle_server_tasks")) { + idle_server_tasks = false; + option_sources["idle_server_tasks"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--connect_timeout_secs")) != NULL) { if (!blaze_util::safe_strto32(value, &connect_timeout_secs) || diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index cca38369c3..9f4cd0c622 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -301,6 +301,8 @@ class StartupOptions { // The hash function to use when computing file digests. std::string digest_function; + bool idle_server_tasks; + // The startup options as received from the user and rc files, tagged with // their origin. This is populated by ProcessArgs. std::vector original_startup_options_; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index a8b1d0ae75..cae2e779b1 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -925,11 +925,15 @@ public final class BlazeRuntime { Class factoryClass = Class.forName( "com.google.devtools.build.lib.server.GrpcServerImpl$Factory"); RPCServer.Factory factory = (RPCServer.Factory) factoryClass.getConstructor().newInstance(); - rpcServer[0] = factory.create(dispatcher, runtime.getClock(), - startupOptions.commandPort, - runtime.getWorkspace().getWorkspace(), - runtime.getServerDirectory(), - startupOptions.maxIdleSeconds); + rpcServer[0] = + factory.create( + dispatcher, + runtime.getClock(), + startupOptions.commandPort, + runtime.getWorkspace().getWorkspace(), + runtime.getServerDirectory(), + startupOptions.maxIdleSeconds, + startupOptions.idleServerTasks); } catch (ReflectiveOperationException | IllegalArgumentException e) { throw new AbruptExitException("gRPC server not compiled in", ExitCode.BLAZE_INTERNAL_ERROR); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java index 6d434f8d56..d5588f525e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java @@ -430,4 +430,15 @@ public class BlazeServerStartupOptions extends OptionsBase { "Changed the expansion of --config flags to be done in-place, as opposed to in a fixed " + "point expansion between normal rc options and command-line specified options.") public boolean expandConfigsInPlace; + + @Option( + name = "idle_server_tasks", + defaultValue = "true", // NOTE: only for documentation, value is set and used by the client. + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = { + OptionEffectTag.LOSES_INCREMENTAL_STATE, + OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS, + }, + help = "Run System.gc() when the server is idle") + public boolean idleServerTasks; } diff --git a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java index 2d02631e40..b23d22017c 100644 --- a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java +++ b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java @@ -152,10 +152,17 @@ public class GrpcServerImpl implements RPCServer { */ public static class Factory implements RPCServer.Factory { @Override - public RPCServer create(BlazeCommandDispatcher dispatcher, Clock clock, int port, - Path workspace, Path serverDirectory, int maxIdleSeconds) throws IOException { + public RPCServer create( + BlazeCommandDispatcher dispatcher, + Clock clock, + int port, + Path workspace, + Path serverDirectory, + int maxIdleSeconds, + boolean idleServerTasks) + throws IOException { return new GrpcServerImpl( - dispatcher, clock, port, workspace, serverDirectory, maxIdleSeconds); + dispatcher, clock, port, workspace, serverDirectory, maxIdleSeconds, idleServerTasks); } } @@ -512,13 +519,21 @@ public class GrpcServerImpl implements RPCServer { private final String pidInFile; private final List filesToDeleteAtExit = new ArrayList<>(); private final int port; + private final boolean doIdleServerTasks; private Server server; private IdleServerTasks idleServerTasks; boolean serving; - public GrpcServerImpl(BlazeCommandDispatcher dispatcher, Clock clock, int port, - Path workspace, Path serverDirectory, int maxIdleSeconds) throws IOException { + public GrpcServerImpl( + BlazeCommandDispatcher dispatcher, + Clock clock, + int port, + Path workspace, + Path serverDirectory, + int maxIdleSeconds, + boolean doIdleServerTasks) + throws IOException { Runtime.getRuntime().addShutdownHook(new Thread() { @Override public void run() { @@ -555,20 +570,24 @@ public class GrpcServerImpl implements RPCServer { pidFileWatcherThread = new PidFileWatcherThread(); pidFileWatcherThread.start(); - idleServerTasks = new IdleServerTasks(); - idleServerTasks.idle(); + this.doIdleServerTasks = doIdleServerTasks; + idle(); } private void idle() { Preconditions.checkState(idleServerTasks == null); - idleServerTasks = new IdleServerTasks(); - idleServerTasks.idle(); + if (doIdleServerTasks) { + idleServerTasks = new IdleServerTasks(); + idleServerTasks.idle(); + } } private void busy() { - Preconditions.checkState(idleServerTasks != null); - idleServerTasks.busy(); - idleServerTasks = null; + if (doIdleServerTasks) { + Preconditions.checkState(idleServerTasks != null); + idleServerTasks.busy(); + idleServerTasks = null; + } } private static String generateCookie(SecureRandom random, int byteCount) { diff --git a/src/main/java/com/google/devtools/build/lib/server/IdleServerTasks.java b/src/main/java/com/google/devtools/build/lib/server/IdleServerTasks.java index 01e596a00c..d8cb41a2b3 100644 --- a/src/main/java/com/google/devtools/build/lib/server/IdleServerTasks.java +++ b/src/main/java/com/google/devtools/build/lib/server/IdleServerTasks.java @@ -16,6 +16,10 @@ package com.google.devtools.build.lib.server; import com.google.common.base.Preconditions; import com.google.devtools.build.lib.profiler.AutoProfiler; +import com.google.devtools.build.lib.util.StringUtilities; +import java.lang.management.ManagementFactory; +import java.lang.management.MemoryMXBean; +import java.lang.management.MemoryUsage; import java.util.concurrent.Future; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -29,9 +33,7 @@ class IdleServerTasks { private final ScheduledThreadPoolExecutor executor; private static final Logger logger = Logger.getLogger(IdleServerTasks.class.getName()); - /** - * Must be called from the main thread. - */ + /** Must be called from the main thread. */ public IdleServerTasks() { this.executor = new ScheduledThreadPoolExecutor(1); } @@ -43,14 +45,23 @@ class IdleServerTasks { public void idle() { Preconditions.checkState(!executor.isShutdown()); - // Do a GC cycle while the server is idle. @SuppressWarnings("unused") Future possiblyIgnoredError = executor.schedule( () -> { + MemoryMXBean memBean = ManagementFactory.getMemoryMXBean(); + MemoryUsage before = memBean.getHeapMemoryUsage(); try (AutoProfiler p = AutoProfiler.logged("Idle GC", logger)) { System.gc(); } + MemoryUsage after = memBean.getHeapMemoryUsage(); + logger.info( + String.format( + "[Idle GC] used: %s -> %s, committed: %s -> %s", + StringUtilities.prettyPrintBytes(before.getUsed()), + StringUtilities.prettyPrintBytes(after.getUsed()), + StringUtilities.prettyPrintBytes(before.getCommitted()), + StringUtilities.prettyPrintBytes(after.getCommitted()))); }, 10, TimeUnit.SECONDS); diff --git a/src/main/java/com/google/devtools/build/lib/server/RPCServer.java b/src/main/java/com/google/devtools/build/lib/server/RPCServer.java index 014b17b5ae..9ea22844ee 100644 --- a/src/main/java/com/google/devtools/build/lib/server/RPCServer.java +++ b/src/main/java/com/google/devtools/build/lib/server/RPCServer.java @@ -28,8 +28,15 @@ public interface RPCServer { * Present so that we don't need to invoke a constructor with multiple arguments by reflection. */ interface Factory { - RPCServer create(BlazeCommandDispatcher dispatcher, Clock clock, int port, - Path workspace, Path serverDirectory, int maxIdleSeconds) throws IOException; + RPCServer create( + BlazeCommandDispatcher dispatcher, + Clock clock, + int port, + Path workspace, + Path serverDirectory, + int maxIdleSeconds, + boolean idleServerTasks) + throws IOException; } /** -- cgit v1.2.3