Skip to content

Commit

Permalink
Optimize canonicalizeContentType
Browse files Browse the repository at this point in the history
For context, this function is run on every request within ServeHTTP, and
the mime parsing and now lowercasing, is a marginal, but non trivial
amount of memory allocations.

The expensive bit is hitting the mime.FormatMediaType path, when we
already have a canonical form.

This removes the most expensive part over arguable the most common cases
where there are no additional parameters on the Content-Type.

```
$ go test -v -bench '^BenchmarkCanonicalizeContentType$' -run '^$' .
goos: darwin
goarch: arm64
pkg: github.com/bufbuild/connect-go
BenchmarkCanonicalizeContentType
BenchmarkCanonicalizeContentType/simple
BenchmarkCanonicalizeContentType/simple-10         	 7160896	       157.4 ns/op	      48 B/op	       1 allocs/op
BenchmarkCanonicalizeContentType/with_charset
BenchmarkCanonicalizeContentType/with_charset-10   	 1780041	       674.4 ns/op	     424 B/op	       6 allocs/op
BenchmarkCanonicalizeContentType/with_other_param
BenchmarkCanonicalizeContentType/with_other_param-10         	 2029819	       592.6 ns/op	     424 B/op	       6 allocs/op
PASS
ok  	github.com/bufbuild/connect-go	5.129s
```
  • Loading branch information
mattrobenolt committed Jan 26, 2023
1 parent cb460f1 commit 3810312
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
4 changes: 4 additions & 0 deletions protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ func canonicalizeContentType(ct string) string {
return ct
}

if len(params) == 0 {
return base
}

// According to RFC 9110 Section 8.3.2, the charset parameter value should be treated as case-insensitive.
// mime.FormatMediaType canonicalizes parameter names, but not parameter values,
// because the case sensitivity of a parameter value depends on its semantics.
Expand Down
24 changes: 24 additions & 0 deletions protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestCanonicalizeContentType(t *testing.T) {
}{
{name: "charset param should be treated as lowercase", arg: "application/json; charset=UTF-8", want: "application/json; charset=utf-8"},
{name: "non charset param should not be changed", arg: "multipart/form-data; boundary=fooBar", want: "multipart/form-data; boundary=fooBar"},
{name: "no parameters should not have base normalized", arg: "APPLICATION/json; ", want: "application/json"},
}
for _, tt := range tests {
tt := tt
Expand All @@ -38,3 +39,26 @@ func TestCanonicalizeContentType(t *testing.T) {
})
}
}

func BenchmarkCanonicalizeContentType(b *testing.B) {
b.Run("simple", func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = canonicalizeContentType("application/json")
}
b.ReportAllocs()
})

b.Run("with charset", func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = canonicalizeContentType("application/json; charset=utf-8")
}
b.ReportAllocs()
})

b.Run("with other param", func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = canonicalizeContentType("application/json; foo=utf-8")
}
b.ReportAllocs()
})
}

0 comments on commit 3810312

Please sign in to comment.