Skip to content

Commit a1bf7dd

Browse files
tysonmotejasdel
tysonmote
and
jasdel
authored
Pre-allocate response body using Content-Length (#1565)
Updates SDK API client deserialization to pre-allocate byte slice and string response payloads. This saves us allocations when the response body is greater than 512 bytes. For large responses, like multi-megabyte Lambda Invoke responses, this can be a very significant number of allocations and slice copies because ioutil.ReadAll / io.ReadAll start with 512 bytes and follow append's growth rules: * https://cs.opensource.google/go/go/+/refs/tags/go1.17.6:src/io/io.go;drc=dc289d3dcb59f80b9e23c7e8f237628359d21d92;l=627 * https://cs.opensource.google/go/go/%20/master:src/runtime/slice.go;l=166?q=growslice S3 GetBucketPolicy benchmark: name old time/op new time/op delta GetBucketPolicy-12 12.7ms ± 2% 8.7ms ± 2% -31.60% (p=0.000 n=9+9) name old alloc/op new alloc/op delta GetBucketPolicy-12 83.4MB ± 0% 50.4MB ± 0% -39.60% (p=0.000 n=10+10) name old allocs/op new allocs/op delta GetBucketPolicy-12 256 ± 0% 216 ± 0% -15.49% (p=0.000 n=10+10) Schemas GetCodeBindingSource benchmarks: name old time/op new time/op delta GetCodeBindingSource-12 10.8ms ± 3% 7.4ms ± 4% -31.79% (p=0.000 n=10+10) name old alloc/op new alloc/op delta GetCodeBindingSource-12 70.8MB ± 0% 37.8MB ± 0% -46.64% (p=0.000 n=10+10) name old allocs/op new allocs/op delta GetCodeBindingSource-12 241 ± 0% 200 ± 0% -17.01% (p=0.000 n=10+10) * Add changelog Co-authored-by: Jason Del Ponte <[email protected]>
1 parent 452ee5e commit a1bf7dd

File tree

25 files changed

+658
-224
lines changed

25 files changed

+658
-224
lines changed

‎.changelog/183e085d88ad43669b697fde5cbf7cc5.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"id": "183e085d-88ad-4366-9b69-7fde5cbf7cc5",
3+
"type": "bugfix",
4+
"collapse": true,
5+
"description": "Updates SDK API client deserialization to pre-allocate byte slice and string response payloads, [#1565](https://github.com/aws/aws-sdk-go-v2/pull/1565). Thanks to [Tyson Mote](https://github.com/tysonmote) for submitting this PR.",
6+
"modules": [
7+
"internal/protocoltest/awsrestjson",
8+
"internal/protocoltest/restxml",
9+
"service/apigateway",
10+
"service/apigatewayv2",
11+
"service/appconfig",
12+
"service/appconfigdata",
13+
"service/appsync",
14+
"service/cloudfront",
15+
"service/codeartifact",
16+
"service/codeguruprofiler",
17+
"service/dataexchange",
18+
"service/ebs",
19+
"service/glacier",
20+
"service/internal/benchmark",
21+
"service/iotdataplane",
22+
"service/kinesisvideoarchivedmedia",
23+
"service/kinesisvideomedia",
24+
"service/lakeformation",
25+
"service/lambda",
26+
"service/lexruntimeservice",
27+
"service/lexruntimev2",
28+
"service/location",
29+
"service/medialive",
30+
"service/mediastoredata",
31+
"service/polly",
32+
"service/s3",
33+
"service/sagemakerruntime",
34+
"service/schemas",
35+
"service/workmailmessageflow"
36+
]
37+
}

