API Marshaling: Add generated marshalers for RESTXML protocol #1409

jasdel · 2017-07-19T21:53:23Z

This change is the start of an iterative set of changes that will
replace the SDK's current reflection base protocol marshalers with
generated code for each API type.

This works starts with the RESTXML protocol's encoders. The following
benchmarks show a significant improvement of execution speed and reduced
memory usage.

Benchmarks of RESTXML build and full request lifecycle between master and this change. Build benchmarks focus on just the marshaling of the request's input parameters. Request benchmarks provide a view of the impact of the changes in context to a full request lifecycle. Does not include network latency, nor umarshaling of a response object.

go test -tags bench -run NONE -bench ".*" -benchmem ./private/protocol/restxml

benchmark                                            old ns/op     new ns/op     delta
BenchmarkRESTXMLBuild_Complex_CFCreateDistro-8       264834        59891         -77.39%
BenchmarkRESTXMLBuild_Simple_CFDeleteDistro-8        9666          5894          -39.02%
BenchmarkRESTXMLBuild_REST_S3HeadObject-8            19723         7340          -62.78%
BenchmarkRESTXMLBuild_XML_S3PutObjectAcl-8           44739         15656         -65.01%
BenchmarkRESTXMLRequest_Complex_CFCreateDistro-8     586238        292755        -50.06%
BenchmarkRESTXMLRequest_Simple_CFDeleteDistro-8      168954        158484        -6.20%
BenchmarkRESTXMLRequest_REST_S3HeadObject-8          228720        199964        -12.57%
BenchmarkRESTXMLRequest_XML_S3PutObjectAcl-8         253821        206911        -18.48%

benchmark                                            old allocs     new allocs     delta
BenchmarkRESTXMLBuild_Complex_CFCreateDistro-8       1332           478            -64.11%
BenchmarkRESTXMLBuild_Simple_CFDeleteDistro-8        63             44             -30.16%
BenchmarkRESTXMLBuild_REST_S3HeadObject-8            134            57             -57.46%
BenchmarkRESTXMLBuild_XML_S3PutObjectAcl-8           247            113            -54.25%
BenchmarkRESTXMLRequest_Complex_CFCreateDistro-8     1641           786            -52.10%
BenchmarkRESTXMLRequest_Simple_CFDeleteDistro-8      230            211            -8.26%
BenchmarkRESTXMLRequest_REST_S3HeadObject-8          443            366            -17.38%
BenchmarkRESTXMLRequest_XML_S3PutObjectAcl-8         466            331            -28.97%

benchmark                                            old bytes     new bytes     delta
BenchmarkRESTXMLBuild_Complex_CFCreateDistro-8       87893         27936         -68.22%
BenchmarkRESTXMLBuild_Simple_CFDeleteDistro-8        8104          7880          -2.76%
BenchmarkRESTXMLBuild_REST_S3HeadObject-8            9544          8800          -7.80%
BenchmarkRESTXMLBuild_XML_S3PutObjectAcl-8           17701         11704         -33.88%
BenchmarkRESTXMLRequest_Complex_CFCreateDistro-8     141892        80852         -43.02%
BenchmarkRESTXMLRequest_Simple_CFDeleteDistro-8      52773         52543         -0.44%
BenchmarkRESTXMLRequest_REST_S3HeadObject-8          61947         61193         -1.22%
BenchmarkRESTXMLRequest_XML_S3PutObjectAcl-8         70800         64671         -8.66%

Work in progress PR:

Fill out missing rest or XML encoder implementations
Code generation templates need to be cleaned up.
Refactor code generation to do inline nested complex type marshaling. e.g. map[string][]int64
Verify correctness of marshaling
Linting
Make sure all unsupported encoder modes set error.

#1409

xibz · 2017-07-28T22:34:16Z

private/model/api/passes.go

