proposal: cmd/link: option to force DCE when reflect Method is present

[ikonst]

Proposal Details

Summary

Provide an option (through GODEBUG or a linker flag?) to force exported method dead-code elimination despite presence of reflect Method or (non-constant) MethodByName in the code. When enabled, the resulting executable would have its reflection APIs return only methods that are otherwise referenced.

For edge cases, the user could force a reference e.g. by adding a dummy MethodByName

_, _ = reflect.TypeOf(0).MethodByName("MissingMethod")

Motivation

The user will trade a compatibility promise (could break packages in unpredictable ways) for faster build times and smaller binaries (can be dramatic when importing some excessively large packages).

It's not practical for a user to achieve this otherwise, e.g. through being intentional in the choice of imported packages, in anything but the smallest of codebases. While ensuring that no dependency calls those functions, the user will soon realize that:

• even the ever-ubiquitous text/template package calls reflect's MethodByName
• curation of all packages today could be foiled by any package changing tomorrow
• often method reflection is not actually required (e.g. provided data has only fields but text/template trying to find a method before trying it as a field), and the linker's choice to disable DCE stems from caution rather than an educated guess

P.S.

This very proposal was implemented in https://go-review.googlesource.com/c/go/+/488575 but wasn't meant to be submitted.

[ianlancetaylor]

In order to use such an option you would have to know that it worked for every single call to MethodByName in your program. Given the widespread use of text/template and similar packages in the Go ecosystem, that only seems possible for very restricted programs. So this seems like a configuration knob that would have to be maintained and tested, but that almost nobody would be able to use. The benefit does not seem worth the cost.

[gabyhelp]

Related Issues

• cmd/link: add debug log to identify deadcode blockers #62681 (closed)
• cmd/link: enable linker dead code pruning when using reflect #25081
• cmd/link: there should be a way to determine why deadcode elimination was not performed #60221 (closed)

Related Code Changes

• cmd/link: add --methodbyname flag to fail if MethodByName is used
• cmd/link, reflect: remove some method type data
• reflect: document Method(ByName) w.r.t dead code elimination
• cmd/link: prune unused methods
• cmd/link: debugging flag to perturb/tweak linker dead code
• [dev.link] cmd/link: support reflect.Type.Method tracking in deadcode2

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ikonst]

Thanks for the feedback @ianlancetaylor, I agree with your rationale. Similarly to avoiding MethodByName working only for the smallest codebases, so is being certain that such flag would be safe to use.

I'm retracting this proposal. Members of the community (e.g. Uber here) do appear interested in maintaining DCE in large codebases and finding a practical approach, so I think it was good to put this up as a straw-man proposal (to capture what isn't practical).

[jquirke]

Thanks for the feedback @ianlancetaylor, I agree with your rationale. Similarly to avoiding MethodByName working only for the smallest codebases, so is being certain that such flag would be safe to use.

I'm retracting this proposal. Members of the community (e.g. Uber go-gorm/gorm#6602) do appear interested in maintaining DCE in large codebases and finding a practical approach, so I think it was good to put this up as a straw-man proposal (to capture what isn't practical).

Thankyou for referencing my work in trying play whack-a-mole with DCE problematic modules, but it is absolutely not enjoyable :) and in many cases the author of the module has made "interesting" design choices or has little understanding of the deadcode problem and therefore sees the change as unnecessary. See the comments on 6602

I think a fair "solution" for right now is to have a more comprehensive documentation such as a blog post from the Go team which can provide guidance to developers on this matter. I am happy to volunteer a draft as I have written some internal docs on this at Uber.

I know there is documentation that subtly warns of this issue (e.g. https://pkg.go.dev/reflect#Value.MethodByName) but something more detailed and concrete I believe would help more.

Happy new year everyone.