From 47e39af2cf603117f7febb64acdac8f3d6bfbb24 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Sat, 14 Feb 2026 16:33:15 +0100 Subject: [PATCH] tpl/collections: Speed up where and sort performance MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit ```bash goos: darwin goarch: arm64 pkg: github.com/gohugoio/hugo/tpl/collections cpu: Apple M1 Pro │ master.bench │ fix-whereperfalloc.bench │ │ sec/op │ sec/op vs base │ WhereOps/eq-10 139.62µ ± 3% 66.04µ ± 2% -52.70% (p=0.002 n=6) WhereOps/ne-10 139.54µ ± 1% 65.68µ ± 2% -52.93% (p=0.002 n=6) WhereOps/like-10 179.8µ ± 3% 106.7µ ± 1% -40.66% (p=0.002 n=6) WhereSliceOfStructPointersWithMethod-10 350.7µ ± 1% 252.2µ ± 2% -28.09% (p=0.002 n=6) WhereAndSortPages/Where-10 120.93µ ± 1% 81.69µ ± 1% -32.45% (p=0.002 n=6) WhereAndSortPages/Sort-10 1.608m ± 1% 1.165m ± 1% -27.52% (p=0.002 n=6) geomean 117.2µ 149.4µ -40.03% ¹ │ master.bench │ fix-whereperfalloc.bench │ │ B/op │ B/op vs base │ WhereOps/eq-10 52.18Ki ± 0% 36.62Ki ± 0% -29.82% (p=0.002 n=6) WhereOps/ne-10 52.18Ki ± 0% 36.62Ki ± 0% -29.82% (p=0.002 n=6) WhereOps/like-10 52.20Ki ± 0% 36.63Ki ± 0% -29.83% (p=0.002 n=6) WhereSliceOfStructPointersWithMethod-10 64.52Ki ± 0% 64.59Ki ± 0% +0.12% (p=0.002 n=6) WhereAndSortPages/Where-10 21.05Ki ± 0% 21.11Ki ± 0% +0.30% (p=0.002 n=6) WhereAndSortPages/Sort-10 591.15Ki ± 0% 51.82Ki ± 0% -91.23% (p=0.002 n=6) geomean 7.690Ki 38.91Ki -44.13% ¹ │ master.bench │ fix-whereperfalloc.bench │ │ allocs/op │ allocs/op vs base │ WhereOps/eq-10 2.515k ± 0% 1.517k ± 0% -39.68% (p=0.002 n=6) WhereOps/ne-10 2.515k ± 0% 1.517k ± 0% -39.68% (p=0.002 n=6) WhereOps/like-10 2.515k ± 0% 1.517k ± 0% -39.68% (p=0.002 n=6) WhereSliceOfStructPointersWithMethod-10 4.503k ± 0% 4.504k ± 0% +0.02% (p=0.002 n=6) WhereAndSortPages/Where-10 1.029k ± 0% 1.030k ± 0% +0.10% (p=0.002 n=6) WhereAndSortPages/Sort-10 20.736k ± 0% 1.012k ± 0% -95.12% (p=0.002 n=6) geomean 358.5 1.594k -53.04% ¹ ``` --- .../collections_integration_test.go | 2 + tpl/collections/sort.go | 2 +- tpl/collections/where.go | 206 +++++++++++++++++- tpl/collections/where_test.go | 40 ---- tpl/compare/compare.go | 36 +-- 5 files changed, 229 insertions(+), 57 deletions(-) diff --git a/tpl/collections/collections_integration_test.go b/tpl/collections/collections_integration_test.go index ee003e049..750ea7cd8 100644 --- a/tpl/collections/collections_integration_test.go +++ b/tpl/collections/collections_integration_test.go @@ -588,6 +588,8 @@ All. seq := s.RegularPages() ns := s.TemplateStore.GetTemplateFuncsNamespace("collections").(*collections.Namespace) + b.ResetTimer() + b.Run("Where", func(b *testing.B) { for b.Loop() { v, err := ns.Where(context.Background(), seq, "Title", "ge", "Page0480") diff --git a/tpl/collections/sort.go b/tpl/collections/sort.go index 6d6a2ad56..15f6eb34a 100644 --- a/tpl/collections/sort.go +++ b/tpl/collections/sort.go @@ -89,7 +89,7 @@ func (ns *Namespace) Sort(ctx context.Context, l any, args ...any) (any, error) if !v.IsValid() { continue } - // Special handling of lower cased hmaps. + // Special handling of lower cased maps. if params, ok := v.Interface().(hmaps.Params); ok { v = reflect.ValueOf(params.GetNested(path[i+1:]...)) break diff --git a/tpl/collections/where.go b/tpl/collections/where.go index bc7dcd229..0cb84b8f5 100644 --- a/tpl/collections/where.go +++ b/tpl/collections/where.go @@ -393,16 +393,219 @@ func parseWhereArgs(args ...any) (mv reflect.Value, op string, err error) { return } +// elemResolver resolves a sub-element from a reflect.Value. +// Built once before the loop to avoid repeated type checks and reflect.ValueOf allocations. +type elemResolver func(reflect.Value) (reflect.Value, error) + +// newElemResolver returns a resolver optimized for the given element type and path. +// Returns nil if optimization isn't possible, in which case the caller should +// fall back to evaluateSubElem. +func (ns *Namespace) newElemResolver(ctxv reflect.Value, elemType reflect.Type, path []string) elemResolver { + if elemType.Kind() == reflect.Interface { + if len(path) != 1 { + return nil + } + return ns.newInterfaceMethodResolver(ctxv, elemType, path[0]) + } + + baseType := elemType + isPtr := baseType.Kind() == reflect.Pointer + if isPtr { + baseType = baseType.Elem() + } + + if baseType == reflect.TypeFor[hmaps.Params]() { + return func(v reflect.Value) (reflect.Value, error) { + if isPtr { + if v.IsNil() { + return zero, nil + } + v = v.Elem() + } + params := v.Interface().(hmaps.Params) + return reflect.ValueOf(params.GetNested(path...)), nil + } + } + + if len(path) != 1 { + return nil + } + name := path[0] + + // Check for method first, matching evaluateSubElem order. + ptrType := baseType + if !hreflect.IsInterfaceOrPointer(ptrType.Kind()) { + ptrType = reflect.PointerTo(baseType) + } + mt := hreflect.GetMethodByNameForType(ptrType, name) + if mt.Func.IsValid() { + return ns.newMethodResolver(ctxv, elemType, mt) + } + + switch baseType.Kind() { + case reflect.Map: + if baseType.Key().Kind() == reflect.String { + kv := reflect.ValueOf(name) + return func(v reflect.Value) (reflect.Value, error) { + if isPtr { + if v.IsNil() { + return zero, nil + } + v = v.Elem() + } + return v.MapIndex(kv), nil + } + } + case reflect.Struct: + ft, ok := baseType.FieldByName(name) + if ok { + if ft.PkgPath != "" && !ft.Anonymous { + return func(v reflect.Value) (reflect.Value, error) { + return zero, fmt.Errorf("%s is an unexported field of struct type %s", name, elemType) + } + } + idx := ft.Index + return func(v reflect.Value) (reflect.Value, error) { + if isPtr { + if v.IsNil() { + return zero, nil + } + v = v.Elem() + } + return v.FieldByIndex(idx), nil + } + } + } + + return nil +} + +func (ns *Namespace) newMethodResolver(ctxv reflect.Value, elemType reflect.Type, mt reflect.Method) elemResolver { + if mt.PkgPath != "" { + return func(v reflect.Value) (reflect.Value, error) { + return zero, fmt.Errorf("%s is an unexported method of type %s", mt.Name, elemType) + } + } + + numIn := mt.Type.NumIn() + maxNumIn := 1 + needsCtx := numIn > 1 && hreflect.IsContextType(mt.Type.In(1)) + if needsCtx { + maxNumIn = 2 + } + + switch { + case mt.Type.NumIn() > maxNumIn: + return nil + case mt.Type.NumOut() == 0: + return nil + case mt.Type.NumOut() > 2: + return nil + case mt.Type.NumOut() == 1 && mt.Type.Out(0).Implements(errorType): + return nil + case mt.Type.NumOut() == 2 && !mt.Type.Out(1).Implements(errorType): + return nil + } + + fn := mt.Func + hasErrOut := mt.Type.NumOut() == 2 + isPtr := elemType.Kind() == reflect.Pointer + + var callArgs []reflect.Value + if needsCtx { + callArgs = make([]reflect.Value, 2) + callArgs[1] = ctxv + } else { + callArgs = make([]reflect.Value, 1) + } + + return func(v reflect.Value) (reflect.Value, error) { + if isPtr && v.IsNil() { + return zero, nil + } + recv := v + if !isPtr && !hreflect.IsInterfaceOrPointer(recv.Kind()) && recv.CanAddr() { + recv = recv.Addr() + } + callArgs[0] = recv + res := fn.Call(callArgs) + if hasErrOut && !res[1].IsNil() { + return zero, res[1].Interface().(error) + } + return res[0], nil + } +} + +// newInterfaceMethodResolver returns a method resolver or nil if not possible, in which the caller should fall back to evaluateSubElem. +func (ns *Namespace) newInterfaceMethodResolver(ctxv reflect.Value, ifaceType reflect.Type, name string) elemResolver { + mt, ok := ifaceType.MethodByName(name) + if !ok { + return nil + } + + // For interface methods, Type does not include the receiver. + mType := mt.Type + numIn := mType.NumIn() + maxNumIn := 0 + needsCtx := numIn > 0 && hreflect.IsContextType(mType.In(0)) + if needsCtx { + maxNumIn = 1 + } + + switch { + case mType.NumIn() > maxNumIn: + return nil + case mType.NumOut() == 0: + return nil + case mType.NumOut() > 2: + return nil + case mType.NumOut() == 1 && mType.Out(0).Implements(errorType): + return nil + case mType.NumOut() == 2 && !mType.Out(1).Implements(errorType): + return nil + } + + index := mt.Index + hasErrOut := mType.NumOut() == 2 + + var callArgs []reflect.Value + if needsCtx { + callArgs = []reflect.Value{ctxv} + } + + return func(v reflect.Value) (reflect.Value, error) { + if v.IsNil() { + return zero, nil + } + res := v.Method(index).Call(callArgs) + if hasErrOut && !res[1].IsNil() { + return zero, res[1].Interface().(error) + } + return res[0], nil + } +} + // checkWhereArray handles the where-matching logic when the seqv value is an // Array or Slice. func (ns *Namespace) checkWhereArray(ctxv, seqv, kv, mv reflect.Value, path []string, op string) (any, error) { rv := reflect.MakeSlice(seqv.Type(), 0, 0) + var resolve elemResolver + if kv.Kind() == reflect.String && len(path) > 0 { + resolve = ns.newElemResolver(ctxv, seqv.Type().Elem(), path) + } + for i := range seqv.Len() { var vvv reflect.Value rvv := seqv.Index(i) - if kv.Kind() == reflect.String { + if resolve != nil { + var err error + vvv, err = resolve(rvv) + if err != nil { + return nil, err + } + } else if kv.Kind() == reflect.String { if params, ok := rvv.Interface().(hmaps.Params); ok { vvv = reflect.ValueOf(params.GetNested(path...)) } else { @@ -416,7 +619,6 @@ func (ns *Namespace) checkWhereArray(ctxv, seqv, kv, mv reflect.Value, path []st if i < len(path)-1 && vvv.IsValid() { if params, ok := vvv.Interface().(hmaps.Params); ok { - // The current path element is the map itself, .Params. vvv = reflect.ValueOf(params.GetNested(path[i+1:]...)) break } diff --git a/tpl/collections/where_test.go b/tpl/collections/where_test.go index 1822e9435..b4fd6e6a8 100644 --- a/tpl/collections/where_test.go +++ b/tpl/collections/where_test.go @@ -904,46 +904,6 @@ func BenchmarkWhereOps(b *testing.B) { }) } -func BenchmarkWhereMap(b *testing.B) { - ns := newNs() - seqString := map[string]string{} - seqAny := map[string]any{} - seqInt := map[string]int{} - - for i := range 1000 { - seqString[fmt.Sprintf("key%d", i)] = "value" - seqAny[fmt.Sprintf("key%d", i)] = "value" - seqInt[fmt.Sprintf("key%d", i)] = i - } - - b.Run("String", func(b *testing.B) { - for b.Loop() { - _, err := ns.Where(context.Background(), seqString, "key", "eq", "value") - if err != nil { - b.Fatal(err) - } - } - }) - - b.Run("Int", func(b *testing.B) { - for b.Loop() { - _, err := ns.Where(context.Background(), seqAny, "key", "eq", 42) - if err != nil { - b.Fatal(err) - } - } - }) - - b.Run("Any", func(b *testing.B) { - for b.Loop() { - _, err := ns.Where(context.Background(), seqAny, "key", "eq", "value") - if err != nil { - b.Fatal(err) - } - } - }) -} - func BenchmarkWhereSliceOfStructPointersWithMethod(b *testing.B) { // TstRv2 ns := newNs() diff --git a/tpl/compare/compare.go b/tpl/compare/compare.go index c5eaa87f5..efac28931 100644 --- a/tpl/compare/compare.go +++ b/tpl/compare/compare.go @@ -275,6 +275,13 @@ func (ns *Namespace) compareGetWithCollator(collator *langs.Collator, a any, b a } } + // Fast path: both values are plain strings. + if as, aOk := a.(string); aOk { + if bs, bOk := b.(string); bOk { + return ns.compareTwoStrings(collator, as, bs) + } + } + var left, right float64 var leftStr, rightStr *string av := reflect.ValueOf(a) @@ -350,33 +357,34 @@ func (ns *Namespace) compareGetWithCollator(collator *langs.Collator, a any, b a } } - if (ns.caseInsensitive || collator != nil) && leftStr != nil && rightStr != nil { + if leftStr != nil && rightStr != nil { + return ns.compareTwoStrings(collator, *leftStr, *rightStr) + } + + return left, right +} + +func (ns *Namespace) compareTwoStrings(collator *langs.Collator, a, b string) (float64, float64) { + if ns.caseInsensitive || collator != nil { var c int if collator != nil { - c = collator.CompareStrings(*leftStr, *rightStr) + c = collator.CompareStrings(a, b) } else { - c = compare.Strings(*leftStr, *rightStr) + c = compare.Strings(a, b) } if c < 0 { return 0, 1 } else if c > 0 { return 1, 0 - } else { - return 0, 0 } + return 0, 0 } - - switch { - case leftStr == nil || rightStr == nil: - case *leftStr < *rightStr: + if a < b { return 0, 1 - case *leftStr > *rightStr: + } else if a > b { return 1, 0 - default: - return 0, 0 } - - return left, right + return 0, 0 } func (ns *Namespace) toTimeUnix(v reflect.Value) int64 { -- 2.39.5