Skip to content

Commit 4782ee5

Browse files
Lazy initialize HttpRouteStatsTracker in MethodHandlers (elastic#114107) (elastic#115620)
We use about 1M for the route stats trackers instances per ES instance. Making this lazy init should come at a trivial overhead and in fact makes the computation of the node stats cheaper by saving spurious sums on 0-valued long adders.
1 parent a73e972 commit 4782ee5

File tree

3 files changed

+46
-23
lines changed

3 files changed

+46
-23
lines changed

server/src/main/java/org/elasticsearch/http/HttpRouteStats.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public record HttpRouteStats(
4949
long[] responseTimeHistogram
5050
) implements Writeable, ToXContentObject {
5151

52+
public static final HttpRouteStats EMPTY = new HttpRouteStats(0, 0, new long[0], 0, 0, new long[0], new long[0]);
53+
5254
public HttpRouteStats(StreamInput in) throws IOException {
5355
this(in.readVLong(), in.readVLong(), in.readVLongArray(), in.readVLong(), in.readVLong(), in.readVLongArray(), in.readVLongArray());
5456
}

server/src/main/java/org/elasticsearch/rest/MethodHandlers.java

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import org.elasticsearch.http.HttpRouteStats;
1414
import org.elasticsearch.http.HttpRouteStatsTracker;
1515

16+
import java.lang.invoke.MethodHandles;
17+
import java.lang.invoke.VarHandle;
1618
import java.util.EnumMap;
1719
import java.util.Map;
1820
import java.util.Set;
@@ -25,7 +27,18 @@ final class MethodHandlers {
2527
private final String path;
2628
private final Map<RestRequest.Method, Map<RestApiVersion, RestHandler>> methodHandlers;
2729

28-
private final HttpRouteStatsTracker statsTracker = new HttpRouteStatsTracker();
30+
@SuppressWarnings("unused") // only accessed via #STATS_TRACKER_HANDLE, lazy initialized because instances consume non-trivial heap
31+
private volatile HttpRouteStatsTracker statsTracker;
32+
33+
private static final VarHandle STATS_TRACKER_HANDLE;
34+
35+
static {
36+
try {
37+
STATS_TRACKER_HANDLE = MethodHandles.lookup().findVarHandle(MethodHandlers.class, "statsTracker", HttpRouteStatsTracker.class);
38+
} catch (NoSuchFieldException | IllegalAccessException e) {
39+
throw new ExceptionInInitializerError(e);
40+
}
41+
}
2942

3043
MethodHandlers(String path) {
3144
this.path = path;
@@ -73,19 +86,26 @@ Set<RestRequest.Method> getValidMethods() {
7386
return methodHandlers.keySet();
7487
}
7588

76-
public void addRequestStats(int contentLength) {
77-
statsTracker.addRequestStats(contentLength);
78-
}
79-
80-
public void addResponseStats(long contentLength) {
81-
statsTracker.addResponseStats(contentLength);
89+
public HttpRouteStats getStats() {
90+
var tracker = existingStatsTracker();
91+
if (tracker == null) {
92+
return HttpRouteStats.EMPTY;
93+
}
94+
return tracker.getStats();
8295
}
8396

84-
public void addResponseTime(long timeMillis) {
85-
statsTracker.addResponseTime(timeMillis);
97+
public HttpRouteStatsTracker statsTracker() {
98+
var tracker = existingStatsTracker();
99+
if (tracker == null) {
100+
var newTracker = new HttpRouteStatsTracker();
101+
if ((tracker = (HttpRouteStatsTracker) STATS_TRACKER_HANDLE.compareAndExchange(this, null, newTracker)) == null) {
102+
tracker = newTracker;
103+
}
104+
}
105+
return tracker;
86106
}
87107

88-
public HttpRouteStats getStats() {
89-
return statsTracker.getStats();
108+
private HttpRouteStatsTracker existingStatsTracker() {
109+
return (HttpRouteStatsTracker) STATS_TRACKER_HANDLE.getAcquire(this);
90110
}
91111
}

server/src/main/java/org/elasticsearch/rest/RestController.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.elasticsearch.core.TimeValue;
3737
import org.elasticsearch.http.HttpHeadersValidationException;
3838
import org.elasticsearch.http.HttpRouteStats;
39+
import org.elasticsearch.http.HttpRouteStatsTracker;
3940
import org.elasticsearch.http.HttpServerTransport;
4041
import org.elasticsearch.indices.breaker.CircuitBreakerService;
4142
import org.elasticsearch.rest.RestHandler.Route;
@@ -914,7 +915,7 @@ public void sendResponse(RestResponse response) {
914915
private static final class ResourceHandlingHttpChannel extends DelegatingRestChannel {
915916
private final CircuitBreakerService circuitBreakerService;
916917
private final int contentLength;
917-
private final MethodHandlers methodHandlers;
918+
private final HttpRouteStatsTracker statsTracker;
918919
private final long startTime;
919920
private final AtomicBoolean closed = new AtomicBoolean();
920921

@@ -927,7 +928,7 @@ private static final class ResourceHandlingHttpChannel extends DelegatingRestCha
927928
super(delegate);
928929
this.circuitBreakerService = circuitBreakerService;
929930
this.contentLength = contentLength;
930-
this.methodHandlers = methodHandlers;
931+
this.statsTracker = methodHandlers.statsTracker();
931932
this.startTime = rawRelativeTimeInMillis();
932933
}
933934

@@ -936,12 +937,12 @@ public void sendResponse(RestResponse response) {
936937
boolean success = false;
937938
try {
938939
close();
939-
methodHandlers.addRequestStats(contentLength);
940-
methodHandlers.addResponseTime(rawRelativeTimeInMillis() - startTime);
940+
statsTracker.addRequestStats(contentLength);
941+
statsTracker.addResponseTime(rawRelativeTimeInMillis() - startTime);
941942
if (response.isChunked() == false) {
942-
methodHandlers.addResponseStats(response.content().length());
943+
statsTracker.addResponseStats(response.content().length());
943944
} else {
944-
final var responseLengthRecorder = new ResponseLengthRecorder(methodHandlers);
945+
final var responseLengthRecorder = new ResponseLengthRecorder(statsTracker);
945946
final var headers = response.getHeaders();
946947
response = RestResponse.chunked(
947948
response.status(),
@@ -976,23 +977,23 @@ private void close() {
976977
}
977978
}
978979

979-
private static class ResponseLengthRecorder extends AtomicReference<MethodHandlers> implements Releasable {
980+
private static class ResponseLengthRecorder extends AtomicReference<HttpRouteStatsTracker> implements Releasable {
980981
private long responseLength;
981982

982-
private ResponseLengthRecorder(MethodHandlers methodHandlers) {
983-
super(methodHandlers);
983+
private ResponseLengthRecorder(HttpRouteStatsTracker routeStatsTracker) {
984+
super(routeStatsTracker);
984985
}
985986

986987
@Override
987988
public void close() {
988989
// closed just before sending the last chunk, and also when the whole RestResponse is closed since the client might abort the
989990
// connection before we send the last chunk, in which case we won't have recorded the response in the
990991
// stats yet; thus we need run-once semantics here:
991-
final var methodHandlers = getAndSet(null);
992-
if (methodHandlers != null) {
992+
final var routeStatsTracker = getAndSet(null);
993+
if (routeStatsTracker != null) {
993994
// if we started sending chunks then we're closed on the transport worker, no need for sync
994995
assert responseLength == 0L || Transports.assertTransportThread();
995-
methodHandlers.addResponseStats(responseLength);
996+
routeStatsTracker.addResponseStats(responseLength);
996997
}
997998
}
998999

0 commit comments

Comments
 (0)