Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intern common TimeValue constants #107985

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 24 additions & 8 deletions libs/core/src/main/java/org/elasticsearch/core/TimeValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 ZERO = timeValueMillis(0);
public static final TimeValue MAX_VALUE = TimeValue.timeValueNanos(Long.MAX_VALUE);
public static final TimeValue MINUS_ONE = new TimeValue(-1, TimeUnit.MILLISECONDS);
public static final TimeValue ZERO = new TimeValue(0, TimeUnit.MILLISECONDS);
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;
Expand Down Expand Up @@ -49,14 +51,28 @@ public static TimeValue timeValueNanos(long nanos) {
}

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);
}

Expand Down Expand Up @@ -355,18 +371,18 @@ public static TimeValue parseTimeValue(@Nullable String sValue, TimeValue defaul
}
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")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,4 +242,16 @@ private TimeUnit randomTimeUnitObject() {
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

import java.io.IOException;
import java.util.Map;
import java.util.concurrent.TimeUnit;

import static org.elasticsearch.action.ValidateActions.addValidationError;

Expand All @@ -35,7 +34,7 @@
*/
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@
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
public abstract class InstanceShardOperationRequest<Request extends InstanceShardOperationRequest<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);

protected TimeValue timeout = DEFAULT_TIMEOUT;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1405,9 +1405,15 @@ protected static void throwEOF(int bytesToRead, int bytesAvailable) throws EOFEx
* Read a {@link TimeValue} from the stream
*/
public TimeValue readTimeValue() throws IOException {
long duration = readZLong();
TimeUnit timeUnit = TIME_UNITS[readByte()];
return new TimeValue(duration, timeUnit);
final long duration = readZLong();
final TimeUnit timeUnit = TIME_UNITS[readByte()];
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);
};
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,22 @@ public void testTimeValueSerialize() throws Exception {
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();
Expand Down