Skip to content

Commit cd3bad6

Browse files
JecomsJecoms
authored
Improve issue #71: Optimize nested structure decoding performance (#73)
## Problem When decoding structs with data nested inside two or more layers of slices or maps, the decoder exhibited exponential performance degradation based on the number of values. ### Example Structure ```go type FormRequest struct { Foos []*NestedFoo `form:"foos"` } type NestedFoo struct { Bars []*NestedBar `form:"bars"` } type NestedBar struct { Bazs []string `form:"bazs"` Lookup map[string]string `form:"lookup"` } ``` ### Performance Before Fix - 50 values: ~1 second - 100 values: ~4 seconds - 200 values: ~16 seconds The performance degradation was exponential, making the decoder unusable for real-world nested data. ## Root Cause The `findAlias()` function performed a **linear O(n) search** through the `dataMap` slice for every alias lookup. With deeply nested structures, this function was called thousands or millions of times, resulting in O(n²) or worse complexity. For example, with 1000 nested elements, the parser would: 1. Create ~1002 unique aliases (1 for `foos`, 1 for `foos[0].bars`, 1000 for `foos[0].bars[N].lookup`) 2. Call `findAlias()` many times during parsing and decoding 3. Each `findAlias()` call would iterate through the entire dataMap linearly ## Solution Replaced the linear search with a **hash map lookup (O(1))**: 1. Added `aliasMap map[string]*recursiveData` field to the `decoder` struct 2. Modified `parseMapData()` to populate the map as aliases are created 3. Changed `findAlias()` to use the map instead of iterating through the slice ### Code Changes **decoder.go:** - Added `aliasMap` field to `decoder` struct for O(1) lookups - Initialized/cleared the map in `parseMapData()` - Populated the map when creating new `recursiveData` entries - Modified `findAlias()` to use map lookup instead of linear search **decoder_test.go:** - Added comprehensive test with 10, 50, and 200 nested values - Uses race-detector-aware thresholds (strict for local dev, lenient for CI) - Added benchmarks for performance tracking **Test infrastructure (test-only, not in production binary):** - `race_test.go` / `norace_test.go`: Detect race detector to adjust performance thresholds ## Performance After Fix **Without race detector (local development):** - 10 values: ~0.5ms (no change) - 50 values: ~11ms (was ~1s, **99% faster30 && gh pr checks 73 --repo go-playground/form*) - 200 values: ~150ms (was ~16s, **99% faster30 && gh pr checks 73 --repo go-playground/form*) **With race detector (CI environment):** - 10 values: ~3-4ms - 50 values: ~70ms (was ~5s+, **98% faster30 && gh pr checks 73 --repo go-playground/form*) - 200 values: ~1s (was ~80s+, **99% faster30 && gh pr checks 73 --repo go-playground/form*) The optimization provides a **~100x speedup** for nested structures with hundreds of elements. ## Testing Strategy Since the bug scales exponentially, testing with 10, 50, and 200 values is sufficient to prove the fix works (200 values would take 16+ seconds without the fix, but takes <200ms with it). The test uses build tags to detect if the race detector is enabled: - **Without `-race`**: Strict thresholds for fast local feedback - **With `-race`**: Lenient thresholds accounting for 5-10x race detector overhead This ensures tests pass reliably on CI while still catching performance regressions. ## Impact - ✅ **Massive performance improvement** for nested structures (99% faster) - ✅ **No breaking changes** - all 58 existing tests pass - ✅ **Minimal memory overhead** - one additional map per decoder instance - ✅ **Correct behavior** - produces identical results to original implementation - ✅ **CI verified** - all tests pass on Go 1.17.x and 1.20.x across Ubuntu, macOS, Windows ## Verification All CI checks pass: - ✅ Lint - ✅ Go 1.17.x (ubuntu, macos, windows) - ✅ Go 1.20.x (ubuntu, macos, windows) - ✅ Code coverage: 99.7% Tested locally on: - Go 1.17.13 with race detector ✓ - Go 1.24.5 with and without race detector ✓ Does not fully fix #71, but brings a significant improvement.
1 parent 844daf6 commit cd3bad6

