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

expvar: make it possible to remove memstat and cmdline defaults #29105

Open
as opened this Issue Dec 5, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@as
Copy link
Contributor

as commented Dec 5, 2018

The expvar package contains an init function that registers the memstats and cmdline vars to the debug/vars http handler. The package does not provide a means to remove these defaults, forcing the consumer to deal with them. It would be nice if the user had a means to remove these defaults without resorting to unsafe or forking the package.

A benchmark I've added reveals a mild but unnecessary performance impact with the defaults (when the number of total expvar.Vars is expected to be small), but the real problem I think is aesthetic: My other vars have nothing to do with memory usage or the command line, so the memory profile is intrusive to the application.

We also can't use memstats and cmdline as named for our own purposes. This does not impact me personally.

goos: windows
goarch: amd64
pkg: expvar
BenchmarkExpvarHandler/cmdline.memstats-8               200000          7219 ns/op
BenchmarkExpvarHandler/none-8                            5000000           278 ns/op
BenchmarkExpvarHandler/user.10.vars-8                     300000          5130 ns/op
PASS
ok      expvar    4.822s
func BenchmarkExpvarHandler(b *testing.B) {
    for _, tc := range []struct {
        Name string
        Init func()
    }{
        {"cmdline.memstats", func() {}},
        {"none", func() { RemoveAll() }},
        {"user.10.vars", func() { for i := 0; i < 10; i++ { NewInt(fmt.Sprintf("i%d", i)) } }},
    } {
        tc.Init()
        b.Run(tc.Name, func(b *testing.B) {
            for n := 0; n < b.N; n++ {
                expvarHandler(writer{}, nil)
            }
        })
    }
}

type writer http.Header
func (w writer) WriteHeader(_ int)           {}
func (w writer) Write(p []byte) (int, error) { return len(p), nil }
func (w writer) Header() http.Header         { return http.Header(w) }

Interestingly, the package tests contain an unexported RemoveAll function. The recently-closed issue #27555 discusses it in detail, but does not mention the potential use for removing the default vars.

@as as changed the title expvar: make it possible to remove memory and command line defaults expvar: make it possible to remove memstat and cmdline defaults Dec 5, 2018

@agnivade agnivade added this to the Go1.13 milestone Dec 11, 2018

@agnivade

This comment has been minimized.

Copy link
Member

agnivade commented Dec 11, 2018

/cc @bradfitz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment