Skip to content

Commit

Permalink
WIP: Use MethodByName only with constants
Browse files Browse the repository at this point in the history
We would like to enable dead-code-elimination, as unblocked by https://go-review.googlesource.com/c/go/+/522436
  • Loading branch information
justinsb committed Aug 4, 2024
1 parent 1dc08db commit a5fbdbe
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 22 deletions.
5 changes: 3 additions & 2 deletions upup/pkg/fi/cloudup/awstasks/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
"k8s.io/kops/upup/pkg/fi/cloudup/terraform"
"k8s.io/kops/util/pkg/reflectutils"
)

type renderTest struct {
Expand Down Expand Up @@ -59,14 +60,14 @@ func doRenderTests(t *testing.T, method string, cases []*renderTest) {

err := func() error {
// @step: invoke the rendering method of the target
resp := reflect.ValueOf(c.Resource).MethodByName(method).Call(inputs)
resp := reflectutils.GetMethodByName(reflect.ValueOf(c.Resource), method).Call(inputs)
if err := resp[0].Interface(); err != nil {
return err.(error)
}

// @step: invoke the target finish up
in := []reflect.Value{reflect.ValueOf(make(map[string]fi.CloudupTask))}
resp = reflect.ValueOf(target).MethodByName("Finish").Call(in)
resp = reflectutils.GetMethodByName(reflect.ValueOf(target), "Finish").Call(in)
if err := resp[0].Interface(); err != nil {
return err.(error)
}
Expand Down
56 changes: 42 additions & 14 deletions upup/pkg/fi/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"k8s.io/klog/v2"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/nodeup"
"k8s.io/kops/util/pkg/reflectutils"
"k8s.io/kops/util/pkg/vfs"
)

Expand Down Expand Up @@ -196,19 +197,44 @@ func (c *Context[T]) Render(a, e, changes Task[T]) error {

targetType := reflect.ValueOf(c.Target).Type()

var renderer *reflect.Method
var renderer *reflect.Value
var rendererArgs []reflect.Value

for i := 0; i < vType.NumMethod(); i++ {
method := vType.Method(i)
if !strings.HasPrefix(method.Name, "Render") {
rendererName := ""

renderMethodNames := []string{"Render"}

targetTypeName := fmt.Sprintf("%T", c.Target)
switch targetTypeName {
case "*awsup.AWSAPITarget":
renderMethodNames = append(renderMethodNames, "RenderAWS")
case "*azure.AzureAPITarget":
renderMethodNames = append(renderMethodNames, "RenderAzure")
case "*do.DOAPITarget":
renderMethodNames = append(renderMethodNames, "RenderDO")
case "*gce.GCEAPITarget":
renderMethodNames = append(renderMethodNames, "RenderGCE")
case "*hetzner.HetznerAPITarget":
renderMethodNames = append(renderMethodNames, "RenderHetzner")
case "*openstack.OpenstackAPITarget":
renderMethodNames = append(renderMethodNames, "RenderOpenstack")
case "*scaleway.ScwAPITarget":
renderMethodNames = append(renderMethodNames, "RenderScw")
case "*terraform.TerraformTarget":
renderMethodNames = append(renderMethodNames, "RenderTerraform")
default:
panic(fmt.Sprintf("targetType %q is not recognized", targetTypeName))
}
for _, methodName := range renderMethodNames {
method := reflectutils.GetMethodByName(v, methodName)
if !method.IsValid() {
continue
}
match := true

methodType := method.Type()
var args []reflect.Value
for j := 0; j < method.Type.NumIn(); j++ {
arg := method.Type.In(j)
for j := 0; j < methodType.NumIn(); j++ {
arg := methodType.In(j)
if arg.ConvertibleTo(vType) {
continue
}
Expand All @@ -225,27 +251,29 @@ func (c *Context[T]) Render(a, e, changes Task[T]) error {
}
if match {
if renderer != nil {
if method.Name == "Render" {
if methodName == "Render" {
continue
}
if renderer.Name != "Render" {
return fmt.Errorf("found multiple Render methods that could be involved on %T", e)
if rendererName != "Render" {
return fmt.Errorf("found multiple Render methods that could be invoked on %T", e)
}
}
renderer = &method
rendererName = methodName
rendererArgs = args
}

}

if renderer == nil {
return fmt.Errorf("could not find Render method on type %T (target %T)", e, c.Target)
}

rendererArgs = append(rendererArgs, reflect.ValueOf(a))
rendererArgs = append(rendererArgs, reflect.ValueOf(e))
rendererArgs = append(rendererArgs, reflect.ValueOf(changes))
klog.V(11).Infof("Calling method %s on %T", renderer.Name, e)
m := v.MethodByName(renderer.Name)
rv := m.Call(rendererArgs)
klog.V(11).Infof("Calling method %s on %T", rendererName, e)
// m := v.MethodByName(renderer.Name)
rv := (*renderer).Call(rendererArgs)
var rvErr error
if !rv[0].IsNil() {
rvErr = rv[0].Interface().(error)
Expand Down
47 changes: 41 additions & 6 deletions util/pkg/reflectutils/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,50 @@ func JSONMergeStruct(dest, src interface{}) {
}
}

// GetMethodByName implements a switch statement so that we can help the compiler with dead-code-elimination.
// For background: https://github.com/golang/protobuf/issues/1561
func GetMethodByName(reflectValue reflect.Value, methodName string) reflect.Value {
switch methodName {
case "CheckChanges":
return reflectValue.MethodByName("CheckChanges")
case "Find":
return reflectValue.MethodByName("Find")
case "ShouldCreate":
return reflectValue.MethodByName("ShouldCreate")
case "Finish":
return reflectValue.MethodByName("Finish")
case "Render":
return reflectValue.MethodByName("Render")
case "RenderAWS":
return reflectValue.MethodByName("RenderAWS")
case "RenderAzure":
return reflectValue.MethodByName("RenderAzure")
case "RenderDO":
return reflectValue.MethodByName("RenderDO")
case "RenderGCE":
return reflectValue.MethodByName("RenderGCE")
case "RenderHetzner":
return reflectValue.MethodByName("RenderHetzner")
case "RenderOpenstack":
return reflectValue.MethodByName("RenderOpenstack")
case "RenderScw":
return reflectValue.MethodByName("RenderScw")
case "RenderTerraform":
return reflectValue.MethodByName("RenderTerraform")

default:
panic(fmt.Sprintf("need to add %q to getMethodByName helper", methodName))
}
}

// InvokeMethod calls the specified method by reflection
func InvokeMethod(target interface{}, name string, args ...interface{}) ([]reflect.Value, error) {
func InvokeMethod(target interface{}, methodName string, args ...interface{}) ([]reflect.Value, error) {
v := reflect.ValueOf(target)

method, found := v.Type().MethodByName(name)
if !found {
m := GetMethodByName(v, methodName)
if !m.IsValid() {
return nil, &MethodNotFoundError{
Name: name,
Name: methodName,
Target: target,
}
}
Expand All @@ -73,8 +109,7 @@ func InvokeMethod(target interface{}, name string, args ...interface{}) ([]refle
for _, a := range args {
argValues = append(argValues, reflect.ValueOf(a))
}
klog.V(12).Infof("Calling method %s on %T", method.Name, target)
m := v.MethodByName(method.Name)
klog.V(12).Infof("Calling method %s on %T", methodName, target)
rv := m.Call(argValues)
return rv, nil
}
Expand Down

0 comments on commit a5fbdbe

Please sign in to comment.