sweet: simplify diagnostics config handling

Currently, diagnostics track and pass around a potentially different
results directory for each diagnostic type. In practice, these are all
always the same directory.

Trying to keep track of potentially different per-type directories is
going to get cumbersome in some upcoming changes, so this CL
simplifies all of this by replacing the per-type directory
configuration with a single results directory.

This necessarily changes the way we pass this configuration to the
benchmark drivers. We take advantage of this opportunity to clean that
up as well. Now, the entire driver diagnostics configuration is
represented by a single diagnostics.DriverConfig type, and this type
can be "serialized" to and "deserialized" from command line flags, as
symmetric operations.

Change-Id: Ifbeb7a5d456dfcd6b6211dfb2b870f41bb7c5c70
Reviewed-on: https://go-review.googlesource.com/c/benchmarks/+/600058
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

sweet/benchmarks/internal/driver/driver.go

diff --git a/sweet/benchmarks/internal/driver/driver.go b/sweet/benchmarks/internal/driver/driver.go
index 093e014..0f8890a 100644
--- a/sweet/benchmarks/internal/driver/driver.go
+++ b/sweet/benchmarks/internal/driver/driver.go
@@ -27,12 +27,12 @@
 
 var (
 	coreDumpDir string
-	diag        map[diagnostics.Type]*diagnostics.DriverConfig
+	diag        diagnostics.DriverConfig
 )
 
 func SetFlags(f *flag.FlagSet) {
 	f.StringVar(&coreDumpDir, "dump-cores", "", "dump a core file to the given directory after every benchmark run")
-	diag = diagnostics.SetFlagsForDriver(f)
+	diag.AddFlags(f)
 }
 
 const (
@@ -422,9 +422,7 @@
 		panic("perf process already started")
 	}
 	args := []string{"record", "-o", b.diagnostics[diagnostics.Perf].Name(), "-p", strconv.Itoa(b.pid)}
-	if perfFlags := diag[diagnostics.Perf].Flags; perfFlags != "" {
-		args = append(args, strings.Split(perfFlags, " ")...)
-	}
+	args = append(args, PerfFlags()...)
 	cmd := exec.Command("perf", args...)
 	cmd.Stderr = os.Stderr
 	if err := cmd.Start(); err != nil {
@@ -556,11 +554,8 @@
 }
 
 func DiagnosticEnabled(typ diagnostics.Type) bool {
-	cfg, ok := diag[typ]
-	if !ok {
-		panic("bad profile type")
-	}
-	return cfg.Dir != ""
+	_, ok := diag.ConfigSet.Get(typ)
+	return ok
 }
 
 func WritePprofProfile(prof *profile.Profile, typ diagnostics.Type, pattern string) error {
@@ -594,16 +589,16 @@
 }
 
 func PerfFlags() []string {
-	if !DiagnosticEnabled(diagnostics.Perf) {
+	cfg, ok := diag.ConfigSet.Get(diagnostics.Perf)
+	if !ok {
 		panic("perf not enabled")
 	}
-	return strings.Split(diag[diagnostics.Perf].Flags, " ")
+	return strings.Split(cfg.Flags, " ")
 }
 
 func newDiagnosticDataFile(typ diagnostics.Type, pattern string) (*os.File, error) {
-	cfg, ok := diag[typ]
-	if !ok || cfg.Dir == "" {
+	if !DiagnosticEnabled(typ) {
 		return nil, fmt.Errorf("this type of profile is not currently enabled")
 	}
-	return os.CreateTemp(cfg.Dir, pattern+"."+string(typ))
+	return os.CreateTemp(diag.ResultsDir, pattern+"."+string(typ))
 }

sweet/cmd/sweet/benchmark.go

diff --git a/sweet/cmd/sweet/benchmark.go b/sweet/cmd/sweet/benchmark.go
index 23cd8f1..7df6bf3 100644
--- a/sweet/cmd/sweet/benchmark.go
+++ b/sweet/cmd/sweet/benchmark.go
@@ -16,6 +16,7 @@
 	"strings"
 
 	"golang.org/x/benchmarks/sweet/common"
+	"golang.org/x/benchmarks/sweet/common/diagnostics"
 	"golang.org/x/benchmarks/sweet/common/fileutil"
 	"golang.org/x/benchmarks/sweet/common/log"
 	"golang.org/x/benchmarks/sweet/generators"
@@ -339,9 +340,8 @@
 
 			// We need to pass arguments to the benchmark binary to generate
 			// profiles. See benchmarks/internal/driver for details.
-			for _, d := range cfg.Diagnostics.ToSlice() {
-				args = append(args, d.DriverArgs(resultsProfilesDir)...)
-			}
+			dc := diagnostics.DriverConfig{ResultsDir: resultsProfilesDir, ConfigSet: cfg.Diagnostics}
+			args = append(args, dc.DriverArgs()...)
 		}
 
 		results, err := os.Create(filepath.Join(resultsDir, fmt.Sprintf("%s.results", cfg.Name)))

sweet/common/diagnostics/config.go

diff --git a/sweet/common/diagnostics/config.go b/sweet/common/diagnostics/config.go
index c3739f8..944d2fa 100644
--- a/sweet/common/diagnostics/config.go
+++ b/sweet/common/diagnostics/config.go
@@ -78,15 +78,6 @@
 	return len(c.cfgs) == 0
 }
 
