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

The use of text/template disables dead code elimination in all users of cobra #2015

Open
alexander-hirth opened this issue Aug 14, 2023 · 2 comments · May be fixed by #1956
Open

The use of text/template disables dead code elimination in all users of cobra #2015

alexander-hirth opened this issue Aug 14, 2023 · 2 comments · May be fixed by #1956

Comments

@alexander-hirth
Copy link

Any use of reflect.Value.MethodByName() or reflect.Type.MethodByName() disables the DCE. The compiler assumes that these functions will look any method up, and cannot remove methods even if they are never called.

text/template is a very notable offender in this respect. It calls MethodByName() with random strings extracted from template texts.

Command::UsageFunc() and Command::HelpFunc() use text/template, hence they disable the DCE for all users of cobra. Since cobra is the go-to module to implement CLIs, this essentially disables DCE in all command line tools written in go.

A low-level library that is used by almost everyone should not pessimise the code generation by disabling the DCE. I'd avoid text/template altogether.

@aderouineau
Copy link

aderouineau commented Nov 24, 2024

I would like to +1 this. When building the very small code below, the binary is almost 40MB. When removing context.WithValue(), it's about 5MB.

package main

import (
	"context"
	"fmt"
	"os"

	"github.com/aws/aws-sdk-go-v2/service/ec2"
	"github.com/spf13/cobra"
)

func main() {
	ctx := context.Background()
	ctx = context.WithValue(ctx, "ec2Client", &ec2.Client{})

	rootCmd := &cobra.Command{}

	if err := rootCmd.ExecuteContext(ctx); err != nil {
		fmt.Fprintln(os.Stderr, err)
		os.Exit(1)
	}
}

@alexander-hirth Have you found an alternative?

@marckhouzam
Copy link
Collaborator

See #1956

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants