internal/descfmt: replace uses of MethodByName with direct calls when possible Update internal/descfmt/stringer.go to avoid calling MethodByName when possible. MethodByName allows calling a method even when the signature is unknown, but it forces the linker to keep all methods with a matching name, increasing binary size with dead code. On the other hand casting to an interface only keeps methods with a matching signature. Using MethodByName also bypasses compiler typing, so making a direct call is safer. In this specific case, the code checks whether a type implements an interface or is a specific type before using MethodByName and FieldByName to access its methods and fields, so we can just access them directly. I checked the size difference for the datadog-agent and kubernetes binaries (ranging from 20 to 100MiB), and the resulting gains were between 100 and 300kiB, which is small but not negligible. This is a follow-up of https://go.dev/cl/734280. Fixes golang/protobuf#1708 Change-Id: Ib6ca16f7f7a871c8625d934c6115b47e525a42e9 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/735401 Reviewed-by: Lasse Folger <lassefolger@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Stapelberg <stapelberg@google.com>
diff --git a/internal/descfmt/stringer.go b/internal/descfmt/stringer.go index caa7e80..0b0dfac 100644 --- a/internal/descfmt/stringer.go +++ b/internal/descfmt/stringer.go
@@ -83,12 +83,13 @@ case protoreflect.FileImports: for i := 0; i < vs.Len(); i++ { var rs records - rv := reflect.ValueOf(vs.Get(i)) - rs.Append(rv, []methodAndName{ - {rv.MethodByName("Path"), "Path"}, - {rv.MethodByName("Package"), "Package"}, - {rv.MethodByName("IsPublic"), "IsPublic"}, - {rv.MethodByName("IsWeak"), "IsWeak"}, + fi := vs.Get(i) + rv := reflect.ValueOf(fi) + rs.Append(rv, []attrAndName{ + {fi.Path(), "Path"}, + {fi.Package(), "Package"}, + {fi.IsPublic, "IsPublic"}, + {fi.IsWeak, "IsWeak"}, }...) ss = append(ss, "{"+rs.Join()+"}") } @@ -104,9 +105,9 @@ } } -type methodAndName struct { - method reflect.Value - name string +type attrAndName struct { + attr any + name string } func FormatDesc(s fmt.State, r rune, t protoreflect.Descriptor) { @@ -126,58 +127,58 @@ start = rt.Name() + "{" } - _, isFile := t.(protoreflect.FileDescriptor) + fd, isFile := t.(protoreflect.FileDescriptor) rs := records{ allowMulti: allowMulti, record: record, } if t.IsPlaceholder() { if isFile { - rs.Append(rv, []methodAndName{ - {rv.MethodByName("Path"), "Path"}, - {rv.MethodByName("Package"), "Package"}, - {rv.MethodByName("IsPlaceholder"), "IsPlaceholder"}, + rs.Append(rv, []attrAndName{ + {fd.Path(), "Path"}, + {fd.Package(), "Package"}, + {fd.IsPlaceholder(), "IsPlaceholder"}, }...) } else { - rs.Append(rv, []methodAndName{ - {rv.MethodByName("FullName"), "FullName"}, - {rv.MethodByName("IsPlaceholder"), "IsPlaceholder"}, + rs.Append(rv, []attrAndName{ + {t.FullName(), "FullName"}, + {t.IsPlaceholder(), "IsPlaceholder"}, }...) } } else { switch { case isFile: - rs.Append(rv, methodAndName{rv.MethodByName("Syntax"), "Syntax"}) + rs.Append(rv, attrAndName{fd.Syntax(), "Syntax"}) case isRoot: - rs.Append(rv, []methodAndName{ - {rv.MethodByName("Syntax"), "Syntax"}, - {rv.MethodByName("FullName"), "FullName"}, + rs.Append(rv, []attrAndName{ + {t.Syntax(), "Syntax"}, + {t.FullName(), "FullName"}, }...) default: - rs.Append(rv, methodAndName{rv.MethodByName("Name"), "Name"}) + rs.Append(rv, attrAndName{t.Name(), "Name"}) } switch t := t.(type) { case protoreflect.FieldDescriptor: - accessors := []methodAndName{ - {rv.MethodByName("Number"), "Number"}, - {rv.MethodByName("Cardinality"), "Cardinality"}, - {rv.MethodByName("Kind"), "Kind"}, - {rv.MethodByName("HasJSONName"), "HasJSONName"}, - {rv.MethodByName("JSONName"), "JSONName"}, - {rv.MethodByName("HasPresence"), "HasPresence"}, - {rv.MethodByName("IsExtension"), "IsExtension"}, - {rv.MethodByName("IsPacked"), "IsPacked"}, - {rv.MethodByName("IsWeak"), "IsWeak"}, - {rv.MethodByName("IsList"), "IsList"}, - {rv.MethodByName("IsMap"), "IsMap"}, - {rv.MethodByName("MapKey"), "MapKey"}, - {rv.MethodByName("MapValue"), "MapValue"}, - {rv.MethodByName("HasDefault"), "HasDefault"}, - {rv.MethodByName("Default"), "Default"}, - {rv.MethodByName("ContainingOneof"), "ContainingOneof"}, - {rv.MethodByName("ContainingMessage"), "ContainingMessage"}, - {rv.MethodByName("Message"), "Message"}, - {rv.MethodByName("Enum"), "Enum"}, + accessors := []attrAndName{ + {t.Number(), "Number"}, + {t.Cardinality(), "Cardinality"}, + {t.Kind(), "Kind"}, + {t.HasJSONName(), "HasJSONName"}, + {t.JSONName(), "JSONName"}, + {t.HasPresence(), "HasPresence"}, + {t.IsExtension(), "IsExtension"}, + {t.IsPacked(), "IsPacked"}, + {t.IsWeak(), "IsWeak"}, + {t.IsList(), "IsList"}, + {t.IsMap(), "IsMap"}, + {t.MapKey(), "MapKey"}, + {t.MapValue(), "MapValue"}, + {t.HasDefault(), "HasDefault"}, + {t.Default(), "Default"}, + {t.ContainingOneof(), "ContainingOneof"}, + {t.ContainingMessage(), "ContainingMessage"}, + {t.Message(), "Message"}, + {t.Enum(), "Enum"}, } for _, s := range accessors { switch s.name { @@ -223,62 +224,54 @@ } case protoreflect.FileDescriptor: - rs.Append(rv, []methodAndName{ - {rv.MethodByName("Path"), "Path"}, - {rv.MethodByName("Package"), "Package"}, - {rv.MethodByName("Imports"), "Imports"}, - {rv.MethodByName("Messages"), "Messages"}, - {rv.MethodByName("Enums"), "Enums"}, - {rv.MethodByName("Extensions"), "Extensions"}, - {rv.MethodByName("Services"), "Services"}, + rs.Append(rv, []attrAndName{ + {t.Path(), "Path"}, + {t.Package(), "Package"}, + {t.Imports(), "Imports"}, + {t.Messages(), "Messages"}, + {t.Enums(), "Enums"}, + {t.Extensions(), "Extensions"}, + {t.Services(), "Services"}, }...) case protoreflect.MessageDescriptor: - rs.Append(rv, []methodAndName{ - {rv.MethodByName("IsMapEntry"), "IsMapEntry"}, - {rv.MethodByName("Fields"), "Fields"}, - {rv.MethodByName("Oneofs"), "Oneofs"}, - {rv.MethodByName("ReservedNames"), "ReservedNames"}, - {rv.MethodByName("ReservedRanges"), "ReservedRanges"}, - {rv.MethodByName("RequiredNumbers"), "RequiredNumbers"}, - {rv.MethodByName("ExtensionRanges"), "ExtensionRanges"}, - {rv.MethodByName("Messages"), "Messages"}, - {rv.MethodByName("Enums"), "Enums"}, - {rv.MethodByName("Extensions"), "Extensions"}, + rs.Append(rv, []attrAndName{ + {t.IsMapEntry(), "IsMapEntry"}, + {t.Fields(), "Fields"}, + {t.Oneofs(), "Oneofs"}, + {t.ReservedNames(), "ReservedNames"}, + {t.ReservedRanges(), "ReservedRanges"}, + {t.RequiredNumbers(), "RequiredNumbers"}, + {t.ExtensionRanges(), "ExtensionRanges"}, + {t.Messages(), "Messages"}, + {t.Enums(), "Enums"}, + {t.Extensions(), "Extensions"}, }...) case protoreflect.EnumDescriptor: - rs.Append(rv, []methodAndName{ - {rv.MethodByName("Values"), "Values"}, - {rv.MethodByName("ReservedNames"), "ReservedNames"}, - {rv.MethodByName("ReservedRanges"), "ReservedRanges"}, - {rv.MethodByName("IsClosed"), "IsClosed"}, + rs.Append(rv, []attrAndName{ + {t.Values(), "Values"}, + {t.ReservedNames(), "ReservedNames"}, + {t.ReservedRanges(), "ReservedRanges"}, + {t.IsClosed(), "IsClosed"}, }...) case protoreflect.EnumValueDescriptor: - rs.Append(rv, []methodAndName{ - {rv.MethodByName("Number"), "Number"}, - }...) + rs.Append(rv, attrAndName{t.Number(), "Number"}) case protoreflect.ServiceDescriptor: - // MethodByName(<constant-string>) makes the linker keep all methods with - // the given name for all reachable types. - // In particular for the name "Methods" it means the linker will keep - // `reflect.Value.Methods()` and `reflect.Type.Methods()` with Go 1.26+, - // which disable method dead code elimination entirely. - // So we avoid using MethodByName for Methods. - rs.appendCallResult(rv, "Methods", reflect.ValueOf(t.Methods())) + rs.Append(rv, attrAndName{t.Methods(), "Methods"}) case protoreflect.MethodDescriptor: - rs.Append(rv, []methodAndName{ - {rv.MethodByName("Input"), "Input"}, - {rv.MethodByName("Output"), "Output"}, - {rv.MethodByName("IsStreamingClient"), "IsStreamingClient"}, - {rv.MethodByName("IsStreamingServer"), "IsStreamingServer"}, + rs.Append(rv, []attrAndName{ + {t.Input(), "Input"}, + {t.Output(), "Output"}, + {t.IsStreamingClient(), "IsStreamingClient"}, + {t.IsStreamingServer(), "IsStreamingServer"}, }...) } - if m := rv.MethodByName("GoType"); m.IsValid() { - rs.Append(rv, methodAndName{m, "GoType"}) + if m, ok := t.(interface{ GoType() reflect.Type }); ok { + rs.Append(rv, attrAndName{m.GoType(), "GoType"}) } } return start + rs.Join() + end @@ -301,26 +294,20 @@ rs.recs = append(rs.recs, newRecs) } -func (rs *records) Append(v reflect.Value, accessors ...methodAndName) { - for _, a := range accessors { - var rv reflect.Value - if a.method.IsValid() { - rv = a.method.Call(nil)[0] - } - rs.appendCallResult(v, a.name, rv) +func (rs *records) Append(v reflect.Value, results ...attrAndName) { + for _, r := range results { + rs.appendAttribute(v, r.name, r.attr) } } -func (rs *records) appendCallResult(val reflect.Value, name string, rv reflect.Value) { +func (rs *records) appendAttribute(val reflect.Value, name string, attrVal any) { if rs.record != nil { rs.record(name) } - if val.Kind() == reflect.Struct && !rv.IsValid() { - rv = val.FieldByName(name) + if attrVal == nil { + return } - if !rv.IsValid() { - panic(fmt.Sprintf("unknown accessor: %v.%s", val.Type(), name)) - } + rv := reflect.ValueOf(attrVal) if _, ok := rv.Interface().(protoreflect.Value); ok { rv = rv.MethodByName("Interface").Call(nil)[0] if !rv.IsNil() {