-// ToSlice returns each Config contained in the ConfigSet in a slice.
-func (c ConfigSet) ToSlice() []Config {
-	cfgs := make([]Config, 0, len(c.cfgs))
-	for _, cfg := range c.cfgs {
-		cfgs = append(cfgs, cfg)
-	}
-	return cfgs
-}
-
 // Type is a diagnostic type supported by Sweet.
 type Type string
 
@@ -102,11 +93,6 @@
 	return t == CPUProfile || t == MemProfile
 }
 
-// AsFlag returns the Type suitable for use as a CLI flag.
-func (t Type) AsFlag() string {
-	return "-" + string(t)
-}
-
 // Types returns a slice of all supported types.
 func Types() []Type {
 	return []Type{

sweet/common/diagnostics/driver.go

diff --git a/sweet/common/diagnostics/driver.go b/sweet/common/diagnostics/driver.go
index ff692f0..a3bbafb 100644
--- a/sweet/common/diagnostics/driver.go
+++ b/sweet/common/diagnostics/driver.go
@@ -9,32 +9,45 @@
 	"fmt"
 )
 
+// DriverConfig is a diagnostics configuration that can be passed to a benchmark
+// driver by serializing to and from command-line flags.
+type DriverConfig struct {
+	ConfigSet
+	ResultsDir string
+}
+
 // DriverArgs returns the arguments that should be passed to a Sweet benchmark
-// binary to collect data for the Config.
-func (d Config) DriverArgs(resultsDir string) []string {
-	flag := d.Type.AsFlag()
-	args := []string{flag, resultsDir}
-	if d.Flags != "" {
-		args = append(args, flag+"-flags", d.Flags)
+// binary to collect data for c.
+func (c *DriverConfig) DriverArgs() []string {
+	args := []string{"-results-dir", c.ResultsDir}
+	for _, c1 := range c.cfgs {
+		args = append(args, "-"+string(c1.Type))
+		if c1.Type == Perf {
+			// String flag
+			args = append(args, c1.Flags)
+		}
 	}
 	return args
 }
 
-type DriverConfig struct {
-	Config
-	Dir string
-}
+// AddFlags populates f with flags that will fill in c.
+func (c *DriverConfig) AddFlags(f *flag.FlagSet) {
+	*c = DriverConfig{}
+	c.ConfigSet.cfgs = make(map[Type]Config)
 
-func SetFlagsForDriver(f *flag.FlagSet) map[Type]*DriverConfig {
-	storage := make(map[Type]*DriverConfig)
+	f.StringVar(&c.ResultsDir, "results-dir", "", "directory to write diagnostics data")
 	for _, t := range Types() {
-		dc := new(DriverConfig)
-		dc.Type = t
-		storage[t] = dc
-		f.StringVar(&dc.Dir, string(t), "", fmt.Sprintf("directory to write %s data", t))
+		t := t
 		if t == Perf {
-			f.StringVar(&dc.Flags, string(t)+"-flags", "", "flags for Linux perf")
+			f.Func(string(t), fmt.Sprintf("enable %s diagnostics with `flags`", t), func(s string) error {
+				c.cfgs[t] = Config{Type: t, Flags: s}
+				return nil
+			})
+		} else {
+			f.BoolFunc(string(t), fmt.Sprintf("enable %s diagnostics", t), func(s string) error {
+				c.cfgs[t] = Config{Type: t}
+				return nil
+			})
 		}
 	}
-	return storage
 }