From ed8a3caa0df9ce36a5b60aebeee201187098d205 Mon Sep 17 00:00:00 2001
From: Kyle Eckhart <kgeckhart@users.noreply.github.com>
Date: Thu, 23 Jan 2025 21:25:38 -0500
Subject: [PATCH] Reduce fmt.Sprintf allocations in query encoding (#2919)

* Pre-compute prefix when array is not flat

* Switch to string concat for object keys

* Change array key formatting and add comments about why sprintf is not used

* Add changelog entry

* Update .changelog/fd3c62c5-c1cf-48de-a223-ea0fdf4136c9.json

---------

Co-authored-by: Luc Talatinian <102624213+lucix-aws@users.noreply.github.com>
---
 .../fd3c62c5-c1cf-48de-a223-ea0fdf4136c9.json |  8 +++
 aws/protocol/query/array.go                   | 29 +++------
 aws/protocol/query/object.go                  | 11 ++--
 aws/protocol/query/value.go                   |  2 +
 aws/protocol/query/value_test.go              | 64 +++++++++++++++++++
 5 files changed, 88 insertions(+), 26 deletions(-)
 create mode 100644 .changelog/fd3c62c5-c1cf-48de-a223-ea0fdf4136c9.json
 create mode 100644 aws/protocol/query/value_test.go

diff --git a/.changelog/fd3c62c5-c1cf-48de-a223-ea0fdf4136c9.json b/.changelog/fd3c62c5-c1cf-48de-a223-ea0fdf4136c9.json
new file mode 100644
index 00000000000..7208ae19a19
--- /dev/null
+++ b/.changelog/fd3c62c5-c1cf-48de-a223-ea0fdf4136c9.json
@@ -0,0 +1,8 @@
+{
+  "id": "fd3c62c5-c1cf-48de-a223-ea0fdf4136c9",
+  "type": "feature",
+  "description": "Reduce allocations in query encoding.",
+  "modules": [
+    "."
+  ]
+}
\ No newline at end of file
diff --git a/aws/protocol/query/array.go b/aws/protocol/query/array.go
index 47ebc0f5476..6669a3ddfd0 100644
--- a/aws/protocol/query/array.go
+++ b/aws/protocol/query/array.go
@@ -1,8 +1,8 @@
 package query
 
 import (
-	"fmt"
 	"net/url"
+	"strconv"
 )
 
 // Array represents the encoding of Query lists and sets. A Query array is a
@@ -21,19 +21,8 @@ type Array struct {
 	// keys for each element in the list. For example, an entry might have the
 	// key "ParentStructure.ListName.member.MemberName.1".
 	//
-	// While this is currently represented as a string that gets added to, it
-	// could also be represented as a stack that only gets condensed into a
-	// string when a finalized key is created. This could potentially reduce
-	// allocations.
+	// When the array is not flat the prefix will contain the memberName otherwise the memberName is ignored
 	prefix string
-	// Whether the list is flat or not. A list that is not flat will produce the
-	// following entry to the url.Values for a given entry:
-	//     ListName.MemberName.1=value
-	// A list that is flat will produce the following:
-	//     ListName.1=value
-	flat bool
-	// The location name of the member. In most cases this should be "member".
-	memberName string
 	// Elements are stored in values, so we keep track of the list size here.
 	size int32
 	// Empty lists are encoded as "<prefix>=", if we add a value later we will
@@ -45,11 +34,14 @@ func newArray(values url.Values, prefix string, flat bool, memberName string) *A
 	emptyValue := newValue(values, prefix, flat)
 	emptyValue.String("")
 
+	if !flat {
+		// This uses string concatenation in place of fmt.Sprintf as fmt.Sprintf has a much higher resource overhead
+		prefix = prefix + keySeparator + memberName
+	}
+
 	return &Array{
 		values:     values,
 		prefix:     prefix,
-		flat:       flat,
-		memberName: memberName,
 		emptyValue: emptyValue,
 	}
 }
@@ -63,10 +55,7 @@ func (a *Array) Value() Value {
 
 	// Query lists start a 1, so adjust the size first
 	a.size++
-	prefix := a.prefix
-	if !a.flat {
-		prefix = fmt.Sprintf("%s.%s", prefix, a.memberName)
-	}
 	// Lists can't have flat members
-	return newValue(a.values, fmt.Sprintf("%s.%d", prefix, a.size), false)
+	// This uses string concatenation in place of fmt.Sprintf as fmt.Sprintf has a much higher resource overhead
+	return newValue(a.values, a.prefix+keySeparator+strconv.FormatInt(int64(a.size), 10), false)
 }
diff --git a/aws/protocol/query/object.go b/aws/protocol/query/object.go
index 455b92515ca..305a8ace302 100644
--- a/aws/protocol/query/object.go
+++ b/aws/protocol/query/object.go
@@ -1,9 +1,6 @@
 package query
 
-import (
-	"fmt"
-	"net/url"
-)
+import "net/url"
 
 // Object represents the encoding of Query structures and unions. A Query
 // object is a representation of a mapping of string keys to arbitrary
@@ -56,14 +53,16 @@ func (o *Object) FlatKey(name string) Value {
 
 func (o *Object) key(name string, flatValue bool) Value {
 	if o.prefix != "" {
-		return newValue(o.values, fmt.Sprintf("%s.%s", o.prefix, name), flatValue)
+		// This uses string concatenation in place of fmt.Sprintf as fmt.Sprintf has a much higher resource overhead
+		return newValue(o.values, o.prefix+keySeparator+name, flatValue)
 	}
 	return newValue(o.values, name, flatValue)
 }
 
 func (o *Object) keyWithValues(name string, flatValue bool) Value {
 	if o.prefix != "" {
-		return newAppendValue(o.values, fmt.Sprintf("%s.%s", o.prefix, name), flatValue)
+		// This uses string concatenation in place of fmt.Sprintf as fmt.Sprintf has a much higher resource overhead
+		return newAppendValue(o.values, o.prefix+keySeparator+name, flatValue)
 	}
 	return newAppendValue(o.values, name, flatValue)
 }
diff --git a/aws/protocol/query/value.go b/aws/protocol/query/value.go
index a9251521f12..8063c592ddd 100644
--- a/aws/protocol/query/value.go
+++ b/aws/protocol/query/value.go
@@ -7,6 +7,8 @@ import (
 	"github.com/aws/smithy-go/encoding/httpbinding"
 )
 
+const keySeparator = "."
+
 // Value represents a Query Value type.
 type Value struct {
 	// The query values to add the value to.
diff --git a/aws/protocol/query/value_test.go b/aws/protocol/query/value_test.go
new file mode 100644
index 00000000000..f8336f39d46
--- /dev/null
+++ b/aws/protocol/query/value_test.go
@@ -0,0 +1,64 @@
+package query
+
+import (
+	"fmt"
+	"strconv"
+	"testing"
+)
+
+var output string
+
+func Benchmark_sprintf_strings(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		output = fmt.Sprintf("%s.%s", "foo", "bar")
+	}
+}
+
+func Benchmark_concat_strings(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		output = "foo" + keySeparator + "bar"
+	}
+}
+
+func Benchmark_int_formatting(b *testing.B) {
+	benchmarkFuncs := []struct {
+		name      string
+		formatter func(val int32)
+	}{
+		{
+			name: "array - sprintf", formatter: func(val int32) {
+				output = fmt.Sprintf("%s.%d", "foo", val)
+			},
+		},
+		{
+			name: "array - concat strconv", formatter: func(val int32) {
+				output = "foo" + keySeparator + strconv.FormatInt(int64(val), 10)
+			},
+		},
+		{
+			name: "map - sprintf", formatter: func(val int32) {
+				output = fmt.Sprintf("%s.%d.%s", "foo", val, "bar")
+				output = fmt.Sprintf("%s.%d.%s", "foo", val, "bar")
+			},
+		},
+		{
+			name: "map - concat strconv", formatter: func(val int32) {
+				valString := strconv.FormatInt(int64(val), 10)
+				output = "foo" + keySeparator + valString + keySeparator + "bar"
+				output = "foo" + keySeparator + valString + keySeparator + "bar"
+			},
+		},
+	}
+
+	sizesToTest := []int32{1, 10, 100, 250, 500, 1000}
+
+	for _, bm := range benchmarkFuncs {
+		for _, size := range sizesToTest {
+			b.Run(fmt.Sprintf("%s with %d size", bm.name, size), func(b *testing.B) {
+				for i := 0; i < b.N; i++ {
+					bm.formatter(size)
+				}
+			})
+		}
+	}
+}