reflection in cli.HandleAction prevents accurate memory profiling

[bfallik]

This one had be scratching my head for a few days.

It seems like the use of reflection in cli.HandleAction prevents the collection or reporting of useful profiling data.

Given this sample program:

package main

import (
    "log"
    "os"
    "runtime/pprof"

    "github.com/urfave/cli"
)

var data []byte

func allocData() error {
    data = make([]byte, 1024*1024)
    f, err := os.Create("heapprofile.pprof")
    if err != nil {
        log.Fatal(err)
    }
    pprof.WriteHeapProfile(f)
    return f.Close()
}

func main() {
    app := cli.NewApp()
    app.Name = "allocData"
    app.Action = func(c *cli.Context) error {
        return allocData()
    }

    app.Run(os.Args)
}

attempting to analyze the memory profile doesn't yield any fruitful information:

$ go tool pprof -inuse_space bin/clitest heapprofile.pprof
Entering interactive mode (type "help" for commands)
(pprof) top5
1.16MB of 1.16MB total (  100%)
Showing top 5 nodes out of 7 (cum >= 1.16MB)
      flat  flat%   sum%        cum   cum%
    1.16MB   100%   100%     1.16MB   100%  reflect.Value.call
         0     0%   100%     1.16MB   100%  clypd/vendor/github.com/urfave/cli.(*App).Run
         0     0%   100%     1.16MB   100%  clypd/vendor/github.com/urfave/cli.HandleAction
         0     0%   100%     1.16MB   100%  main.main
         0     0%   100%     1.16MB   100%  reflect.Value.Call

It looks like all allocations for the action are aggregated and hidden by the reflection function call.

I was able to workaround this in my test case by circumventing cli entirely but it'd be great if HandleAction could prefer an interface type assertion before resorting to reflection.

I'm also not sure if this is a bug in Go itself. Should the compiler and tools correctly collect memory profiling data across reflection boundaries?

Thanks in advance.

[jszwedko]

Oh wow that is pretty interesting, I apologize for causing what I'm sure was some amount of confusion. I've addressed the worst offender in #556 (which would have hid the entire action). I think the other usages would be much more difficult to expunge, but these wouldn't hide function calls anyway which seems to be the primary concern here.

Thank you for the report!

[bfallik]

@jszwedko thanks for taking care of this!

[jszwedko]

👍 no problem! Closing this out, but please reopen if this doesn't address your issue.