add -diff flag for better profile comparision (#369)

Author: nolanmar511

doc/README.md

@@ -216,6 +216,44 @@ search for them in a directory pointed to by the environment variable
 * **-weblist= _regex_:** Generates a source/assembly combined annotated listing for
   functions matching *regex*, and starts a web browser to display it.
 
+## Comparing profiles
+
+pprof can subtract one profile from another, provided the profiles are of
+compatible types (i.e. two heap profiles). pprof has two options which can be
+used to specify the filename or URL for a profile to be subtracted from the
+source profile:
+
+* **-diff_base= _profile_:** useful for comparing two profiles. Percentages in
+the output are relative to the total of samples in the diff base profile.
+
+* **-base= _profile_:** useful for subtracting a cumulative profile, like a
+[golang block profile](https://golang.org/doc/diagnostics.html#profiling),
+from another cumulative profile collected from the same program at a later time.
+When comparing cumulative profiles collected on the same program, percentages in
+the output are relative to the difference between the total for the source
+profile and the total for the base profile.
+
+The **-normalize** flag can be used when a base profile is specified with either 
+the `-diff_base` or the `-base` option. This flag scales the source profile so
+that the total of samples in the source profile is equal to the total of samples
+in the base profile prior to subtracting the base profile from the source
+profile. Useful for determining the relative differences between profiles, for
+example, which profile has a larger percentage of CPU time used in a particular
+function.
+
+When using the **-diff_base** option, some report entries may have negative
+values. If the merged profile is output as a protocol buffer, all samples in the
+diff base profile will have a label with the key "pprof::base" and a value of
+"true". If pprof is then used to look at the merged profile, it will behave as
+if separate source and base profiles were passed in.
+
+When using the **-base** option to subtract one cumulative profile from another
+collected on the same program at a later time, percentages will be relative to
+the difference between the total for the source profile and the total for
+the base profile, and all values will be positive. In the general case, some 
+report entries may have negative values and percentages will be relative to the
+total of the absolute value of all samples when aggregated at the address level.
+
 # Fetching profiles
 
 pprof can read profiles from a file or directly from a URL over http. Its native
@@ -237,11 +275,6 @@ them. This is useful to combine profiles from multiple processes of a
 distributed job. The profiles may be from different programs but must be
 compatible (for example, CPU profiles cannot be combined with heap profiles).
 
-pprof can subtract a profile from another in order to compare them. For that,
-use the **-base= _profile_** option, where *profile* is the filename or URL for the
-profile to be subtracted. This may result on some report entries having negative
-values.
-
 ## Symbolization
 
 pprof can add symbol information to a profile that was collected only with
@@ -286,4 +319,3 @@ the symbolization handler.
 
 * **-symbolize=demangle=templates:** Demangle, and trim function parameters, but
   not template parameters.
-

internal/driver/cli.go

@@ -15,6 +15,7 @@
 package driver
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -28,6 +29,7 @@ type source struct {
 	ExecName  string
 	BuildID   string
 	Base      []string
+	DiffBase  bool
 	Normalize bool
 
 	Seconds      int
@@ -43,7 +45,8 @@ type source struct {
 func parseFlags(o *plugin.Options) (*source, []string, error) {
 	flag := o.Flagset
 	// Comparisons.
-	flagBase := flag.StringList("base", "", "Source for base profile for comparison")
+	flagBase := flag.StringList("base", "", "Source for base profile for profile subtraction")
+	flagDiffBase := flag.StringList("diff_base", "", "Source for diff base profile for comparison")
 	// Source options.
 	flagSymbolize := flag.String("symbolize", "", "Options for profile symbolization")
 	flagBuildID := flag.String("buildid", "", "Override build id for first mapping")
@@ -85,7 +88,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) {
 			usageMsgVars)
 	})
 	if len(args) == 0 {
-		return nil, nil, fmt.Errorf("no profile source specified")
+		return nil, nil, errors.New("no profile source specified")
 	}
 
 	var execName string
@@ -112,7 +115,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) {
 		return nil, nil, err
 	}
 	if cmd != nil && *flagHTTP != "" {
-		return nil, nil, fmt.Errorf("-http is not compatible with an output format on the command line")
+		return nil, nil, errors.New("-http is not compatible with an output format on the command line")
 	}
 
 	si := pprofVariables["sample_index"].value
@@ -140,15 +143,13 @@ func parseFlags(o *plugin.Options) (*source, []string, error) {
 		Comment:      *flagAddComment,
 	}
 
-	for _, s := range *flagBase {
-		if *s != "" {
-			source.Base = append(source.Base, *s)
-		}
+	if err := source.addBaseProfiles(*flagBase, *flagDiffBase); err != nil {
+		return nil, nil, err
 	}
 
 	normalize := pprofVariables["normalize"].boolValue()
 	if normalize && len(source.Base) == 0 {
-		return nil, nil, fmt.Errorf("Must have base profile to normalize by")
+		return nil, nil, errors.New("must have base profile to normalize by")
 	}
 	source.Normalize = normalize
 
@@ -158,6 +159,34 @@ func parseFlags(o *plugin.Options) (*source, []string, error) {
 	return source, cmd, nil
 }
 
+// addBaseProfiles adds the list of base profiles or diff base profiles to
+// the source. This function will return an error if both base and diff base
+// profiles are specified.
+func (source *source) addBaseProfiles(flagBase, flagDiffBase []*string) error {
+	base, diffBase := dropEmpty(flagBase), dropEmpty(flagDiffBase)
+	if len(base) > 0 && len(diffBase) > 0 {
+		return errors.New("-base and -diff_base flags cannot both be specified")
+	}
+
+	source.Base = base
+	if len(diffBase) > 0 {
+		source.Base, source.DiffBase = diffBase, true
+	}
+	return nil
+}
+
+// dropEmpty list takes a slice of string pointers, and outputs a slice of
+// non-empty strings associated with the flag.
+func dropEmpty(list []*string) []string {
+	var l []string
+	for _, s := range list {
+		if *s != "" {
+			l = append(l, *s)
+		}
+	}
+	return l
+}
+
 // installFlags creates command line flags for pprof variables.
 func installFlags(flag plugin.FlagSet) flagsInstalled {
 	f := flagsInstalled{
@@ -240,15 +269,15 @@ func outputFormat(bcmd map[string]*bool, acmd map[string]*string) (cmd []string,
 	for n, b := range bcmd {
 		if *b {
 			if cmd != nil {
-				return nil, fmt.Errorf("must set at most one output format")
+				return nil, errors.New("must set at most one output format")
 			}
 			cmd = []string{n}
 		}
 	}
 	for n, s := range acmd {
 		if *s != "" {
 			if cmd != nil {
-				return nil, fmt.Errorf("must set at most one output format")
+				return nil, errors.New("must set at most one output format")
 			}
 			cmd = []string{n, *s}
 		}

internal/driver/driver_test.go

@@ -288,7 +288,7 @@ type testFlags struct {
 	floats      map[string]float64
 	strings     map[string]string
 	args        []string
-	stringLists map[string][]*string
+	stringLists map[string][]string
 }
 
 func (testFlags) ExtraUsage() string { return "" }
@@ -355,7 +355,12 @@ func (f testFlags) StringVar(p *string, s, d, c string) {
 
 func (f testFlags) StringList(s, d, c string) *[]*string {
 	if t, ok := f.stringLists[s]; ok {
-		return &t
+		// convert slice of strings to slice of string pointers before returning.
+		tp := make([]*string, len(t))
+		for i, v := range t {
+			tp[i] = &v
+		}
+		return &tp
 	}
 	return &[]*string{}
 }

internal/driver/fetch.go

@@ -63,6 +63,9 @@ func fetchProfiles(s *source, o *plugin.Options) (*profile.Profile, error) {
 	}
 
 	if pbase != nil {
+		if s.DiffBase {
+			pbase.SetLabel("pprof::base", []string{"true"})
+		}
 		if s.Normalize {
 			err := p.Normalize(pbase)
 			if err != nil {