Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't call MethodByName with a variable arg #6602

Merged
merged 1 commit into from
Oct 10, 2023

Commits on Sep 19, 2023

  1. Don't call MethodByName with a variable arg

    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)
    ```
    jquirke committed Sep 19, 2023
    Configuration menu
    Copy the full SHA
    a6e25fb View commit details
    Browse the repository at this point in the history