track flags in a map
I got to thinking that seeing flags like,
out/fm --nonativeFonts -b cpu --nativeFonts -s ...
is kind of confusing, and I'm also trying to figure out
how to identify these runs to Gold. I think the answer
to both might be to track a map[string]string for flags,
allowing overrides rather than just appending, and then
that flag map ends up being the identifying properties.
Change-Id: Ie5f80ee8b145c205edc768ae871eb70a3e1bc5b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378355
Reviewed-by: Eric Boren <borenet@google.com>
diff --git a/infra/bots/task_drivers/fm_driver/fm_driver.go b/infra/bots/task_drivers/fm_driver/fm_driver.go
index c8bb92c..77c6fae 100644
--- a/infra/bots/task_drivers/fm_driver/fm_driver.go
+++ b/infra/bots/task_drivers/fm_driver/fm_driver.go
@@ -15,6 +15,7 @@
"os"
"path/filepath"
"runtime"
+ "sort"
"strings"
"sync"
"sync/atomic"
@@ -180,13 +181,41 @@
}()
}
- var worker func(context.Context, []string, []string) int
- worker = func(ctx context.Context, sources, flags []string) (failures int) {
+ // We'll pass `flag: value` as `--flag value` to FM.
+ // Such a short type name makes it easy to write out literals.
+ type F = map[string]string
+
+ flatten := func(flags F) (flat []string) {
+ // It's not strictly important that we sort, but it makes reading bot logs easier.
+ keys := []string{}
+ for k := range flags {
+ keys = append(keys, k)
+ }
+ sort.Strings(keys)
+
+ for _, k := range keys {
+ v := flags[k]
+
+ if v == "true" {
+ flat = append(flat, "--"+k)
+ } else if v == "false" {
+ flat = append(flat, "--no"+k)
+ } else if len(k) == 1 {
+ flat = append(flat, "-"+k, v)
+ } else {
+ flat = append(flat, "--"+k, v)
+ }
+ }
+ return
+ }
+
+ var worker func(context.Context, []string, F) int
+ worker = func(ctx context.Context, sources []string, flags F) (failures int) {
stdout := &bytes.Buffer{}
stderr := &bytes.Buffer{}
cmd := &exec.Command{Name: fm, Stdout: stdout, Stderr: stderr}
cmd.Args = append(cmd.Args, "-i", *resources)
- cmd.Args = append(cmd.Args, flags...)
+ cmd.Args = append(cmd.Args, flatten(flags)...)
cmd.Args = append(cmd.Args, "-s")
cmd.Args = append(cmd.Args, sources...)
@@ -264,7 +293,7 @@
WG *sync.WaitGroup
Failures *int32
Sources []string // Passed to FM -s: names of gms/tests, paths to images, .skps, etc.
- Flags []string // Other flags to pass to FM: --ct 565, --msaa 16, etc.
+ Flags F // Other flags to pass to FM: --ct 565, --msaa 16, etc.
}
queue := make(chan Work, 1<<20) // Arbitrarily huge buffer to avoid ever blocking.
@@ -292,7 +321,7 @@
pendingKickoffs := &sync.WaitGroup{}
var totalFailures int32 = 0
- kickoff := func(sources, flags []string) {
+ kickoff := func(sources []string, flags F) {
if len(sources) == 0 {
return // A blank or commented job line from -script or the command line.
}
@@ -308,7 +337,7 @@
// For organizational purposes, create a step representing this call to kickoff(),
// with each batch of sources nested inside.
ctx := startStep(ctx,
- td.Props(fmt.Sprintf("%s, %s…", strings.Join(flags, " "), sources[0])))
+ td.Props(fmt.Sprintf("%s, %s…", strings.Join(flatten(flags), " "), sources[0])))
pendingBatches := &sync.WaitGroup{}
failures := new(int32)
@@ -339,7 +368,8 @@
}
// Parse a job like "gms b=cpu ct=8888" into sources and flags for kickoff().
- parse := func(job []string) (sources, flags []string) {
+ parse := func(job []string) (sources []string, flags F) {
+ flags = make(F)
for _, token := range job {
// Everything after # is a comment.
if strings.HasPrefix(token, "#") {
@@ -354,13 +384,7 @@
// Is this a flag to pass through to FM?
if parts := strings.Split(token, "="); len(parts) == 2 {
- f := "-"
- if len(parts[0]) > 1 {
- f += "-"
- }
- f += parts[0]
-
- flags = append(flags, f, parts[1])
+ flags[parts[0]] = parts[1]
continue
}
@@ -398,13 +422,17 @@
OS, model, CPU_or_GPU := parts[1], parts[3], parts[4]
// Bots use portable fonts except where we explicitly opt-in to native fonts.
- defaultFlags := []string{"--nonativeFonts"}
+ defaultFlags := F{"nativeFonts": "false"}
- run := func(sources []string, extraFlags string) {
+ run := func(sources []string, extraFlags F) {
// Default then extra to allow overriding the defaults.
- flags := []string{}
- flags = append(flags, defaultFlags...)
- flags = append(flags, strings.Fields(extraFlags)...)
+ flags := F{}
+ for k, v := range defaultFlags {
+ flags[k] = v
+ }
+ for k, v := range extraFlags {
+ flags[k] = v
+ }
kickoff(sources, flags)
}
@@ -430,34 +458,38 @@
if strings.Contains(*bot, "TSAN") {
// Run each test a few times in parallel to uncover races.
- defaultFlags = append(defaultFlags, "--race", "4")
+ defaultFlags["race"] = "4"
}
if CPU_or_GPU == "CPU" {
- defaultFlags = append(defaultFlags, "-b", "cpu")
+ defaultFlags["b"] = "cpu"
// FM's default ct/gamut/tf flags are equivalent to --config srgb in DM.
- run(gms, "")
- run(gms, "--nativeFonts")
- run(imgs, "")
- run(svgs, "")
- run(skps, "--clipW 1000 --clipH 1000")
- run(tests, "--race 0") // Several unit tests are not reentrant.
+ run(gms, F{})
+ run(gms, F{"nativeFonts": "true"})
+ run(imgs, F{})
+ run(svgs, F{})
+ run(skps, F{"clipW": "1000", "clipH": "1000"})
+ run(tests, F{"race": "0"}) // Several unit tests are not reentrant.
if model == "GCE" {
- run(gms, "--ct g8 --legacy") // --config g8
- run(gms, "--ct 565 --legacy") // --config 565
- run(gms, "--ct 8888 --legacy") // --config 8888.
- run(gms, "--ct f16") // --config esrgb
- run(gms, "--ct f16 --tf linear") // --config f16
- run(gms, "--ct 8888 --gamut p3") // --config p3
- run(gms, "--ct 8888 --gamut narrow --tf 2.2") // --config narrow
- run(gms, "--ct f16 --gamut rec2020 --tf rec2020") // --config erec2020
+ run(gms, F{"ct": "g8", "legacy": "true"}) // --config g8
+ run(gms, F{"ct": "565", "legacy": "true"}) // --config 565
+ run(gms, F{"ct": "8888", "legacy": "true"}) // --config 8888
+ run(gms, F{"ct": "f16"}) // --config esrgb
+ run(gms, F{"ct": "f16", "tf": "linear"}) // --config f16
+ run(gms, F{"ct": "8888", "gamut": "p3"}) // --config p3
+ run(gms, F{"ct": "8888", "gamut": "narrow", "tf": "2.2"}) // --config narrow
+ run(gms, F{"ct": "f16", "gamut": "rec2020", "tf": "rec2020"}) // --config erec2020
- run(gms, "--skvm")
- run(gms, "--skvm --ct f16")
+ run(gms, F{"skvm": "true"})
+ run(gms, F{"skvm": "true", "ct": "f16"})
- run(imgs, "--decodeToDst --ct f16 --gamut rec2020 --tf rec2020")
+ run(imgs, F{
+ "decodeToDst": "true",
+ "ct": "f16",
+ "gamut": "rec2020",
+ "tf": "rec2020"})
}
// TODO: pic-8888 equivalent?
