Skip to content
This repository was archived by the owner on Jun 30, 2023. It is now read-only.

Commit aa4914e

Browse files
Clément Denistangiel
Clément Denis
authored and
tangiel
committed
Improve CPU and memory usage by serializing the response object directly
- Prevents an intermediate String object from being created - Content-Length header is not set calculated by default anymore - Content-Length can be reenabled with addContentLength=true init param
1 parent 2f34aad commit aa4914e

File tree

7 files changed

+70
-45
lines changed

7 files changed

+70
-45
lines changed

‎endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public abstract class ServletInitializationParameters {
3838
private static final String ILLEGAL_ARGUMENT_BACKEND_ERROR = "illegalArgumentIsBackendError";
3939
private static final String EXCEPTION_COMPATIBILITY = "enableExceptionCompatibility";
4040
private static final String PRETTY_PRINT = "prettyPrint";
41+
private static final String ADD_CONTENT_LENGTH = "addContentLength";
4142

4243
private static final Splitter CSV_SPLITTER = Splitter.on(',').omitEmptyStrings().trimResults();
4344
private static final Joiner CSV_JOINER = Joiner.on(',').skipNulls();
@@ -83,13 +84,22 @@ public String apply(Class<?> clazz) {
8384
*/
8485
public abstract boolean isPrettyPrintEnabled();
8586

87+
/**
88+
* Returns if the Content-Length header should be set on response. Should be disabled when running
89+
* on App Engine, as Content-Length header is discarded by front-end servers. If enabled, has a
90+
* small negative impact on CPU usage and latency.
91+
*
92+
*/
93+
public abstract boolean isAddContentLength();
94+
8695
public static Builder builder() {
8796
return new AutoValue_ServletInitializationParameters.Builder()
8897
.setServletRestricted(true)
8998
.setClientIdWhitelistEnabled(true)
9099
.setIllegalArgumentBackendError(false)
91100
.setExceptionCompatibilityEnabled(true)
92-
.setPrettyPrintEnabled(true);
101+
.setPrettyPrintEnabled(true)
102+
.setAddContentLength(false);
93103
}
94104

95105
/**
@@ -160,6 +170,11 @@ public Builder setRestricted(boolean servletRestricted) {
160170
*/
161171
public abstract Builder setPrettyPrintEnabled(boolean prettyPrint);
162172

173+
/**
174+
* Sets if the content length header should be set. Defaults to {@code false}.
175+
*/
176+
public abstract Builder setAddContentLength(boolean addContentLength);
177+
163178
abstract ServletInitializationParameters autoBuild();
164179

165180
public ServletInitializationParameters build() {
@@ -203,6 +218,10 @@ public static ServletInitializationParameters fromServletConfig(
203218
if (prettyPrint != null) {
204219
builder.setPrettyPrintEnabled(parseBoolean(prettyPrint, PRETTY_PRINT));
205220
}
221+
String addContentLength = config.getInitParameter(ADD_CONTENT_LENGTH);
222+
if (addContentLength != null) {
223+
builder.setAddContentLength(parseBoolean(addContentLength, ADD_CONTENT_LENGTH));
224+
}
206225
}
207226
return builder.build();
208227
}
@@ -238,6 +257,7 @@ public ImmutableMap<String, String> asMap() {
238257
.put(ILLEGAL_ARGUMENT_BACKEND_ERROR, Boolean.toString(isIllegalArgumentBackendError()))
239258
.put(EXCEPTION_COMPATIBILITY, Boolean.toString(isExceptionCompatibilityEnabled()))
240259
.put(PRETTY_PRINT, Boolean.toString(isPrettyPrintEnabled()))
260+
.put(ADD_CONTENT_LENGTH, Boolean.toString(isAddContentLength()))
241261
.build();
242262
}
243263
}

‎endpoints-framework/src/main/java/com/google/api/server/spi/handlers/EndpointsMethodHandler.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ protected ResultWriter createResultWriter(EndpointsContext context,
9191
ApiSerializationConfig serializationConfig) {
9292
return new RestResponseResultWriter(context.getResponse(), serializationConfig,
9393
StandardParameters.shouldPrettyPrint(context),
94+
initParameters.isAddContentLength(),
9495
initParameters.isExceptionCompatibilityEnabled());
9596
}
9697

‎endpoints-framework/src/main/java/com/google/api/server/spi/response/RestResponseResultWriter.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,11 @@
1616
package com.google.api.server.spi.response;
1717

1818
import com.fasterxml.jackson.databind.ObjectMapper;
19-
import com.fasterxml.jackson.databind.node.ArrayNode;
2019
import com.fasterxml.jackson.databind.node.ObjectNode;
2120
import com.google.api.server.spi.ObjectMapperUtil;
2221
import com.google.api.server.spi.ServiceException;
2322
import com.google.api.server.spi.config.model.ApiSerializationConfig;
2423
import com.google.common.base.Strings;
25-
import com.google.common.collect.ImmutableList;
26-
import com.google.common.collect.ImmutableMap;
2724

2825
import java.io.IOException;
2926

@@ -39,8 +36,8 @@ public class RestResponseResultWriter extends ServletResponseResultWriter {
3936

4037
public RestResponseResultWriter(
4138
HttpServletResponse servletResponse, ApiSerializationConfig serializationConfig,
42-
boolean prettyPrint, boolean enableExceptionCompatibility) {
43-
super(servletResponse, serializationConfig, prettyPrint);
39+
boolean prettyPrint, boolean addContentLength, boolean enableExceptionCompatibility) {
40+
super(servletResponse, serializationConfig, prettyPrint, addContentLength);
4441
this.enableExceptionCompatibility = enableExceptionCompatibility;
4542
this.objectMapper = ObjectMapperUtil.createStandardObjectMapper(serializationConfig);
4643
}
@@ -70,8 +67,7 @@ public void writeError(ServiceException e) throws IOException {
7067
e.getReason() : errorMap.getReason(e.getStatusCode());
7168
String domain = !Strings.isNullOrEmpty(e.getDomain()) ?
7269
e.getDomain() : errorMap.getDomain(e.getStatusCode());
73-
write(code, e.getHeaders(),
74-
writeValueAsString(createError(code, reason, domain, e.getMessage())));
70+
write(code, e.getHeaders(), createError(code, reason, domain, e.getMessage()));
7571
}
7672

7773
private Object createError(int code, String reason, String domain, String message) {

‎endpoints-framework/src/main/java/com/google/api/server/spi/response/ServletResponseResultWriter.java‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,28 @@
2323
import com.google.api.server.spi.types.DateAndTime;
2424
import com.google.api.server.spi.types.SimpleDate;
2525
import com.google.appengine.api.datastore.Blob;
26+
import com.google.common.io.ByteStreams;
27+
import com.google.common.io.CountingOutputStream;
2628

2729
import com.fasterxml.jackson.core.JsonGenerator;
28-
import com.fasterxml.jackson.core.JsonProcessingException;
2930
import com.fasterxml.jackson.core.Version;
3031
import com.fasterxml.jackson.databind.JsonSerializer;
3132
import com.fasterxml.jackson.databind.ObjectWriter;
3233
import com.fasterxml.jackson.databind.SerializerProvider;
3334
import com.fasterxml.jackson.databind.module.SimpleModule;
3435

36+
import java.io.BufferedOutputStream;
37+
import java.io.BufferedWriter;
3538
import java.io.IOException;
39+
import java.nio.charset.StandardCharsets;
3640
import java.util.Collections;
3741
import java.util.Date;
3842
import java.util.HashMap;
3943
import java.util.LinkedHashSet;
4044
import java.util.Map;
4145
import java.util.Set;
4246

47+
import javax.servlet.ServletOutputStream;
4348
import javax.servlet.http.HttpServletResponse;
4449

4550
/**
@@ -68,15 +73,16 @@ public class ServletResponseResultWriter implements ResultWriter {
6873

6974
private final HttpServletResponse servletResponse;
7075
private final ObjectWriter objectWriter;
76+
private final boolean addContentLength;
7177

7278
public ServletResponseResultWriter(
7379
HttpServletResponse servletResponse, ApiSerializationConfig serializationConfig) {
74-
this(servletResponse, serializationConfig, false /* prettyPrint */);
80+
this(servletResponse, serializationConfig, false /* prettyPrint */, false /* addContentLength */);
7581
}
7682

7783
public ServletResponseResultWriter(
7884
HttpServletResponse servletResponse, ApiSerializationConfig serializationConfig,
79-
boolean prettyPrint) {
85+
boolean prettyPrint, boolean addContentLength) {
8086
this.servletResponse = servletResponse;
8187
Set<SimpleModule> modules = new LinkedHashSet<>();
8288
modules.addAll(WRITER_MODULES);
@@ -90,27 +96,26 @@ public ServletResponseResultWriter(
9096
objectWriter = objectWriter.with(new EndpointsPrettyPrinter());
9197
}
9298
this.objectWriter = objectWriter;
99+
this.addContentLength = addContentLength;
93100
}
94101

95102
@Override
96103
public void write(Object response) throws IOException {
97104
if (response == null) {
98105
write(HttpServletResponse.SC_NO_CONTENT, null, null);
99106
} else {
100-
write(HttpServletResponse.SC_OK, null,
101-
writeValueAsString(ResponseUtil.wrapCollection(response)));
107+
write(HttpServletResponse.SC_OK, null, ResponseUtil.wrapCollection(response));
102108
}
103109
}
104110

105111
@Override
106112
public void writeError(ServiceException e) throws IOException {
107113
Map<String, String> errors = new HashMap<>();
108114
errors.put(Constant.ERROR_MESSAGE, e.getMessage());
109-
write(e.getStatusCode(), e.getHeaders(),
110-
writeValueAsString(errors));
115+
write(e.getStatusCode(), e.getHeaders(), errors);
111116
}
112117

113-
protected void write(int status, Map<String, String> headers, String content) throws IOException {
118+
protected void write(int status, Map<String, String> headers, Object content) throws IOException {
114119
// write response status code
115120
servletResponse.setStatus(status);
116121

@@ -124,8 +129,12 @@ protected void write(int status, Map<String, String> headers, String content) th
124129
// write response body
125130
if (content != null) {
126131
servletResponse.setContentType(SystemService.MIME_JSON);
127-
servletResponse.setContentLength(content.getBytes("UTF-8").length);
128-
servletResponse.getWriter().write(content);
132+
if (addContentLength) {
133+
CountingOutputStream counter = new CountingOutputStream(ByteStreams.nullOutputStream());
134+
objectWriter.writeValue(counter, content);
135+
servletResponse.setContentLength((int) counter.getCount());
136+
}
137+
objectWriter.writeValue(servletResponse.getOutputStream(), content);
129138
}
130139
}
131140

@@ -203,13 +212,4 @@ public void serialize(Blob value, JsonGenerator jgen, SerializerProvider provide
203212
return writeBlobAsBase64Module;
204213
}
205214

206-
// Writes a value as a JSON string and translates Jackson exceptions into IOException.
207-
protected String writeValueAsString(Object value)
208-
throws IOException {
209-
try {
210-
return objectWriter.writeValueAsString(value);
211-
} catch (JsonProcessingException e) {
212-
throw new IOException(e);
213-
}
214-
}
215215
}

‎endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ public void testBuilder_defaults() {
4848
assertThat(initParameters.isIllegalArgumentBackendError()).isFalse();
4949
assertThat(initParameters.isExceptionCompatibilityEnabled()).isTrue();
5050
assertThat(initParameters.isPrettyPrintEnabled()).isTrue();
51-
verifyAsMap(initParameters, "", "true", "true", "false", "true", "true");
51+
assertThat(initParameters.isAddContentLength()).isFalse();
52+
verifyAsMap(initParameters, "", "true", "true", "false", "true", "true", "false");
5253
}
5354

5455
@Test
@@ -60,13 +61,14 @@ public void testBuilder_emptySetsAndTrue() {
6061
.setIllegalArgumentBackendError(true)
6162
.setExceptionCompatibilityEnabled(true)
6263
.setPrettyPrintEnabled(true)
64+
.setAddContentLength(true)
6365
.build();
6466
assertThat(initParameters.getServiceClasses()).isEmpty();
6567
assertThat(initParameters.isServletRestricted()).isTrue();
6668
assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue();
6769
assertThat(initParameters.isIllegalArgumentBackendError()).isTrue();
6870
assertThat(initParameters.isExceptionCompatibilityEnabled()).isTrue();
69-
verifyAsMap(initParameters, "", "true", "true", "true", "true", "true");
71+
verifyAsMap(initParameters, "", "true", "true", "true", "true", "true", "true");
7072
}
7173

7274
@Test
@@ -78,12 +80,13 @@ public void testBuilder_oneEntrySetsAndFalse() {
7880
.setIllegalArgumentBackendError(false)
7981
.setExceptionCompatibilityEnabled(false)
8082
.setPrettyPrintEnabled(false)
83+
.setAddContentLength(false)
8184
.build();
8285
assertThat(initParameters.getServiceClasses()).containsExactly(String.class);
8386
assertThat(initParameters.isServletRestricted()).isFalse();
8487
assertThat(initParameters.isClientIdWhitelistEnabled()).isFalse();
8588
verifyAsMap(
86-
initParameters, String.class.getName(), "false", "false", "false", "false", "false");
89+
initParameters, String.class.getName(), "false", "false", "false", "false", "false","false");
8790
}
8891

