mirror of
https://github.com/gohugoio/hugo.git
synced 2024-11-21 20:46:30 -05:00
Cache reflect.MethodByName
The isolated benchmark for the function is obviously much faster: ```bash name old time/op new time/op delta GetMethodByName-10 1.21µs ± 7% 0.23µs ± 5% -81.42% (p=0.029 n=4+4) name old alloc/op new alloc/op delta GetMethodByName-10 680B ± 0% 0B -100.00% (p=0.029 n=4+4) name old allocs/op new allocs/op delta GetMethodByName-10 20.0 ± 0% 0.0 -100.00% (p=0.029 n=4+4) ``` But more pleasing is the overall performance looking at the site benchmarks: ```bash name old time/op new time/op delta SiteNew/Regular_Bundle_with_image-10 6.25ms ± 2% 6.10ms ± 2% ~ (p=0.057 n=4+4) SiteNew/Regular_Bundle_with_JSON_file-10 6.30ms ± 2% 5.66ms ±11% ~ (p=0.057 n=4+4) SiteNew/Regular_Tags_and_categories-10 22.2ms ± 2% 17.4ms ± 1% -21.88% (p=0.029 n=4+4) SiteNew/Regular_Canonify_URLs-10 108ms ± 0% 107ms ± 0% -1.20% (p=0.029 n=4+4) SiteNew/Regular_Deep_content_tree-10 36.1ms ± 1% 33.8ms ± 1% -6.44% (p=0.029 n=4+4) SiteNew/Regular_TOML_front_matter-10 24.9ms ± 1% 22.6ms ± 1% -9.30% (p=0.029 n=4+4) SiteNew/Regular_Many_HTML_templates-10 17.9ms ± 1% 16.7ms ± 1% -6.43% (p=0.029 n=4+4) SiteNew/Regular_Page_collections-10 23.3ms ± 1% 22.0ms ± 0% -5.58% (p=0.029 n=4+4) SiteNew/Regular_List_terms-10 8.00ms ± 1% 7.63ms ± 0% -4.62% (p=0.029 n=4+4) name old alloc/op new alloc/op delta SiteNew/Regular_Bundle_with_image-10 2.10MB ± 0% 2.07MB ± 0% -1.46% (p=0.029 n=4+4) SiteNew/Regular_Bundle_with_JSON_file-10 1.88MB ± 0% 1.85MB ± 0% -1.76% (p=0.029 n=4+4) SiteNew/Regular_Tags_and_categories-10 13.5MB ± 0% 11.6MB ± 0% -13.99% (p=0.029 n=4+4) SiteNew/Regular_Canonify_URLs-10 96.1MB ± 0% 95.8MB ± 0% -0.40% (p=0.029 n=4+4) SiteNew/Regular_Deep_content_tree-10 28.4MB ± 0% 27.3MB ± 0% -3.83% (p=0.029 n=4+4) SiteNew/Regular_TOML_front_matter-10 16.9MB ± 0% 15.1MB ± 0% -10.58% (p=0.029 n=4+4) SiteNew/Regular_Many_HTML_templates-10 8.98MB ± 0% 8.44MB ± 0% -6.04% (p=0.029 n=4+4) SiteNew/Regular_Page_collections-10 17.1MB ± 0% 16.5MB ± 0% -3.91% (p=0.029 n=4+4) SiteNew/Regular_List_terms-10 3.92MB ± 0% 3.72MB ± 0% -5.03% (p=0.029 n=4+4) name old allocs/op new allocs/op delta SiteNew/Regular_Bundle_with_image-10 25.8k ± 0% 24.9k ± 0% -3.49% (p=0.029 n=4+4) SiteNew/Regular_Bundle_with_JSON_file-10 25.8k ± 0% 24.9k ± 0% -3.49% (p=0.029 n=4+4) SiteNew/Regular_Tags_and_categories-10 288k ± 0% 233k ± 0% -18.90% (p=0.029 n=4+4) SiteNew/Regular_Canonify_URLs-10 375k ± 0% 364k ± 0% -2.80% (p=0.029 n=4+4) SiteNew/Regular_Deep_content_tree-10 314k ± 0% 283k ± 0% -9.77% (p=0.029 n=4+4) SiteNew/Regular_TOML_front_matter-10 302k ± 0% 252k ± 0% -16.55% (p=0.029 n=4+4) SiteNew/Regular_Many_HTML_templates-10 133k ± 0% 117k ± 0% -11.81% (p=0.029 n=4+4) SiteNew/Regular_Page_collections-10 202k ± 0% 183k ± 0% -9.55% (p=0.029 n=4+4) SiteNew/Regular_List_terms-10 55.6k ± 0% 49.8k ± 0% -10.40% (p=0.029 n=4+4) ``` Thanks to @quasilyte for the suggestion. Fixes 9386
This commit is contained in:
Bjørn Erik Pedersen
parent
ff02d41721
commit
4576c82ed4
9 changed files with 117 additions and 17 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue