diff --git a/decoder.go b/decoder.go index e212422..7c6f704 100644 --- a/decoder.go +++ b/decoder.go @@ -19,6 +19,7 @@ type decoder struct { d *Decoder errs DecodeErrors dm dataMap + aliasMap map[string]*recursiveData values url.Values maxKeyLen int namespace []byte @@ -32,10 +33,8 @@ func (d *decoder) setError(namespace []byte, err error) { } func (d *decoder) findAlias(ns string) *recursiveData { - for i := 0; i < len(d.dm); i++ { - if d.dm[i].alias == ns { - return d.dm[i] - } + if d.aliasMap != nil { + return d.aliasMap[ns] } return nil } @@ -49,6 +48,14 @@ func (d *decoder) parseMapData() { d.maxKeyLen = 0 d.dm = d.dm[0:0] + if d.aliasMap == nil { + d.aliasMap = make(map[string]*recursiveData) + } else { + for k := range d.aliasMap { + delete(d.aliasMap, k) + } + } + var i int var idx int var l int @@ -94,6 +101,7 @@ func (d *decoder) parseMapData() { } rd.alias = k[:idx] + d.aliasMap[rd.alias] = rd } // is map + key diff --git a/decoder_test.go b/decoder_test.go index 99897ed..40d583f 100644 --- a/decoder_test.go +++ b/decoder_test.go @@ -1936,3 +1936,151 @@ func TestDecoder_InvalidSliceIndex(t *testing.T) { Equal(t, v2.PostIds[0], "1") Equal(t, v2.PostIds[1], "2") } + +// Issue #71: Nested structure decoding performance +// https://github.com/go-playground/form/issues/71 +func TestIssue71NestedPerformance(t *testing.T) { + type NestedBar struct { + Bazs []string `form:"bazs"` + Lookup map[string]string `form:"lookup"` + } + + type NestedFoo struct { + Bars []*NestedBar `form:"bars"` + } + + type FormRequest struct { + Foos []*NestedFoo `form:"foos"` + } + + decoder := NewDecoder() + + // Adjust thresholds based on race detector + // Race detector adds 5-10x overhead, especially on older Go versions + // Using smaller counts (10, 50, 200) since we know the bug scales exponentially. + // Without the fix, even 200 values would take 10+ seconds. + var thresholds []struct { + numValues int + maxTime time.Duration + } + + if raceEnabled { + // Lenient thresholds for race detector mode (CI) + t.Log("Using lenient thresholds (race detector enabled)") + thresholds = []struct { + numValues int + maxTime time.Duration + }{ + {10, 50 * time.Millisecond}, + {50, 500 * time.Millisecond}, // Without fix: ~5s, with fix: ~50-100ms + {200, 5 * time.Second}, // Without fix: ~80s+, with fix: ~500ms-2s + } + } else { + // Strict thresholds for normal mode (local dev) + t.Log("Using strict thresholds (race detector disabled)") + thresholds = []struct { + numValues int + maxTime time.Duration + }{ + {10, 10 * time.Millisecond}, + {50, 50 * time.Millisecond}, // Without fix: ~1s, with fix: ~5-10ms + {200, 500 * time.Millisecond}, // Without fix: ~16s, with fix: ~50-100ms + } + } + + for _, tt := range thresholds { + urlValues := make(url.Values) + + // Generate test data with nested structure + for i := 0; i < tt.numValues; i++ { + urlValues.Add(fmt.Sprintf("foos[0].bars[%d].bazs", i), fmt.Sprintf("value%d", i)) + urlValues.Add(fmt.Sprintf("foos[0].bars[%d].lookup[A]", i), fmt.Sprintf("lookupA%d", i)) + } + + var req FormRequest + start := time.Now() + err := decoder.Decode(&req, urlValues) + elapsed := time.Since(start) + + if err != nil { + t.Errorf("Decode error for %d values: %v", tt.numValues, err) + } + + // Verify correct decoding + if len(req.Foos) != 1 { + t.Errorf("Expected 1 Foo, got %d", len(req.Foos)) + } + if len(req.Foos[0].Bars) != tt.numValues { + t.Errorf("Expected %d Bars, got %d", tt.numValues, len(req.Foos[0].Bars)) + } + + t.Logf("%6d decoded values took: %v", tt.numValues, elapsed) + + if elapsed > tt.maxTime { + t.Errorf("Decoding %d values took %v, expected less than %v (performance regression?)", + tt.numValues, elapsed, tt.maxTime) + } + } +} + +func BenchmarkIssue71Nested100(b *testing.B) { + type NestedBar struct { + Bazs []string `form:"bazs"` + Lookup map[string]string `form:"lookup"` + } + + type NestedFoo struct { + Bars []*NestedBar `form:"bars"` + } + + type FormRequest struct { + Foos []*NestedFoo `form:"foos"` + } + + decoder := NewDecoder() + urlValues := make(url.Values) + + for i := 0; i < 100; i++ { + urlValues.Add(fmt.Sprintf("foos[0].bars[%d].bazs", i), fmt.Sprintf("value%d", i)) + urlValues.Add(fmt.Sprintf("foos[0].bars[%d].lookup[A]", i), fmt.Sprintf("lookupA%d", i)) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + var req FormRequest + if err := decoder.Decode(&req, urlValues); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkIssue71Nested1000(b *testing.B) { + type NestedBar struct { + Bazs []string `form:"bazs"` + Lookup map[string]string `form:"lookup"` + } + + type NestedFoo struct { + Bars []*NestedBar `form:"bars"` + } + + type FormRequest struct { + Foos []*NestedFoo `form:"foos"` + } + + decoder := NewDecoder() + urlValues := make(url.Values) + + for i := 0; i < 1000; i++ { + urlValues.Add(fmt.Sprintf("foos[0].bars[%d].bazs", i), fmt.Sprintf("value%d", i)) + urlValues.Add(fmt.Sprintf("foos[0].bars[%d].lookup[A]", i), fmt.Sprintf("lookupA%d", i)) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + var req FormRequest + if err := decoder.Decode(&req, urlValues); err != nil { + b.Fatal(err) + } + } +} diff --git a/norace_test.go b/norace_test.go new file mode 100644 index 0000000..1f442b2 --- /dev/null +++ b/norace_test.go @@ -0,0 +1,8 @@ +//go:build !race +// +build !race + +package form + +// raceEnabled is false when tests are run without -race flag. +// This is only used in tests and not included in the production binary. +const raceEnabled = false diff --git a/race_test.go b/race_test.go new file mode 100644 index 0000000..b7c73fb --- /dev/null +++ b/race_test.go @@ -0,0 +1,8 @@ +//go:build race +// +build race + +package form + +// raceEnabled is true when tests are run with -race flag. +// This is only used in tests and not included in the production binary. +const raceEnabled = true