Skip to content

Commit a02f682

Browse files
Lazy initialize HttpRouteStatsTracker in MethodHandlers (#114107)
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 a0c1df0 commit a02f682

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;
@@ -879,7 +880,7 @@ public void sendResponse(RestResponse response) {
879880
private static final class ResourceHandlingHttpChannel extends DelegatingRestChannel {
880881
private final CircuitBreakerService circuitBreakerService;
881882
private final int contentLength;
882-
private final MethodHandlers methodHandlers;
883+
private final HttpRouteStatsTracker statsTracker;
883884
private final long startTime;
884885
private final AtomicBoolean closed = new AtomicBoolean();
885886

@@ -892,7 +893,7 @@ private static final class ResourceHandlingHttpChannel extends DelegatingRestCha
892893
super(delegate);
893894
this.circuitBreakerService = circuitBreakerService;
894895
this.contentLength = contentLength;
895-
this.methodHandlers = methodHandlers;
896+
this.statsTracker = methodHandlers.statsTracker();
896897
this.startTime = rawRelativeTimeInMillis();
897898
}
898899

@@ -901,12 +902,12 @@ public void sendResponse(RestResponse response) {
901902
boolean success = false;
902903
try {
903904
close();
904-
methodHandlers.addRequestStats(contentLength);
905-
methodHandlers.addResponseTime(rawRelativeTimeInMillis() - startTime);
905+
statsTracker.addRequestStats(contentLength);
906+
statsTracker.addResponseTime(rawRelativeTimeInMillis() - startTime);
906907
if (response.isChunked() == false) {
907-
methodHandlers.addResponseStats(response.content().length());
908+
statsTracker.addResponseStats(response.content().length());
908909
} else {
909-
final var responseLengthRecorder = new ResponseLengthRecorder(methodHandlers);
910+
final var responseLengthRecorder = new ResponseLengthRecorder(statsTracker);
910911
final var headers = response.getHeaders();
911912
response = RestResponse.chunked(
912913
response.status(),
@@ -941,23 +942,23 @@ private void close() {
941942
}
942943
}
943944

944-
private static class ResponseLengthRecorder extends AtomicReference<MethodHandlers> implements Releasable {
945+
private static class ResponseLengthRecorder extends AtomicReference<HttpRouteStatsTracker> implements Releasable {
945946
private long responseLength;
946947

947-
private ResponseLengthRecorder(MethodHandlers methodHandlers) {
948-
super(methodHandlers);
948+
private ResponseLengthRecorder(HttpRouteStatsTracker routeStatsTracker) {
949+
super(routeStatsTracker);
949950
}
950951

951952
@Override
952953
public void close() {
953954
// closed just before sending the last chunk, and also when the whole RestResponse is closed since the client might abort the
954955
// connection before we send the last chunk, in which case we won't have recorded the response in the
955956
// stats yet; thus we need run-once semantics here:
956-
final var methodHandlers = getAndSet(null);
957-
if (methodHandlers != null) {
957+
final var routeStatsTracker = getAndSet(null);
958+
if (routeStatsTracker != null) {
958959
// if we started sending chunks then we're closed on the transport worker, no need for sync
959960
assert responseLength == 0L || Transports.assertTransportThread();
960-
methodHandlers.addResponseStats(responseLength);
961+
routeStatsTracker.addResponseStats(responseLength);
961962
}
962963
}
963964

0 commit comments

Comments
 (0)