diff --git a/common/hreflect/helpers.go b/common/hreflect/helpers.go index beab182bb..7087a9677 100644 --- a/common/hreflect/helpers.go +++ b/common/hreflect/helpers.go @@ -19,6 +19,7 @@ package hreflect import ( "context" "reflect" + "sync" "github.com/gohugoio/hugo/common/types" ) @@ -115,6 +116,58 @@ func IsTruthfulValue(val reflect.Value) (truth bool) { return } +type methodKey struct { + typ reflect.Type + name string +} + +type methods struct { + sync.RWMutex + cache map[methodKey]int +} + +var methodCache = &methods{cache: make(map[methodKey]int)} + +// GetMethodByName is the samve as reflect.Value.MethodByName, but it caches the +// type lookup. +func GetMethodByName(v reflect.Value, name string) reflect.Value { + index := GetMethodIndexByName(v.Type(), name) + + if index == -1 { + return reflect.Value{} + } + + return v.Method(index) +} + +// GetMethodIndexByName returns the index of the method with the given name, or +// -1 if no such method exists. +func GetMethodIndexByName(tp reflect.Type, name string) int { + k := methodKey{tp, name} + methodCache.RLock() + index, found := methodCache.cache[k] + methodCache.RUnlock() + if found { + return index + } + + methodCache.Lock() + defer methodCache.Unlock() + + m, ok := tp.MethodByName(name) + index = m.Index + if !ok { + index = -1 + } + methodCache.cache[k] = index + + if !ok { + return -1 + } + + return m.Index +} + // Based on: https://github.com/golang/go/blob/178a2c42254166cffed1b25fb1d3c7a5727cada6/src/text/template/exec.go#L931 func indirectInterface(v reflect.Value) reflect.Value { if v.Kind() != reflect.Interface { diff --git a/common/hreflect/helpers_test.go b/common/hreflect/helpers_test.go index 480ccb27a..d16b9b9b3 100644 --- a/common/hreflect/helpers_test.go +++ b/common/hreflect/helpers_test.go @@ -30,6 +30,16 @@ func TestIsTruthful(t *testing.T) { c.Assert(IsTruthful(time.Time{}), qt.Equals, false) } +func TestGetMethodByName(t *testing.T) { + c := qt.New(t) + v := reflect.ValueOf(&testStruct{}) + tp := v.Type() + + c.Assert(GetMethodIndexByName(tp, "Method1"), qt.Equals, 0) + c.Assert(GetMethodIndexByName(tp, "Method3"), qt.Equals, 2) + c.Assert(GetMethodIndexByName(tp, "Foo"), qt.Equals, -1) +} + func BenchmarkIsTruthFul(b *testing.B) { v := reflect.ValueOf("Hugo") @@ -40,3 +50,37 @@ func BenchmarkIsTruthFul(b *testing.B) { } } } + +type testStruct struct{} + +func (t *testStruct) Method1() string { + return "Hugo" +} + +func (t *testStruct) Method2() string { + return "Hugo" +} + +func (t *testStruct) Method3() string { + return "Hugo" +} + +func (t *testStruct) Method4() string { + return "Hugo" +} + +func (t *testStruct) Method5() string { + return "Hugo" +} + +func BenchmarkGetMethodByName(b *testing.B) { + v := reflect.ValueOf(&testStruct{}) + methods := []string{"Method1", "Method2", "Method3", "Method4", "Method5"} + + b.ResetTimer() + for i := 0; i < b.N; i++ { + for _, method := range methods { + _ = GetMethodByName(v, method) + } + } +} diff --git a/langs/i18n/i18n.go b/langs/i18n/i18n.go index e45a16822..64340c857 100644 --- a/langs/i18n/i18n.go +++ b/langs/i18n/i18n.go @@ -160,7 +160,7 @@ func getPluralCount(v interface{}) interface{} { if f.IsValid() { return toPluralCountValue(f.Interface()) } - m := vv.MethodByName(countFieldName) + m := hreflect.GetMethodByName(vv, countFieldName) if m.IsValid() && m.Type().NumIn() == 0 && m.Type().NumOut() == 1 { c := m.Call(nil) return toPluralCountValue(c[0].Interface()) @@ -169,7 +169,6 @@ func getPluralCount(v interface{}) interface{} { } return toPluralCountValue(v) - } // go-i18n expects floats to be represented by string. diff --git a/resources/page/pagegroup.go b/resources/page/pagegroup.go index e07efa7ca..4c0adb68b 100644 --- a/resources/page/pagegroup.go +++ b/resources/page/pagegroup.go @@ -24,6 +24,7 @@ import ( "github.com/spf13/cast" "github.com/gohugoio/hugo/common/collections" + "github.com/gohugoio/hugo/common/hreflect" "github.com/gohugoio/hugo/compare" "github.com/gohugoio/hugo/resources/resource" @@ -112,8 +113,9 @@ func (p Pages) GroupBy(key string, order ...string) (PagesGroup, error) { } var ft interface{} - m, ok := pagePtrType.MethodByName(key) - if ok { + index := hreflect.GetMethodIndexByName(pagePtrType, key) + if index != -1 { + m := pagePtrType.Method(index) if m.Type.NumOut() == 0 || m.Type.NumOut() > 2 { return nil, errors.New(key + " is a Page method but you can't use it with GroupBy") } @@ -125,6 +127,7 @@ func (p Pages) GroupBy(key string, order ...string) (PagesGroup, error) { } ft = m } else { + var ok bool ft, ok = pagePtrType.Elem().FieldByName(key) if !ok { return nil, errors.New(key + " is neither a field nor a method of Page") @@ -146,7 +149,7 @@ func (p Pages) GroupBy(key string, order ...string) (PagesGroup, error) { case reflect.StructField: fv = ppv.Elem().FieldByName(key) case reflect.Method: - fv = ppv.MethodByName(key).Call([]reflect.Value{})[0] + fv = hreflect.GetMethodByName(ppv, key).Call([]reflect.Value{})[0] } if !fv.IsValid() { continue diff --git a/resources/postpub/postpub.go b/resources/postpub/postpub.go index c11dda577..f1012b7da 100644 --- a/resources/postpub/postpub.go +++ b/resources/postpub/postpub.go @@ -21,6 +21,7 @@ import ( "github.com/spf13/cast" + "github.com/gohugoio/hugo/common/hreflect" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/media" "github.com/gohugoio/hugo/resources/resource" @@ -125,7 +126,7 @@ func (r *PostPublishResource) fieldToString(receiver interface{}, path string) s default: v := receiverv.FieldByName(fieldname) if !v.IsValid() { - method := receiverv.MethodByName(fieldname) + method := hreflect.GetMethodByName(receiverv, fieldname) if method.IsValid() { vals := method.Call(nil) if len(vals) > 0 { diff --git a/tpl/collections/apply.go b/tpl/collections/apply.go index 0833e5507..55bab5d1a 100644 --- a/tpl/collections/apply.go +++ b/tpl/collections/apply.go @@ -131,7 +131,7 @@ func (ns *Namespace) lookupFunc(fname string) (reflect.Value, bool) { nv = reflect.ValueOf(v) // method - m := nv.MethodByName(ss[1]) + m := hreflect.GetMethodByName(nv, ss[1]) if m.Kind() == reflect.Invalid { return reflect.Value{}, false diff --git a/tpl/collections/where.go b/tpl/collections/where.go index 8ffcf6165..ac604760d 100644 --- a/tpl/collections/where.go +++ b/tpl/collections/where.go @@ -19,6 +19,7 @@ import ( "reflect" "strings" + "github.com/gohugoio/hugo/common/hreflect" "github.com/gohugoio/hugo/common/maps" ) @@ -300,8 +301,9 @@ func evaluateSubElem(obj reflect.Value, elemName string) (reflect.Value, error) objPtr = objPtr.Addr() } - mt, ok := objPtr.Type().MethodByName(elemName) - if ok { + index := hreflect.GetMethodIndexByName(objPtr.Type(), elemName) + if index != -1 { + mt := objPtr.Type().Method(index) switch { case mt.PkgPath != "": return zero, fmt.Errorf("%s is an unexported method of type %s", elemName, typ) @@ -513,5 +515,5 @@ func toTimeUnix(v reflect.Value) int64 { if v.Type() != timeType { panic("coding error: argument must be time.Time type reflect Value") } - return v.MethodByName("Unix").Call([]reflect.Value{})[0].Int() + return hreflect.GetMethodByName(v, "Unix").Call([]reflect.Value{})[0].Int() } diff --git a/tpl/internal/go_templates/texttemplate/hugo_template_test.go b/tpl/internal/go_templates/texttemplate/hugo_template_test.go index 150802bf4..cc88151e3 100644 --- a/tpl/internal/go_templates/texttemplate/hugo_template_test.go +++ b/tpl/internal/go_templates/texttemplate/hugo_template_test.go @@ -21,6 +21,7 @@ import ( "testing" qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/common/hreflect" ) type TestStruct struct { @@ -59,7 +60,7 @@ func (e *execHelper) GetMethod(ctx context.Context, tmpl Preparer, receiver refl if name != "Hello1" { return zero, zero } - m := receiver.MethodByName("Hello2") + m := hreflect.GetMethodByName(receiver, "Hello2") return m, reflect.ValueOf("v2") } diff --git a/tpl/tplimpl/template_funcs.go b/tpl/tplimpl/template_funcs.go index 8692b9ee2..0ecd5df46 100644 --- a/tpl/tplimpl/template_funcs.go +++ b/tpl/tplimpl/template_funcs.go @@ -20,9 +20,9 @@ import ( "reflect" "strings" - "github.com/gohugoio/hugo/tpl" - + "github.com/gohugoio/hugo/common/hreflect" "github.com/gohugoio/hugo/common/maps" + "github.com/gohugoio/hugo/tpl" template "github.com/gohugoio/hugo/tpl/internal/go_templates/htmltemplate" texttemplate "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate" @@ -111,9 +111,6 @@ func (t *templateExecHelper) GetMapValue(ctx context.Context, tmpl texttemplate. func (t *templateExecHelper) GetMethod(ctx context.Context, tmpl texttemplate.Preparer, receiver reflect.Value, name string) (method reflect.Value, firstArg reflect.Value) { if t.running { - // This is a hot path and receiver.MethodByName really shows up in the benchmarks, - // so we maintain a list of method names with that signature. - // TODO(bep) I have a branch that makes this construct superflous. switch name { case "GetPage", "Render": if info, ok := tmpl.(tpl.Info); ok { @@ -124,7 +121,7 @@ func (t *templateExecHelper) GetMethod(ctx context.Context, tmpl texttemplate.Pr } } - fn := receiver.MethodByName(name) + fn := hreflect.GetMethodByName(receiver, name) if !fn.IsValid() { return zero, zero }