Intern common TimeValue constants (#107985)

The values `30s` and `1m` are used as defaults in various places in ES,
there's no need to create a new `TimeValue` instance each time they
appear. Moreover we already have constants for `0` and `-1`, but we
don't use these constants when reading the values off the wire.

This commit adds constants for `30s` and `1m` and adjusts the
deserialization code to avoid unnecessary allocation for common
`TimeValue` instances.

Relates #107984

libs/core/src/main/java/org/elasticsearch/core/TimeValue.java

@@ -17,9 +17,11 @@ public class TimeValue implements Comparable<TimeValue> {
     /** How many nano-seconds in one milli-second */
     public static final long NSEC_PER_MSEC = TimeUnit.NANOSECONDS.convert(1, TimeUnit.MILLISECONDS);
 
-    public static final TimeValue MINUS_ONE = timeValueMillis(-1);
+    public static final TimeValue MINUS_ONE = new TimeValue(-1, TimeUnit.MILLISECONDS);
-    public static final TimeValue ZERO = timeValueMillis(0);
+    public static final TimeValue ZERO = new TimeValue(0, TimeUnit.MILLISECONDS);
-    public static final TimeValue MAX_VALUE = TimeValue.timeValueNanos(Long.MAX_VALUE);
+    public static final TimeValue MAX_VALUE = new TimeValue(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+    public static final TimeValue THIRTY_SECONDS = new TimeValue(30, TimeUnit.SECONDS);
+    public static final TimeValue ONE_MINUTE = new TimeValue(1, TimeUnit.MINUTES);
 
     private static final long C0 = 1L;
     private static final long C1 = C0 * 1000L;
@@ -49,14 +51,28 @@ public class TimeValue implements Comparable<TimeValue> {
     }
 
     public static TimeValue timeValueMillis(long millis) {
+        if (millis == 0) {
+            return ZERO;
+        }
+        if (millis == -1) {
+            return MINUS_ONE;
+        }
         return new TimeValue(millis, TimeUnit.MILLISECONDS);
     }
 
     public static TimeValue timeValueSeconds(long seconds) {
+        if (seconds == 30) {
+            // common value, no need to allocate each time
+            return THIRTY_SECONDS;
+        }
         return new TimeValue(seconds, TimeUnit.SECONDS);
     }
 
     public static TimeValue timeValueMinutes(long minutes) {
+        if (minutes == 1) {
+            // common value, no need to allocate each time
+            return ONE_MINUTE;
+        }
         return new TimeValue(minutes, TimeUnit.MINUTES);
     }
 
@@ -355,18 +371,18 @@ public class TimeValue implements Comparable<TimeValue> {
         }
         final String normalized = sValue.toLowerCase(Locale.ROOT).trim();
         if (normalized.endsWith("nanos")) {
-            return new TimeValue(parse(sValue, normalized, "nanos", settingName), TimeUnit.NANOSECONDS);
+            return TimeValue.timeValueNanos(parse(sValue, normalized, "nanos", settingName));
         } else if (normalized.endsWith("micros")) {
             return new TimeValue(parse(sValue, normalized, "micros", settingName), TimeUnit.MICROSECONDS);
         } else if (normalized.endsWith("ms")) {
-            return new TimeValue(parse(sValue, normalized, "ms", settingName), TimeUnit.MILLISECONDS);
+            return TimeValue.timeValueMillis(parse(sValue, normalized, "ms", settingName));
         } else if (normalized.endsWith("s")) {
-            return new TimeValue(parse(sValue, normalized, "s", settingName), TimeUnit.SECONDS);
+            return TimeValue.timeValueSeconds(parse(sValue, normalized, "s", settingName));
         } else if (sValue.endsWith("m")) {
             // parsing minutes should be case-sensitive as 'M' means "months", not "minutes"; this is the only special case.
-            return new TimeValue(parse(sValue, normalized, "m", settingName), TimeUnit.MINUTES);
+            return TimeValue.timeValueMinutes(parse(sValue, normalized, "m", settingName));
         } else if (normalized.endsWith("h")) {
-            return new TimeValue(parse(sValue, normalized, "h", settingName), TimeUnit.HOURS);
+            return TimeValue.timeValueHours(parse(sValue, normalized, "h", settingName));
         } else if (normalized.endsWith("d")) {
             return new TimeValue(parse(sValue, normalized, "d", settingName), TimeUnit.DAYS);
         } else if (normalized.matches("-0*1")) {

libs/core/src/test/java/org/elasticsearch/common/unit/TimeValueTests.java

@@ -242,4 +242,16 @@ public class TimeValueTests extends ESTestCase {
             TimeUnit.DAYS
         );
     }
+
+    public void testInternedValues() {
+        assertSame(TimeValue.timeValueMillis(-1), TimeValue.MINUS_ONE);
+        assertSame(TimeValue.timeValueMillis(0), TimeValue.ZERO);
+        assertSame(TimeValue.timeValueSeconds(30), TimeValue.THIRTY_SECONDS);
+        assertSame(TimeValue.timeValueMinutes(1), TimeValue.ONE_MINUTE);
+
+        assertSame(TimeValue.parseTimeValue("-1", getTestName()), TimeValue.MINUS_ONE);
+        assertSame(TimeValue.parseTimeValue("0", getTestName()), TimeValue.ZERO);
+        assertSame(TimeValue.parseTimeValue("30s", getTestName()), TimeValue.THIRTY_SECONDS);
+        assertSame(TimeValue.parseTimeValue("1m", getTestName()), TimeValue.ONE_MINUTE);
+    }
 }

server/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthRequest.java

@@ -24,13 +24,12 @@ import org.elasticsearch.tasks.TaskId;
 
 import java.io.IOException;
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 
 public class ClusterHealthRequest extends MasterNodeReadRequest<ClusterHealthRequest> implements IndicesRequest.Replaceable {
 
     private String[] indices;
     private IndicesOptions indicesOptions = IndicesOptions.lenientExpandHidden();
-    private TimeValue timeout = new TimeValue(30, TimeUnit.SECONDS);
+    private TimeValue timeout = TimeValue.timeValueSeconds(30);
     private ClusterHealthStatus waitForStatus;
     private boolean waitForNoRelocatingShards = false;
     private boolean waitForNoInitializingShards = false;

server/src/main/java/org/elasticsearch/action/support/replication/ReplicationRequest.java

@@ -25,7 +25,6 @@ import org.elasticsearch.tasks.TaskId;
 
 import java.io.IOException;
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.action.ValidateActions.addValidationError;
 
@@ -35,7 +34,7 @@ import static org.elasticsearch.action.ValidateActions.addValidationError;
  */
 public abstract class ReplicationRequest<Request extends ReplicationRequest<Request>> extends ActionRequest implements IndicesRequest {
 
-    public static final TimeValue DEFAULT_TIMEOUT = new TimeValue(1, TimeUnit.MINUTES);
+    public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueMinutes(1);
 
     /**
      * Target shard the request should execute on. In case of index and delete requests,

server/src/main/java/org/elasticsearch/action/support/single/instance/InstanceShardOperationRequest.java

@@ -20,7 +20,6 @@ import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.shard.ShardId;
 
 import java.io.IOException;
-import java.util.concurrent.TimeUnit;
 
 // TODO: This request and its associated transport action can be folded into UpdateRequest which is its only concrete production code
 //       implementation
@@ -28,7 +27,7 @@ public abstract class InstanceShardOperationRequest<Request extends InstanceShar
     implements
         IndicesRequest {
 
-    public static final TimeValue DEFAULT_TIMEOUT = new TimeValue(1, TimeUnit.MINUTES);
+    public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueMinutes(1);
 
     protected TimeValue timeout = DEFAULT_TIMEOUT;
 

server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java

@@ -1405,9 +1405,15 @@ public abstract class StreamInput extends InputStream {
      * Read a {@link TimeValue} from the stream
      */
     public TimeValue readTimeValue() throws IOException {
-        long duration = readZLong();
+        final long duration = readZLong();
-        TimeUnit timeUnit = TIME_UNITS[readByte()];
+        final TimeUnit timeUnit = TIME_UNITS[readByte()];
-        return new TimeValue(duration, timeUnit);
+        return switch (timeUnit) {
+            // avoid unnecessary allocation for some common cases:
+            case MILLISECONDS -> TimeValue.timeValueMillis(duration);
+            case SECONDS -> TimeValue.timeValueSeconds(duration);
+            case MINUTES -> TimeValue.timeValueMinutes(duration);
+            default -> new TimeValue(duration, timeUnit);
+        };
     }
 
     /**

server/src/test/java/org/elasticsearch/common/io/stream/BytesStreamsTests.java

@@ -902,6 +902,22 @@ public class BytesStreamsTests extends ESTestCase {
         assertEqualityAfterSerialize(timeValue, 1 + out.bytes().length());
     }
 
+    public void testTimeValueInterning() throws IOException {
+        try (var bytesOut = new BytesStreamOutput()) {
+            bytesOut.writeTimeValue(randomBoolean() ? TimeValue.MINUS_ONE : new TimeValue(-1, TimeUnit.MILLISECONDS));
+            bytesOut.writeTimeValue(randomBoolean() ? TimeValue.ZERO : new TimeValue(0, TimeUnit.MILLISECONDS));
+            bytesOut.writeTimeValue(randomBoolean() ? TimeValue.THIRTY_SECONDS : new TimeValue(30, TimeUnit.SECONDS));
+            bytesOut.writeTimeValue(randomBoolean() ? TimeValue.ONE_MINUTE : new TimeValue(1, TimeUnit.MINUTES));
+
+            try (var in = bytesOut.bytes().streamInput()) {
+                assertSame(TimeValue.MINUS_ONE, in.readTimeValue());
+                assertSame(TimeValue.ZERO, in.readTimeValue());
+                assertSame(TimeValue.THIRTY_SECONDS, in.readTimeValue());
+                assertSame(TimeValue.ONE_MINUTE, in.readTimeValue());
+            }
+        }
+    }
+
     private static class TestStreamOutput extends BytesStream {
 
         private final BytesStreamOutput output = new BytesStreamOutput();