mirror of https://github.com/go-gorm/gorm.git
Don't call MethodByName with a variable arg (#6602)
Go 1.22 goes somewhat toward addressing the issue using reflect MethodByName disabling linker deadcode elimination (DCE) and the resultant large increase in binary size because the linker cannot prune unused code because it might be reached via reflection. Go Issue golang/go#62257 reduces the number of incidences of this problem by leveraging a compiler assist to avoid marking functions containing calls to MethodByName as ReflectMethods as long as the arguments are constants. An analysis of Uber Technologies code base however shows that a number of transitive imports still contain calls to MethodByName with a variable argument, including GORM. In the case of GORM, the solution we are proposing is because the number of possible methods is finite, we will "unroll" this. This demonstrably shows that GORM is not longer a problem for DCE. Before ``` % go version go version devel go1.22-2f3458a8ce Sat Sep 16 16:26:48 2023 -0700 darwin/arm64 % go test ./... -ldflags=-dumpdep 2> >(grep -i -e '->.*<reflectmethod>') gorm.io/gorm.(*Statement).BuildCondition -> gorm.io/gorm/schema.ParseWithSpecialTableName <ReflectMethod> type:reflect.Value <UsedInIface> -> reflect.(*Value).Method <ReflectMethod> type:reflect.Value <UsedInIface> -> reflect.(*Value).MethodByName <ReflectMethod> ok gorm.io/gorm (cached) ok gorm.io/gorm/callbacks (cached) gorm.io/gorm/clause_test.BenchmarkComplexSelect -> gorm.io/gorm/schema.ParseWithSpecialTableName <ReflectMethod> type:reflect.Value <UsedInIface> -> reflect.(*Value).Method <ReflectMethod> type:reflect.Value <UsedInIface> -> reflect.(*Value).MethodByName <ReflectMethod> ? gorm.io/gorm/migrator [no test files] ok gorm.io/gorm/clause (cached) ok gorm.io/gorm/logger (cached) gorm.io/gorm/schema_test.TestAdvancedDataTypeValuerAndSetter -> gorm.io/gorm/schema.ParseWithSpecialTableName <ReflectMethod> type:reflect.Value <UsedInIface> -> reflect.(*Value).Method <ReflectMethod> type:reflect.Value <UsedInIface> -> reflect.(*Value).MethodByName <ReflectMethod> ? gorm.io/gorm/utils/tests [no test files] ok gorm.io/gorm/schema (cached) ok gorm.io/gorm/utils (cached) ``` After ``` %go version go version devel go1.22-2f3458a8ce Sat Sep 16 16:26:48 2023 -0700 darwin/arm64 %go test ./... -ldflags=-dumpdep 2> >(grep -i -e '->.*<reflectmethod>') ok gorm.io/gorm (cached) ok gorm.io/gorm/callbacks (cached) ? gorm.io/gorm/migrator [no test files] ? gorm.io/gorm/utils/tests [no test files] ok gorm.io/gorm/clause (cached) ok gorm.io/gorm/logger (cached) ok gorm.io/gorm/schema (cached) ok gorm.io/gorm/utils (cached) ```
This commit is contained in:
parent
12ba285a52
commit
8c18714462
|
@ -13,6 +13,20 @@ import (
|
|||
"gorm.io/gorm/logger"
|
||||
)
|
||||
|
||||
type callbackType string
|
||||
|
||||
const (
|
||||
callbackTypeBeforeCreate callbackType = "BeforeCreate"
|
||||
callbackTypeBeforeUpdate callbackType = "BeforeUpdate"
|
||||
callbackTypeAfterCreate callbackType = "AfterCreate"
|
||||
callbackTypeAfterUpdate callbackType = "AfterUpdate"
|
||||
callbackTypeBeforeSave callbackType = "BeforeSave"
|
||||
callbackTypeAfterSave callbackType = "AfterSave"
|
||||
callbackTypeBeforeDelete callbackType = "BeforeDelete"
|
||||
callbackTypeAfterDelete callbackType = "AfterDelete"
|
||||
callbackTypeAfterFind callbackType = "AfterFind"
|
||||
)
|
||||
|
||||
// ErrUnsupportedDataType unsupported data type
|
||||
var ErrUnsupportedDataType = errors.New("unsupported data type")
|
||||
|
||||
|
@ -288,14 +302,20 @@ func ParseWithSpecialTableName(dest interface{}, cacheStore *sync.Map, namer Nam
|
|||
}
|
||||
}
|
||||
|
||||
callbacks := []string{"BeforeCreate", "AfterCreate", "BeforeUpdate", "AfterUpdate", "BeforeSave", "AfterSave", "BeforeDelete", "AfterDelete", "AfterFind"}
|
||||
for _, name := range callbacks {
|
||||
if methodValue := modelValue.MethodByName(name); methodValue.IsValid() {
|
||||
callbackTypes := []callbackType{
|
||||
callbackTypeBeforeCreate, callbackTypeAfterCreate,
|
||||
callbackTypeBeforeUpdate, callbackTypeAfterUpdate,
|
||||
callbackTypeBeforeSave, callbackTypeAfterSave,
|
||||
callbackTypeBeforeDelete, callbackTypeAfterDelete,
|
||||
callbackTypeAfterFind,
|
||||
}
|
||||
for _, cbName := range callbackTypes {
|
||||
if methodValue := callBackToMethodValue(modelValue, cbName); methodValue.IsValid() {
|
||||
switch methodValue.Type().String() {
|
||||
case "func(*gorm.DB) error": // TODO hack
|
||||
reflect.Indirect(reflect.ValueOf(schema)).FieldByName(name).SetBool(true)
|
||||
reflect.Indirect(reflect.ValueOf(schema)).FieldByName(string(cbName)).SetBool(true)
|
||||
default:
|
||||
logger.Default.Warn(context.Background(), "Model %v don't match %vInterface, should be `%v(*gorm.DB) error`. Please see https://gorm.io/docs/hooks.html", schema, name, name)
|
||||
logger.Default.Warn(context.Background(), "Model %v don't match %vInterface, should be `%v(*gorm.DB) error`. Please see https://gorm.io/docs/hooks.html", schema, cbName, cbName)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -349,6 +369,39 @@ func ParseWithSpecialTableName(dest interface{}, cacheStore *sync.Map, namer Nam
|
|||
return schema, schema.err
|
||||
}
|
||||
|
||||
// This unrolling is needed to show to the compiler the exact set of methods
|
||||
// that can be used on the modelType.
|
||||
// Prior to go1.22 any use of MethodByName would cause the linker to
|
||||
// abandon dead code elimination for the entire binary.
|
||||
// As of go1.22 the compiler supports one special case of a string constant
|
||||
// being passed to MethodByName. For enterprise customers or those building
|
||||
// large binaries, this gives a significant reduction in binary size.
|
||||
// https://github.com/golang/go/issues/62257
|
||||
func callBackToMethodValue(modelType reflect.Value, cbType callbackType) reflect.Value {
|
||||
switch cbType {
|
||||
case callbackTypeBeforeCreate:
|
||||
return modelType.MethodByName(string(callbackTypeBeforeCreate))
|
||||
case callbackTypeAfterCreate:
|
||||
return modelType.MethodByName(string(callbackTypeAfterCreate))
|
||||
case callbackTypeBeforeUpdate:
|
||||
return modelType.MethodByName(string(callbackTypeBeforeUpdate))
|
||||
case callbackTypeAfterUpdate:
|
||||
return modelType.MethodByName(string(callbackTypeAfterUpdate))
|
||||
case callbackTypeBeforeSave:
|
||||
return modelType.MethodByName(string(callbackTypeBeforeSave))
|
||||
case callbackTypeAfterSave:
|
||||
return modelType.MethodByName(string(callbackTypeAfterSave))
|
||||
case callbackTypeBeforeDelete:
|
||||
return modelType.MethodByName(string(callbackTypeBeforeDelete))
|
||||
case callbackTypeAfterDelete:
|
||||
return modelType.MethodByName(string(callbackTypeAfterDelete))
|
||||
case callbackTypeAfterFind:
|
||||
return modelType.MethodByName(string(callbackTypeAfterFind))
|
||||
default:
|
||||
return reflect.ValueOf(nil)
|
||||
}
|
||||
}
|
||||
|
||||
func getOrParse(dest interface{}, cacheStore *sync.Map, namer Namer) (*Schema, error) {
|
||||
modelType := reflect.ValueOf(dest).Type()
|
||||
for modelType.Kind() == reflect.Slice || modelType.Kind() == reflect.Array || modelType.Kind() == reflect.Ptr {
|
||||
|
|
Loading…
Reference in New Issue