@@ -61,6 +61,16 @@ func (a *API) resolveReferences() {
 			o.ErrorRefs[i].Shape.IsError = true
 		}
 	}
+
+	// TODO put this somewhere better


Is this TODO still valid? Or are we going to put this somewhere else later?

ah no I'll remove that

xibz · 2017-07-28T22:37:26Z

private/model/api/shape_marshal.go

+	}
+	if context.IsRefPayloadReader(refName, ref) {
+		if strings.HasSuffix(context.ShapeName, "Output") {
+			return "// Skipping " + refName + " Output type's body not valid."


Would prefer fmt.Sprintf here

xibz · 2017-07-28T22:37:42Z

private/model/api/shape_marshal.go

+// Will panic if error.
+func MarshalShapeRefGoCode(refName string, ref *ShapeRef, context *Shape) string {
+	if ref.XMLAttribute {
+		return "// Skipping " + refName + " XML Attribute."


fmt.Sprintf here

xibz · 2017-07-28T22:37:51Z

private/model/api/shape_marshal.go

+
+	switch mRef.Location() {
+	case "StatusCode":
+		return "// ignoring invalid encode state, StatusCode. " + refName


This change is the start of an iterative set of changes that will replace the SDK's current reflection base protocol marshalers with generated code for each API type. This works starts with the RESTXML protocol's encoders. The following benchmarks show a significant improvement of execution speed and reduced memory usage. Benchmarks of RESTXML build and full request lifecycle between master and this change. benchmark old ns/op new ns/op delta BenchmarkRESTXMLBuild_Complex_CFCreateDistro-8 264834 59891 -77.39% BenchmarkRESTXMLBuild_Simple_CFDeleteDistro-8 9666 5894 -39.02% BenchmarkRESTXMLBuild_REST_S3HeadObject-8 19723 7340 -62.78% BenchmarkRESTXMLBuild_XML_S3PutObjectAcl-8 44739 15656 -65.01% BenchmarkRESTXMLRequest_Complex_CFCreateDistro-8 586238 292755 -50.06% BenchmarkRESTXMLRequest_Simple_CFDeleteDistro-8 168954 158484 -6.20% BenchmarkRESTXMLRequest_REST_S3HeadObject-8 228720 199964 -12.57% BenchmarkRESTXMLRequest_XML_S3PutObjectAcl-8 253821 206911 -18.48% benchmark old allocs new allocs delta BenchmarkRESTXMLBuild_Complex_CFCreateDistro-8 1332 478 -64.11% BenchmarkRESTXMLBuild_Simple_CFDeleteDistro-8 63 44 -30.16% BenchmarkRESTXMLBuild_REST_S3HeadObject-8 134 57 -57.46% BenchmarkRESTXMLBuild_XML_S3PutObjectAcl-8 247 113 -54.25% BenchmarkRESTXMLRequest_Complex_CFCreateDistro-8 1641 786 -52.10% BenchmarkRESTXMLRequest_Simple_CFDeleteDistro-8 230 211 -8.26% BenchmarkRESTXMLRequest_REST_S3HeadObject-8 443 366 -17.38% BenchmarkRESTXMLRequest_XML_S3PutObjectAcl-8 466 331 -28.97% benchmark old bytes new bytes delta BenchmarkRESTXMLBuild_Complex_CFCreateDistro-8 87893 27936 -68.22% BenchmarkRESTXMLBuild_Simple_CFDeleteDistro-8 8104 7880 -2.76% BenchmarkRESTXMLBuild_REST_S3HeadObject-8 9544 8800 -7.80% BenchmarkRESTXMLBuild_XML_S3PutObjectAcl-8 17701 11704 -33.88% BenchmarkRESTXMLRequest_Complex_CFCreateDistro-8 141892 80852 -43.02% BenchmarkRESTXMLRequest_Simple_CFDeleteDistro-8 52773 52543 -0.44% BenchmarkRESTXMLRequest_REST_S3HeadObject-8 61947 61193 -1.22% BenchmarkRESTXMLRequest_XML_S3PutObjectAcl-8 70800 64671 -8.66%

…n slice instead of string.

validated offline

…aws#1409)" This reverts commit 37448a9. Reverts RESTXML due to aws#1550

* Revert "API Marshaler: Add generated marshalers for RESTJSON protocol (#1547)" This reverts commit 1a10a10. Reverts RESTJSON marshaler improvement due to bug in RESTXML * Revert "API Marshaling: Add generated marshalers for RESTXML protocol (#1409)" This reverts commit 37448a9. Reverts RESTXML due to #1550

jasdel self-assigned this Jul 19, 2017

jasdel added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Jul 19, 2017

jasdel changed the title ~~API Marshaling: Add generated (un)marshalers for RESTXML protocol~~ API Marshaling: Add generated marshalers for RESTXML protocol Jul 19, 2017

jasdel added a commit that referenced this pull request Jul 19, 2017

private/protocol/restxml: Cleanup and add new benchmarks (#1408)

c992785

#1409

jasdel force-pushed the feature/RESTXMLGenMarshalers branch 3 times, most recently from e9910f6 to 1c53899 Compare July 24, 2017 05:03

jasdel added the Review Needed label Jul 24, 2017

jasdel force-pushed the feature/RESTXMLGenMarshalers branch from 134c5ca to d5b167a Compare July 24, 2017 17:31

jasdel removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Jul 24, 2017

jasdel requested a review from xibz July 27, 2017 16:35

jasdel force-pushed the feature/RESTXMLGenMarshalers branch from d5b167a to f6ff120 Compare September 12, 2017 17:26

xibz previously requested changes Sep 21, 2017

View reviewed changes

jasdel added 5 commits September 22, 2017 15:12

regenerate service/tests

e1b575e

minor optimization for byte based protocols for MarshalValue to retur…

b3838e6

…n slice instead of string.

add errors for unsupported encoder methods

ba66c3d

PR fixup

9be458e

jasdel force-pushed the feature/RESTXMLGenMarshalers branch from f6ff120 to 9be458e Compare September 22, 2017 22:33

jasdel removed the Review Needed label Sep 22, 2017

jasdel merged commit 37448a9 into aws:master Sep 25, 2017

jasdel deleted the feature/RESTXMLGenMarshalers branch September 25, 2017 19:17

awstools mentioned this pull request Sep 26, 2017

v1.11.0 #1549

Merged

jasdel added a commit to jasdel/aws-sdk-go that referenced this pull request Sep 26, 2017

Revert "API Marshaling: Add generated marshalers for RESTXML protocol (…

e7dcfd5

…aws#1409)" This reverts commit 37448a9. Reverts RESTXML due to aws#1550

jasdel mentioned this pull request Dec 27, 2017

private/protocol: adding generated marshalers aws/aws-sdk-go-v2#76

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API Marshaling: Add generated marshalers for RESTXML protocol #1409

API Marshaling: Add generated marshalers for RESTXML protocol #1409

jasdel commented Jul 19, 2017 •

edited

Loading

xibz Jul 28, 2017

jasdel Sep 22, 2017

xibz Jul 28, 2017

xibz Jul 28, 2017

xibz Jul 28, 2017

API Marshaling: Add generated marshalers for RESTXML protocol #1409

API Marshaling: Add generated marshalers for RESTXML protocol #1409

Conversation

jasdel commented Jul 19, 2017 • edited Loading

xibz Jul 28, 2017

Choose a reason for hiding this comment

jasdel Sep 22, 2017

Choose a reason for hiding this comment

xibz Jul 28, 2017

Choose a reason for hiding this comment

xibz Jul 28, 2017

Choose a reason for hiding this comment

xibz Jul 28, 2017

Choose a reason for hiding this comment

jasdel commented Jul 19, 2017 •

edited

Loading