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

Author: hlopko

src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java

@@ -282,7 +282,6 @@ public Artifact getStableStatus() {
   }
 
   private class BazelStatusActionFactory implements WorkspaceStatusAction.Factory {
-    private String hostname;
 
     @Override
     public Map<String, String> createDummyWorkspaceStatus() {
@@ -312,11 +311,7 @@ public WorkspaceStatusAction createWorkspaceStatusAction(
      * 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();
     }
   }
 

src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java

@@ -90,7 +90,7 @@ public LocalSpawnRunner(
     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;

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 *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 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!
+   * Performs reverse DNS lookup and can take seconds to complete.
    */
-  public static String findShortHostName() {
+  private static String computeShortHostName() {
     try {
       return InetAddress.getLocalHost().getHostName();
     } catch (UnknownHostException e) {

src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java

@@ -258,7 +258,7 @@ public void vanillaZeroExit() throws Exception {
     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 @@ public void noProcessWrapper() throws Exception {
     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 @@ public void nonZeroExit() throws Exception {
     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 @@ public void processStartupThrows() throws Exception {
     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 @@ public void disallowLocalExecution() throws Exception {
     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();