Only compute hostname once per server lifetime
NetUtil.getShortHostName can take seconds on mac and on windows (like, 20!),
since it performs reverse dns lookup. We already cached hostname for the
BazelWorkspaceStatusModule, let's cache it for entire bazel server. Also make
sure that users of the method understand it's cached.
Fixes #3586.
RELNOTES: None.
PiperOrigin-RevId: 168691615
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
index bb77abc..16f882f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
@@ -282,7 +282,6 @@
}
private class BazelStatusActionFactory implements WorkspaceStatusAction.Factory {
- private String hostname;
@Override
public Map<String, String> createDummyWorkspaceStatus() {
@@ -312,11 +311,7 @@
* changes during bazel server lifetime, bazel will not see the change.
*/
private String getHostname() {
- if (hostname == null) {
- hostname = NetUtil.findShortHostName();
- }
-
- return hostname;
+ return NetUtil.getCachedShortHostName();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
index 57d7576..9aed79b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -90,7 +90,7 @@
this.execRoot = execRoot;
this.processWrapper = getProcessWrapper(execRoot, localOs).getPathString();
this.localExecutionOptions = Preconditions.checkNotNull(localExecutionOptions);
- this.hostName = NetUtil.findShortHostName();
+ this.hostName = NetUtil.getCachedShortHostName();
this.resourceManager = resourceManager;
this.useProcessWrapper = useProcessWrapper;
this.productName = productName;
diff --git a/src/main/java/com/google/devtools/build/lib/util/NetUtil.java b/src/main/java/com/google/devtools/build/lib/util/NetUtil.java
index f22146a..dc2ce88 100644
--- a/src/main/java/com/google/devtools/build/lib/util/NetUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/util/NetUtil.java
@@ -21,14 +21,31 @@
*/
public final class NetUtil {
+ private static String hostname;
+
private NetUtil() {
}
/**
- * Returns the short hostname or <code>unknown</code> if the host name could not be determined.
- * Performs reverse DNS lookup and can take seconds to complete!
+ * Returns the *cached* short hostname (computed at most once per the lifetime of a server). Can
+ * take seconds to complete when the cache is cold.
*/
- public static String findShortHostName() {
+ public static String getCachedShortHostName() {
+ if (hostname == null) {
+ synchronized (NetUtil.class) {
+ if (hostname == null) {
+ hostname = computeShortHostName();
+ }
+ }
+ }
+ return computeShortHostName();
+ }
+
+ /**
+ * Returns the short hostname or <code>unknown</code> if the host name could not be determined.
+ * Performs reverse DNS lookup and can take seconds to complete.
+ */
+ private static String computeShortHostName() {
try {
return InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
index fe84224..4baed38 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
@@ -258,7 +258,7 @@
assertThat(result.status()).isEqualTo(SpawnResult.Status.SUCCESS);
assertThat(result.exitCode()).isEqualTo(0);
assertThat(result.setupSuccess()).isTrue();
- assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName());
+ assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName());
assertThat(captor.getValue().getArgv())
.containsExactlyElementsIn(
@@ -304,7 +304,7 @@
assertThat(result.status()).isEqualTo(SpawnResult.Status.SUCCESS);
assertThat(result.exitCode()).isEqualTo(0);
assertThat(result.setupSuccess()).isTrue();
- assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName());
+ assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName());
assertThat(captor.getValue().getArgv())
.containsExactlyElementsIn(ImmutableList.of("/bin/echo", "Hi!"));
@@ -339,7 +339,7 @@
assertThat(result.status()).isEqualTo(SpawnResult.Status.SUCCESS);
assertThat(result.exitCode()).isEqualTo(3);
assertThat(result.setupSuccess()).isTrue();
- assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName());
+ assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName());
assertThat(captor.getValue().getArgv())
.containsExactlyElementsIn(
@@ -377,7 +377,7 @@
assertThat(result.exitCode()).isEqualTo(-1);
assertThat(result.setupSuccess()).isFalse();
assertThat(result.getWallTimeMillis()).isEqualTo(0);
- assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName());
+ assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName());
assertThat(FileSystemUtils.readContent(fs.getPath("/out/stderr"), StandardCharsets.UTF_8))
.isEqualTo("Action failed to execute: java.io.IOException: I'm sorry, Dave\n");
@@ -399,7 +399,7 @@
assertThat(reply.exitCode()).isEqualTo(-1);
assertThat(reply.setupSuccess()).isFalse();
assertThat(reply.getWallTimeMillis()).isEqualTo(0);
- assertThat(reply.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName());
+ assertThat(reply.getExecutorHostName()).isEqualTo(NetUtil.getCachedShortHostName());
// TODO(ulfjack): Maybe we should only lock after checking?
assertThat(policy.lockOutputFilesCalled).isTrue();
