Skip to content

Commit

Permalink
Minimize allocations parsing Content-Type (#444)
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 -bench '^BenchmarkCanonicalizeContentType$' -run '^$' .
goos: darwin
goarch: arm64
pkg: github.com/bufbuild/connect-go
BenchmarkCanonicalizeContentType/simple-10         	92344741	        12.85 ns/op	       0 B/op	       0 allocs/op
BenchmarkCanonicalizeContentType/with_charset-10   	 1744219	       693.8 ns/op	     424 B/op	       6 allocs/op
BenchmarkCanonicalizeContentType/with_other_param-10         	 1969113	       614.4 ns/op	     424 B/op	       6 allocs/op
PASS
ok  	github.com/bufbuild/connect-go	5.800s
```

---------

Co-authored-by: Akshay Shah <akshay@akshayshah.org>
  • Loading branch information
mattrobenolt and akshayjshah authored Jan 27, 2023
1 parent 8c241dc commit 1dc7f30
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 5 deletions.
32 changes: 27 additions & 5 deletions protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,12 +315,35 @@ func flushResponseWriter(w http.ResponseWriter) {
}
}

func canonicalizeContentType(ct string) string {
base, params, err := mime.ParseMediaType(ct)
if err != nil {
return ct
func canonicalizeContentType(contentType string) string {
// Typically, clients send Content-Type in canonical form, without
// parameters. In those cases, we'd like to avoid parsing and
// canonicalization overhead.
//
// See https://www.rfc-editor.org/rfc/rfc2045.html#section-5.1 for a full
// grammar.
var slashes int
for _, r := range contentType {
switch {
case r >= 'a' && r <= 'z':
case r == '.' || r == '+' || r == '-':
case r == '/':
slashes++
default:
return canonicalizeContentTypeSlow(contentType)
}
}
if slashes == 1 {
return contentType
}
return canonicalizeContentTypeSlow(contentType)
}

func canonicalizeContentTypeSlow(contentType string) string {
base, params, err := mime.ParseMediaType(contentType)
if err != nil {
return contentType
}
// 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 All @@ -329,6 +352,5 @@ func canonicalizeContentType(ct string) string {
if charset, ok := params["charset"]; ok {
params["charset"] = strings.ToLower(charset)
}

return mime.FormatMediaType(base, params)
}
25 changes: 25 additions & 0 deletions protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ func TestCanonicalizeContentType(t *testing.T) {
arg string
want string
}{
{name: "uppercase should be normalized", arg: "APPLICATION/json", want: "application/json"},
{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 be normalized", arg: "APPLICATION/json; ", want: "application/json"},
}
for _, tt := range tests {
tt := tt
Expand All @@ -38,3 +40,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 1dc7f30

Please sign in to comment.