Skip to content

Commit 1b71016

Browse files
committed
Further optimize RegionMap.MarshalJSON().
- Stop calling the encoder for values as well. - Use tests to ensure that regions always contain valid utf8. Allocations are now down 80% in FormatHandler.
1 parent 8ee9fb4 commit 1b71016

File tree

2 files changed

+39
-8
lines changed

2 files changed

+39
-8
lines changed

address.go

+6-8
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package address
66

77
import (
88
"bytes"
9-
"encoding/json"
109
"fmt"
1110
"regexp"
1211
"sort"
@@ -172,17 +171,16 @@ func (r RegionMap) MarshalJSON() ([]byte, error) {
172171
buf := &bytes.Buffer{}
173172
buf.Grow(r.Len() * 30)
174173
buf.WriteByte('{')
175-
encoder := json.NewEncoder(buf)
176174
for i, key := range r.keys {
177175
if i > 0 {
178176
buf.WriteByte(',')
179177
}
180-
// The key is assumed to be safe and need no escaping.
181-
// Not calling encoder.Encode on the key saves many allocations.
182-
buf.WriteString(`"` + key + `":`)
183-
if err := encoder.Encode(r.values[key]); err != nil {
184-
return nil, err
185-
}
178+
// A fully generic MarshalJSON would call encoder.Encode() to ensure both key and value
179+
// are escaped and contain valid utf8. Since all regions are defined in the package, they
180+
// are assumed to be valid, avoiding thousands of allocs when marshalling the format list.
181+
buf.WriteString(`"` + key + `":"`)
182+
buf.WriteString(r.values[key])
183+
buf.WriteString(`"`)
186184
}
187185
buf.WriteByte('}')
188186

address_test.go

+33
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"encoding/json"
88
"reflect"
99
"testing"
10+
"unicode/utf8"
1011

1112
"github.com/bojanz/address"
1213
)
@@ -339,6 +340,38 @@ func TestGetFormats(t *testing.T) {
339340
}
340341
}
341342

343+
func TestGetFormats_ValidRegionData(t *testing.T) {
344+
// Confirm that all regions contain valid utf8.
345+
// Avoids having the check at runtime, in RegionMap.MarshalJSON.
346+
formats := address.GetFormats()
347+
for countryCode, format := range formats {
348+
if format.Regions.Len() > 0 {
349+
keys := format.Regions.Keys()
350+
for _, key := range keys {
351+
value, _ := format.Regions.Get(key)
352+
if !utf8.ValidString(key) {
353+
t.Errorf("invalid key %v in %v regions", key, countryCode)
354+
}
355+
if !utf8.ValidString(value) {
356+
t.Errorf("invalid value %v for key %v in %v regions", value, key, countryCode)
357+
}
358+
}
359+
}
360+
if format.LocalRegions.Len() > 0 {
361+
keys := format.LocalRegions.Keys()
362+
for _, key := range keys {
363+
value, _ := format.LocalRegions.Get(key)
364+
if !utf8.ValidString(key) {
365+
t.Errorf("invalid key %v in %v local regions", key, countryCode)
366+
}
367+
if !utf8.ValidString(value) {
368+
t.Errorf("invalid value %v for key %v in %v regions", value, key, countryCode)
369+
}
370+
}
371+
}
372+
}
373+
}
374+
342375
func contains(a []string, x string) bool {
343376
for _, v := range a {
344377
if v == x {

0 commit comments

Comments
 (0)