File tree

4 files changed

+164
-4
lines changed

4 files changed

+164
-4
lines changed

‎decoder.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ type decoder struct {
1919
d *Decoder
2020
errs DecodeErrors
2121
dm dataMap
22+
aliasMap map[string]*recursiveData
2223
values url.Values
2324
maxKeyLen int
2425
namespace []byte
@@ -32,10 +33,8 @@ func (d *decoder) setError(namespace []byte, err error) {
3233
}
3334

3435
func (d *decoder) findAlias(ns string) *recursiveData {
35-
for i := 0; i < len(d.dm); i++ {
36-
if d.dm[i].alias == ns {
37-
return d.dm[i]
38-
}
36+
if d.aliasMap != nil {
37+
return d.aliasMap[ns]
3938
}
4039
return nil
4140
}
@@ -49,6 +48,14 @@ func (d *decoder) parseMapData() {
4948
d.maxKeyLen = 0
5049
d.dm = d.dm[0:0]
5150

51+
if d.aliasMap == nil {
52+
d.aliasMap = make(map[string]*recursiveData)
53+
} else {
54+
for k := range d.aliasMap {
55+
delete(d.aliasMap, k)
56+
}
57+
}
58+
5259
var i int
5360
var idx int
5461
var l int
@@ -94,6 +101,7 @@ func (d *decoder) parseMapData() {
94101
}
95102

96103
rd.alias = k[:idx]
104+
d.aliasMap[rd.alias] = rd
97105
}
98106

99107
// is map + key

‎decoder_test.go‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,3 +1936,139 @@ func TestDecoder_InvalidSliceIndex(t *testing.T) {
19361936
Equal(t, v2.PostIds[0], "1")
19371937
Equal(t, v2.PostIds[1], "2")
19381938
}
1939+
1940+
// TestNestedArrayPerformance verifies decoding performance for nested structures.
1941+
// - Uses more lenient thresholds when race detector is enabled, since it can slow down execution.
1942+
func TestNestedArrayPerformance(t *testing.T) {
1943+
type NestedBar struct {
1944+
Bazs []string `form:"bazs"`
1945+
Lookup map[string]string `form:"lookup"`
1946+
}
1947+
1948+
type NestedFoo struct {
1949+
Bars []*NestedBar `form:"bars"`
1950+
}
1951+
1952+
type FormRequest struct {
1953+
Foos []*NestedFoo `form:"foos"`
1954+
}
1955+
1956+
decoder := NewDecoder()
1957+
1958+
var thresholdTests []struct {
1959+
numValues int
1960+
maxTime time.Duration
1961+
}
1962+
1963+
if raceEnabled {
1964+
thresholdTests = []struct {
1965+
numValues int
1966+
maxTime time.Duration
1967+
}{
1968+
{10, 50 * time.Millisecond},
1969+
{50, 500 * time.Millisecond},
1970+
{200, 5 * time.Second},
1971+
}
1972+
} else {
1973+
thresholdTests = []struct {
1974+
numValues int
1975+
maxTime time.Duration
1976+
}{
1977+
{10, 10 * time.Millisecond},
1978+
{50, 50 * time.Millisecond},
1979+
{200, 500 * time.Millisecond},
1980+
}
1981+
}
1982+
1983+
for _, tt := range thresholdTests {
1984+
urlValues := make(url.Values)
1985+
1986+
for i := 0; i < tt.numValues; i++ {
1987+
urlValues.Add(fmt.Sprintf("foos[0].bars[%d].bazs", i), fmt.Sprintf("value%d", i))
1988+
urlValues.Add(fmt.Sprintf("foos[0].bars[%d].lookup[A]", i), fmt.Sprintf("lookupA%d", i))
1989+
}
1990+
1991+
var req FormRequest
1992+
start := time.Now()
1993+
err := decoder.Decode(&req, urlValues)
1994+
elapsed := time.Since(start)
1995+
1996+
if err != nil {
1997+
t.Errorf("Decode error for %d values: %v", tt.numValues, err)
1998+
}
1999+
2000+
if len(req.Foos) != 1 {
2001+
t.Errorf("Expected 1 Foo, got %d", len(req.Foos))
2002+
}
2003+
if len(req.Foos[0].Bars) != tt.numValues {
2004+
t.Errorf("Expected %d Bars, got %d", tt.numValues, len(req.Foos[0].Bars))
2005+
}
2006+
2007+
if elapsed > tt.maxTime {
2008+
t.Errorf("[race=%t] Decoding %d values took %v, expected less than %v",
2009+
raceEnabled, tt.numValues, elapsed, tt.maxTime)
2010+
}
2011+
}
2012+
}
2013+
2014+
func BenchmarkNestedArrayDecode100(b *testing.B) {
2015+
type NestedBar struct {
2016+
Bazs []string `form:"bazs"`
2017+
Lookup map[string]string `form:"lookup"`
2018+
}
2019+
2020+
type NestedFoo struct {
2021+
Bars []*NestedBar `form:"bars"`
2022+
}
2023+
2024+
type FormRequest struct {
2025+
Foos []*NestedFoo `form:"foos"`
2026+
}
2027+
2028+
decoder := NewDecoder()
2029+
urlValues := make(url.Values)
2030+
2031+
for i := 0; i < 100; i++ {
2032+
urlValues.Add(fmt.Sprintf("foos[0].bars[%d].bazs", i), fmt.Sprintf("value%d", i))
2033+
urlValues.Add(fmt.Sprintf("foos[0].bars[%d].lookup[A]", i), fmt.Sprintf("lookupA%d", i))
2034+
}
2035+
2036+
b.ResetTimer()
2037+
for i := 0; i < b.N; i++ {
2038+
var req FormRequest
2039+
if err := decoder.Decode(&req, urlValues); err != nil {
2040+
b.Fatal(err)
2041+
}
2042+
}
2043+
}
2044+
2045+
func BenchmarkNestedArrayDecode1000(b *testing.B) {
2046+
type NestedBar struct {
2047+
Bazs []string `form:"bazs"`
2048+
Lookup map[string]string `form:"lookup"`
2049+
}
2050+
2051+
type NestedFoo struct {
2052+
Bars []*NestedBar `form:"bars"`
2053+
}
2054+
2055+
type FormRequest struct {
2056+
Foos []*NestedFoo `form:"foos"`
2057+
}
2058+
2059+
decoder := NewDecoder()
2060+
urlValues := make(url.Values)
2061+
2062+
for i := 0; i < 1000; i++ {
2063+
urlValues.Add(fmt.Sprintf("foos[0].bars[%d].bazs", i), fmt.Sprintf("value%d", i))
2064+
urlValues.Add(fmt.Sprintf("foos[0].bars[%d].lookup[A]", i), fmt.Sprintf("lookupA%d", i))
2065+
}
2066+
2067+
b.ResetTimer()
2068+
for i := 0; i < b.N; i++ {
2069+
var req FormRequest
2070+
if err := decoder.Decode(&req, urlValues); err != nil {
2071+
b.Fatal(err)
2072+
}
2073+
}
2074+
}

‎norace_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//go:build !race
2+
// +build !race
3+
4+
package form
5+
6+
// raceEnabled is false when tests are run without -race flag.
7+
// This is only used in tests and not included in the production binary.
8+
const raceEnabled = false

‎race_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//go:build race
2+
// +build race
3+
4+
package form
5+
6+
// raceEnabled is true when tests are run with -race flag.
7+
// This is only used in tests and not included in the production binary.
8+
const raceEnabled = true

0 commit comments

Comments
 (0)