8992
@Test
@@ -93,7 +96,7 @@ public void testBuilder_twoEntrySets() {
9396
.build();
9497
assertThat(initParameters.getServiceClasses()).containsExactly(String.class, Integer.class);
9598
verifyAsMap(initParameters, String.class.getName() + ',' + Integer.class.getName(), "true",
96-
"true", "false", "true", "true");
99+
"true", "false", "true", "true", "false");
97100
}
98101

99102
@Test
@@ -108,7 +111,7 @@ public void testFromServletConfig_nullConfig() throws ServletException {
108111
@Test
109112
public void testFromServletConfig_nullValues() throws ServletException {
110113
ServletInitializationParameters initParameters =
111-
fromServletConfig(null, null, null, null, null, null);
114+
fromServletConfig(null, null, null, null, null, null, null);
112115
assertThat(initParameters.getServiceClasses()).isEmpty();
113116
assertThat(initParameters.isServletRestricted()).isTrue();
114117
assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue();
@@ -120,7 +123,7 @@ public void testFromServletConfig_nullValues() throws ServletException {
120123
@Test
121124
public void testFromServletConfig_emptySetsAndFalse() throws ServletException {
122125
ServletInitializationParameters initParameters =
123-
fromServletConfig("", "false", "false", "false", "false", "false");
126+
fromServletConfig("", "false", "false", "false", "false", "false", "false");
124127
assertThat(initParameters.getServiceClasses()).isEmpty();
125128
assertThat(initParameters.isServletRestricted()).isFalse();
126129
assertThat(initParameters.isClientIdWhitelistEnabled()).isFalse();
@@ -132,7 +135,7 @@ public void testFromServletConfig_emptySetsAndFalse() throws ServletException {
132135
@Test
133136
public void testFromServletConfig_oneEntrySetsAndTrue() throws ServletException {
134137
ServletInitializationParameters initParameters =
135-
fromServletConfig(String.class.getName(), "true", "true", "true", "true", "true");
138+
fromServletConfig(String.class.getName(), "true", "true", "true", "true", "true", "true");
136139
assertThat(initParameters.getServiceClasses()).containsExactly(String.class);
137140
assertThat(initParameters.isServletRestricted()).isTrue();
138141
assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue();
@@ -144,22 +147,22 @@ public void testFromServletConfig_oneEntrySetsAndTrue() throws ServletException
144147
@Test
145148
public void testFromServletConfig_twoEntrySets() throws ServletException {
146149
ServletInitializationParameters initParameters = fromServletConfig(
147-
String.class.getName() + ',' + Integer.class.getName(), null, null, null, null, null);
150+
String.class.getName() + ',' + Integer.class.getName(), null, null, null, null, null, null);
148151
assertThat(initParameters.getServiceClasses()).containsExactly(String.class, Integer.class);
149152
}
150153

151154
@Test
152155
public void testFromServletConfig_skipsEmptyElements() throws ServletException {
153156
ServletInitializationParameters initParameters = fromServletConfig(
154157
",," + String.class.getName() + ",,," + Integer.class.getName() + ",", null, null, null,
155-
null, null);
158+
null, null, null);
156159
assertThat(initParameters.getServiceClasses()).containsExactly(String.class, Integer.class);
157160
}
158161

159162
@Test
160163
public void testFromServletConfig_invalidRestrictedThrows() throws ServletException {
161164
try {
162-
fromServletConfig(null, "yes", null, null, null, null);
165+
fromServletConfig(null, "yes", null, null, null, null, null);
163166
fail("Expected IllegalArgumentException");
164167
} catch (IllegalArgumentException expected) {
165168
// expected
@@ -170,25 +173,27 @@ private void verifyAsMap(
170173
ServletInitializationParameters initParameters, String serviceClasses,
171174
String isServletRestricted, String isClientIdWhitelistEnabled,
172175
String isIllegalArgumentBackendError, String isExceptionCompatibilityEnabled,
173-
String isPrettyPrintEnabled) {
176+
String isPrettyPrintEnabled, String isAddContentLength) {
174177
Map<String, String> map = initParameters.asMap();
175-
assertThat(map).hasSize(6);
178+
assertThat(map).hasSize(7);
176179
assertThat(map.get("services")).isEqualTo(serviceClasses);
177180
assertThat(map.get("restricted")).isEqualTo(isServletRestricted);
178181
assertThat(map.get("clientIdWhitelistEnabled")).isEqualTo(isClientIdWhitelistEnabled);
179182
assertThat(map.get("illegalArgumentIsBackendError")).isEqualTo(isIllegalArgumentBackendError);
180183
assertThat(map.get("enableExceptionCompatibility")).isEqualTo(isExceptionCompatibilityEnabled);
181184
assertThat(map.get("prettyPrint")).isEqualTo(isPrettyPrintEnabled);
185+
assertThat(map.get("addContentLength")).isEqualTo(isAddContentLength);
182186
}
183187

184188
private ServletInitializationParameters fromServletConfig(
185189
String serviceClasses, String isServletRestricted,
186190
String isClientIdWhitelistEnabled, String isIllegalArgumentBackendError,
187-
String isExceptionCompatibilityEnabled, String isPrettyPrintEnabled)
191+
String isExceptionCompatibilityEnabled, String isPrettyPrintEnabled,
192+
String isAddContentLength)
188193
throws ServletException {
189194
ServletConfig servletConfig = new StubServletConfig(serviceClasses,
190195
isServletRestricted, isClientIdWhitelistEnabled, isIllegalArgumentBackendError,
191-
isExceptionCompatibilityEnabled, isPrettyPrintEnabled);
196+
isExceptionCompatibilityEnabled, isPrettyPrintEnabled, isAddContentLength);
192197
return ServletInitializationParameters.fromServletConfig(
193198
servletConfig, getClass().getClassLoader());
194199
}
@@ -199,14 +204,15 @@ private static class StubServletConfig implements ServletConfig {
199204
public StubServletConfig(
200205
String serviceClasses, String isServletRestricted, String isClientIdWhitelistEnabled,
201206
String isIllegalArgumentBackendError, String isExceptionCompatibilityEnabled,
202-
String isPrettyPrintEnabled) {
207+
String isPrettyPrintEnabled, String isAddContentLength) {
203208
initParameters = Maps.newHashMap();
204209
initParameters.put("services", serviceClasses);
205210
initParameters.put("restricted", isServletRestricted);
206211
initParameters.put("clientIdWhitelistEnabled", isClientIdWhitelistEnabled);
207212
initParameters.put("illegalArgumentIsBackendError", isIllegalArgumentBackendError);
208213
initParameters.put("enableExceptionCompatibility", isExceptionCompatibilityEnabled);
209214
initParameters.put("prettyPrint", isPrettyPrintEnabled);
215+
initParameters.put("addContentLength", isAddContentLength);
210216
}
211217

212218
@Override

‎endpoints-framework/src/test/java/com/google/api/server/spi/response/RestResponseResultWriterTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,8 @@ private void writeError(int exceptionCode, int expectedCode, String reason, Stri
174174
boolean enableExceptionCompatibility) throws Exception {
175175
MockHttpServletResponse response = new MockHttpServletResponse();
176176
RestResponseResultWriter writer = new RestResponseResultWriter(
177-
response, null, true /* prettyPrint */, enableExceptionCompatibility);
177+
response, null, true /* prettyPrint */,
178+
true /* addContentLength */, enableExceptionCompatibility);
178179
writer.writeError(new ServiceException(exceptionCode, message));
179180
ObjectMapper mapper = ObjectMapperUtil.createStandardObjectMapper();
180181
ObjectNode content = mapper.readValue(response.getContentAsString(), ObjectNode.class);
@@ -209,7 +210,8 @@ private void writeError(boolean enableExceptionCompatibility, String customReaso
209210
String expectedReason, String customDomain, String expectedDomain) throws Exception {
210211
MockHttpServletResponse response = new MockHttpServletResponse();
211212
RestResponseResultWriter writer = new RestResponseResultWriter(
212-
response, null, true /* prettyPrint */, enableExceptionCompatibility);
213+
response, null, true /* prettyPrint */,
214+
true /* addContentLength */, enableExceptionCompatibility);
213215
writer.writeError(new ServiceException(400, "error", customReason, customDomain));
214216
ObjectMapper mapper = ObjectMapperUtil.createStandardObjectMapper();
215217
ObjectNode content = mapper.readValue(response.getContentAsString(), ObjectNode.class);

‎endpoints-framework/src/test/java/com/google/api/server/spi/response/ServletResponseResultWriterTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public void testWriteErrorResponseHeaders() throws Exception {
172172
public void testPrettyPrint() throws Exception {
173173
MockHttpServletResponse response = new MockHttpServletResponse();
174174
ServletResponseResultWriter writer = new ServletResponseResultWriter(response, null,
175-
true /* prettyPrint */);
175+
true /* prettyPrint */, true /* addContentLength */);
176176
writer.write(ImmutableMap.of("one", "two", "three", "four"));
177177
// If the response is pretty printed, there should be at least two newlines.
178178
String body = response.getContentAsString();

0 commit comments

Comments
 (0)