‎codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/RestJsonProtocolGenerator.java‎

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -130,18 +130,18 @@ protected void writeMiddlewarePayloadAsDocumentSerializerDelegator(
130130
writer.addUseImports(SmithyGoDependency.SMITHY_JSON);
131131
writer.addUseImports(SmithyGoDependency.BYTES);
132132
writer.write("""
133-
jsonEncoder := smithyjson.NewEncoder()
134-
if err := $L($L, jsonEncoder.Value); err != nil {
135-
return out, metadata, &smithy.SerializationError{Err: err}
136-
}
137-
payload := bytes.NewReader(jsonEncoder.Bytes())""", functionName, s);
133+
jsonEncoder := smithyjson.NewEncoder()
134+
if err := $L($L, jsonEncoder.Value); err != nil {
135+
return out, metadata, &smithy.SerializationError{Err: err}
136+
}
137+
payload := bytes.NewReader(jsonEncoder.Bytes())""", functionName, s);
138138
writeSetStream(writer, "payload");
139139
if (payloadShape.getType() == ShapeType.STRUCTURE) {
140140
writer.openBlock("} else {", "", () -> {
141141
writer.write("""
142-
jsonEncoder := smithyjson.NewEncoder()
143-
jsonEncoder.Value.Object().Close()
144-
payload := bytes.NewReader(jsonEncoder.Bytes())""");
142+
jsonEncoder := smithyjson.NewEncoder()
143+
jsonEncoder.Value.Object().Close()
144+
payload := bytes.NewReader(jsonEncoder.Bytes())""");
145145
writeSetStream(writer, "payload");
146146
});
147147
}
@@ -186,7 +186,8 @@ protected void writeMiddlewareDocumentSerializerDelegator(
186186
writer.write("");
187187

188188
Shape inputShape = ProtocolUtils.expectInput(context.getModel(), operation);
189-
String functionName = ProtocolGenerator.getDocumentSerializerFunctionName(inputShape, context.getService(), getProtocolName());
189+
String functionName = ProtocolGenerator.getDocumentSerializerFunctionName(inputShape, context.getService(),
190+
getProtocolName());
190191

191192
writer.addUseImports(SmithyGoDependency.SMITHY_JSON);
192193
writer.write("jsonEncoder := smithyjson.NewEncoder()");
@@ -238,7 +239,8 @@ protected void writeMiddlewareDocumentDeserializerDelegator(
238239

239240
// if target shape is of type String or type Blob, then delegate deserializers for explicit payload shapes
240241
if (payloadShape.isStringShape() || payloadShape.isBlobShape()) {
241-
writeMiddlewarePayloadBindingDeserializerDelegator(writer, context.getService(), targetShape);
242+
writeMiddlewarePayloadBindingDeserializerDelegator(writer, context.getService(), targetShape,
243+
payloadShape);
242244
return;
243245
}
244246
// for other payload target types we should deserialize using the appropriate document deserializer
@@ -266,10 +268,17 @@ protected void writeMiddlewareDocumentDeserializerDelegator(
266268

267269
// Writes middleware that delegates to deserializers for shapes that have explicit payload.
268270
private void writeMiddlewarePayloadBindingDeserializerDelegator(
269-
GoWriter writer, ServiceShape service, Shape shape
271+
GoWriter writer, ServiceShape service, Shape outputShape, Shape payloadShape
270272
) {
271-
String deserFuncName = ProtocolGenerator.getDocumentDeserializerFunctionName(shape, service, getProtocolName());
272-
writer.write("err = $L(output, response.Body)", deserFuncName);
273+
String deserFuncName = ProtocolGenerator.getDocumentDeserializerFunctionName(outputShape, service,
274+
getProtocolName());
275+
276+
String contentLengthParam = "";
277+
if (!payloadShape.hasTrait(StreamingTrait.class)) {
278+
contentLengthParam = "response.ContentLength";
279+
}
280+
281+
writer.write("err = $L(output, response.Body, $L)", deserFuncName, contentLengthParam);
273282
writer.openBlock("if err != nil {", "}", () -> {
274283
writer.addUseImports(SmithyGoDependency.SMITHY);
275284
writer.write(String.format("return out, metadata, &smithy.DeserializationError{Err:%s}",
@@ -395,7 +404,8 @@ private void writePayloadBindingDeserializer(
395404
var writer = context.getWriter().get();
396405
var symbolProvider = context.getSymbolProvider();
397406
var shapeSymbol = symbolProvider.toSymbol(shape);
398-
var funcName = ProtocolGenerator.getDocumentDeserializerFunctionName(shape, context.getService(), getProtocolName());
407+
var funcName = ProtocolGenerator.getDocumentDeserializerFunctionName(shape, context.getService(),
408+
getProtocolName());
399409

400410
for (var memberShape : new TreeSet<>(shape.members())) {
401411
if (!filterMemberShapes.test(memberShape)) {
@@ -405,26 +415,40 @@ private void writePayloadBindingDeserializer(
405415
var memberName = symbolProvider.toMemberName(memberShape);
406416
var targetShape = context.getModel().expectShape(memberShape.getTarget());
407417
if (targetShape.isStringShape() || targetShape.isBlobShape()) {
408-
writer.openBlock("func $L(v $P, body io.ReadCloser) error {", "}",
409-
funcName, shapeSymbol, () -> {
418+
419+
String contentLengthParam = "";
420+
if (!targetShape.hasTrait(StreamingTrait.class)) {
421+
contentLengthParam = "contentLength int64";
422+
}
423+
424+
writer.openBlock("func $L(v $P, body io.ReadCloser, $L) error {", "}",
425+
funcName, shapeSymbol, contentLengthParam, () -> {
410426
writer.openBlock("if v == nil {", "}", () -> {
411427
writer.write("return fmt.Errorf(\"unsupported deserialization of nil %T\", v)");
412428
});
413429
writer.write("");
414430

415431
if (!targetShape.hasTrait(StreamingTrait.class)) {
416-
writer.addUseImports(SmithyGoDependency.IOUTIL);
417-
writer.write("bs, err := ioutil.ReadAll(body)");
432+
writer.addUseImports(SmithyGoDependency.BYTES);
433+
writer.write("var buf bytes.Buffer");
434+
writer.openBlock("if contentLength > 0 {", "", () -> {
435+
writer.write("buf.Grow(int(contentLength))");
436+
writer.openBlock("} else {", "}", () -> {
437+
writer.write("buf.Grow(512)");
438+
});
439+
});
440+
writer.write("_, err := buf.ReadFrom(body)");
418441
writer.write("if err != nil { return err }");
419-
writer.openBlock("if len(bs) > 0 {", "}", () -> {
442+
writer.openBlock("if buf.Len() > 0 {", "}", () -> {
420443
if (targetShape.isBlobShape()) {
421-
writer.write("v.$L = bs", memberName);
444+
writer.write("v.$L = buf.Bytes()", memberName);
422445
} else { // string
423446
writer.addUseImports(SmithyGoDependency.SMITHY_PTR);
424447
if (targetShape.hasTrait(EnumTrait.class)) {
425-
writer.write("v.$L = $T(bs)", memberName, symbolProvider.toSymbol(targetShape));
448+
writer.write("v.$L = $T(buf.Bytes())", memberName,
449+
symbolProvider.toSymbol(targetShape));
426450
} else {
427-
writer.write("v.$L = ptr.String(string(bs))", memberName);
451+
writer.write("v.$L = ptr.String(buf.String())", memberName);
428452
}
429453
}
430454
});
@@ -511,7 +535,7 @@ protected void generateEventStreamSerializers(
511535

512536
var hasBindings = targetShape.members().stream()
513537
.filter(ms -> ms.getTrait(EventHeaderTrait.class).isPresent()
514-
|| ms.getTrait(EventPayloadTrait.class).isPresent())
538+
|| ms.getTrait(EventPayloadTrait.class).isPresent())
515539
.findAny();
516540
if (hasBindings.isPresent()) {
517541
var payload = targetShape.members().stream()
@@ -571,13 +595,13 @@ protected void generateEventStreamDeserializers(
571595
payloadTarget, ctx.getService(), ctx.getProtocolName());
572596
var ctxWriter = ctx.getWriter().get();
573597
ctxWriter.openBlock("if err := $L(&$L, shape); err != nil {", "}", functionName, operand,
574-
() -> handleDecodeError(ctxWriter))
598+
() -> handleDecodeError(ctxWriter))
575599
.write("return nil");
576600
});
577601

578602
var hasBindings = targetShape.members().stream()
579603
.filter(ms -> ms.getTrait(EventHeaderTrait.class).isPresent()
580-
|| ms.getTrait(EventPayloadTrait.class).isPresent())
604+
|| ms.getTrait(EventPayloadTrait.class).isPresent())
581605
.findAny();
582606
if (hasBindings.isPresent()) {
583607
var payload = targetShape.members().stream()

0 commit comments

Comments
 (0)