google / google/go-cmp

Allow linker to perform dead code elimination for programs using go-cmp

Open
#373 6 comments 5 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
4.7k
Forks
243
PR merge metrics
No merged PRs in 30d

Description

**Describe the bug**
When `reflect`'s `Method` is used with a non-constant argument, the linker has to keep every public method of reachable types because it can't statically determine whether that method will be called through `reflect`. This makes binaries significantly bigger (in my experience, around 30%).
https://github.com/golang/go/blob/go1.23.6/src/cmd/link/internal/ld/deadcode.go#L420-L433

`github.com/google/go-cmp` uses `reflect.Type.Method` to generate a string representation of an unnamed interface type ([source](https://github.com/google/go-cmp/blob/9b12f366a942ebc7254abc7f32ca05068b455fb7/cmp/internal/value/name.go#L144)), which disables this dead code elimination.
Since `go-cmp` is used by various k8s packages, you can't avoid it when using those dependencies.

**To Reproduce**
Consider this simple example:
```go
package main

import (
_ "github.com/aws/aws-sdk-go-v2/service/ec2"
"github.com/google/go-cmp/cmp"
)

func main() {
_ = cmp.Equal(1, 2)
_ = cmd.Diff(1, 2)
}
```

When building with upstream `go-cmp`, the binary is 6.6MB.
When using the build tag added on [my branch](https://github.com/pgimalac/go-cmp/tree/pgimalac/build-tag-no-reflect-method), the binary is 5.1MB.

The binary is 30% bigger when DCE is disabled.

**Fixing**
I propose two ways of fixing this issue.

1. The `Equal` function can be fixed by moving code around, so that the linker can statically determine that the problematic piece of code is not reachable. https://github.com/google/go-cmp/pull/374

2. Adding a build tag to optionally replace the logic of generating a string representation of an unnamed interface, from using `reflect.Type.Method()` to using `reflect.Type.String()`. The main issue is that it changes the generated string (in particular it wouldn't follow the `qualified` argument), and I'm not sure how much of an issue that would be.
https://github.com/google/go-cmp/pull/375

I think 1. should be done as it fixes the issue by default for users of `Equal` without any functional change (except an extra space character), and 2. if you're fine with it would allow users who care to also fix `Diff` (assuming the proposed change is safe and wouldn't break any functionality).

**Additional notes**
This is somewhat similar to https://github.com/spf13/cobra/issues/2015 for `cobra`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.