Skip to content

Commit

Permalink
fix(storage): do not inhibit the dead code elimination. (#8543)
Browse files Browse the repository at this point in the history
When the go compiler encounters a call to MethodByName(), it
assumes that every method of every reachable type can be accessed,
and nothing may be eliminated.

Go 1.22 has gained support for calls like MethodByName("Foobar").
If the argument to MethodByName() is a compile-time constant, then
the compiler no longer disables the DCE, and only keeps methods
named Foobar.

Rewrite setConditionField() so that MethodByName() is only called with
compile-time constants. This way importing cloud.google.com/go/storage
will not inhibit the DCE.

Co-authored-by: Brenna N Epp <brennae@google.com>
  • Loading branch information
dominiquelefevre and BrennaEpp committed Nov 14, 2023
1 parent 19414ae commit ca2493f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 15 deletions.
4 changes: 2 additions & 2 deletions storage/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -1360,11 +1360,11 @@ func applyBucketConds(method string, conds *BucketConditions, call interface{})
cval := reflect.ValueOf(call)
switch {
case conds.MetagenerationMatch != 0:
if !setConditionField(cval, "IfMetagenerationMatch", conds.MetagenerationMatch) {
if !setIfMetagenerationMatch(cval, conds.MetagenerationMatch) {
return fmt.Errorf("storage: %s: ifMetagenerationMatch not supported", method)
}
case conds.MetagenerationNotMatch != 0:
if !setConditionField(cval, "IfMetagenerationNotMatch", conds.MetagenerationNotMatch) {
if !setIfMetagenerationNotMatch(cval, conds.MetagenerationNotMatch) {
return fmt.Errorf("storage: %s: ifMetagenerationNotMatch not supported", method)
}
}
Expand Down
55 changes: 42 additions & 13 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -1806,7 +1806,7 @@ func (c *Conditions) isMetagenerationValid() bool {
func applyConds(method string, gen int64, conds *Conditions, call interface{}) error {
cval := reflect.ValueOf(call)
if gen >= 0 {
if !setConditionField(cval, "Generation", gen) {
if !setGeneration(cval, gen) {
return fmt.Errorf("storage: %s: generation not supported", method)
}
}
Expand All @@ -1818,25 +1818,25 @@ func applyConds(method string, gen int64, conds *Conditions, call interface{}) e
}
switch {
case conds.GenerationMatch != 0:
if !setConditionField(cval, "IfGenerationMatch", conds.GenerationMatch) {
if !setIfGenerationMatch(cval, conds.GenerationMatch) {
return fmt.Errorf("storage: %s: ifGenerationMatch not supported", method)
}
case conds.GenerationNotMatch != 0:
if !setConditionField(cval, "IfGenerationNotMatch", conds.GenerationNotMatch) {
if !setIfGenerationNotMatch(cval, conds.GenerationNotMatch) {
return fmt.Errorf("storage: %s: ifGenerationNotMatch not supported", method)
}
case conds.DoesNotExist:
if !setConditionField(cval, "IfGenerationMatch", int64(0)) {
if !setIfGenerationMatch(cval, int64(0)) {
return fmt.Errorf("storage: %s: DoesNotExist not supported", method)
}
}
switch {
case conds.MetagenerationMatch != 0:
if !setConditionField(cval, "IfMetagenerationMatch", conds.MetagenerationMatch) {
if !setIfMetagenerationMatch(cval, conds.MetagenerationMatch) {
return fmt.Errorf("storage: %s: ifMetagenerationMatch not supported", method)
}
case conds.MetagenerationNotMatch != 0:
if !setConditionField(cval, "IfMetagenerationNotMatch", conds.MetagenerationNotMatch) {
if !setIfMetagenerationNotMatch(cval, conds.MetagenerationNotMatch) {
return fmt.Errorf("storage: %s: ifMetagenerationNotMatch not supported", method)
}
}
Expand Down Expand Up @@ -1897,16 +1897,45 @@ func applySourceCondsProto(gen int64, conds *Conditions, call *storagepb.Rewrite
return nil
}

// setConditionField sets a field on a *raw.WhateverCall.
// setGeneration sets Generation on a *raw.WhateverCall.
// We can't use anonymous interfaces because the return type is
// different, since the field setters are builders.
func setConditionField(call reflect.Value, name string, value interface{}) bool {
m := call.MethodByName(name)
if !m.IsValid() {
return false
// We also make sure to supply a compile-time constant to MethodByName;
// otherwise, the Go Linker will disable dead code elimination, leading
// to larger binaries for all packages that import storage.
func setGeneration(cval reflect.Value, value interface{}) bool {
return setCondition(cval.MethodByName("Generation"), value)
}

// setIfGenerationMatch sets IfGenerationMatch on a *raw.WhateverCall.
// See also setGeneration.
func setIfGenerationMatch(cval reflect.Value, value interface{}) bool {
return setCondition(cval.MethodByName("IfGenerationMatch"), value)
}

// setIfGenerationNotMatch sets IfGenerationNotMatch on a *raw.WhateverCall.
// See also setGeneration.
func setIfGenerationNotMatch(cval reflect.Value, value interface{}) bool {
return setCondition(cval.MethodByName("IfGenerationNotMatch"), value)
}

// setIfMetagenerationMatch sets IfMetagenerationMatch on a *raw.WhateverCall.
// See also setGeneration.
func setIfMetagenerationMatch(cval reflect.Value, value interface{}) bool {
return setCondition(cval.MethodByName("IfMetagenerationMatch"), value)
}

// setIfMetagenerationNotMatch sets IfMetagenerationNotMatch on a *raw.WhateverCall.
// See also setGeneration.
func setIfMetagenerationNotMatch(cval reflect.Value, value interface{}) bool {
return setCondition(cval.MethodByName("IfMetagenerationNotMatch"), value)
}

func setCondition(setter reflect.Value, value interface{}) bool {
if setter.IsValid() {
setter.Call([]reflect.Value{reflect.ValueOf(value)})
}
m.Call([]reflect.Value{reflect.ValueOf(value)})
return true
return setter.IsValid()
}

// Retryer returns an object handle that is configured with custom retry
Expand Down

0 comments on commit ca2493f

Please sign in to comment.