just some refactoring for fm_bot.go
A couple little spots starting to look unwieldy.
Change-Id: If2971b71ae202b152f54ec3df6896d906c34a081
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/208276
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/tools/fm/fm_bot.go b/tools/fm/fm_bot.go
index 9c1b69b..3c1e069 100644
--- a/tools/fm/fm_bot.go
+++ b/tools/fm/fm_bot.go
@@ -63,30 +63,13 @@
return
}
-func callFM(fm string, sources []string, flags []string) bool {
- start := time.Now()
-
- args := flags[:]
- args = append(args, "-s")
- args = append(args, sources...)
-
- cmd := exec.Command(fm, args...)
- output, err := cmd.CombinedOutput()
-
- if err != nil {
- if !*quiet || len(sources) == 1 {
- log.Printf("\n%v #failed (%v):\n%s\n", strings.Join(cmd.Args, " "), err, output)
- }
- return false
- } else if !*quiet {
- log.Printf("\n%v #done in %v:\n%s", strings.Join(cmd.Args, " "), time.Since(start), output)
- }
- return true
+type work struct {
+ Sources []string
+ Flags []string
}
-func sourcesAndFlags(args []string, gms []string) ([]string, []string, error) {
- sources := []string{}
- flags := []string{}
+func parseWork(args []string, gms []string) (*work, error) {
+ w := &work{}
for _, arg := range args {
// I wish we could parse flags here too, but it's too late.
if strings.HasPrefix(arg, "-") {
@@ -94,7 +77,7 @@
if flag.Lookup(arg[1:]) != nil {
msg = "Please pass fm_bot flags like '%s' on the command line before the FM binary."
}
- return nil, nil, fmt.Errorf(msg, arg)
+ return nil, fmt.Errorf(msg, arg)
}
// Everything after a # is a comment.
@@ -104,7 +87,7 @@
// Treat "gm" or "gms" as a shortcut for all known GMs.
if arg == "gm" || arg == "gms" {
- sources = append(sources, gms...)
+ w.Sources = append(w.Sources, gms...)
continue
}
@@ -116,7 +99,7 @@
}
f += parts[0]
- flags = append(flags, f, parts[1])
+ w.Flags = append(w.Flags, f, parts[1])
continue
}
@@ -124,7 +107,7 @@
matchedAnyGM := false
for _, gm := range gms {
if (*exact && gm == arg) || (!*exact && strings.Contains(gm, arg)) {
- sources = append(sources, gm)
+ w.Sources = append(w.Sources, gm)
matchedAnyGM = true
}
}
@@ -136,30 +119,25 @@
// Not all shells expand globs, so we'll do it here just in case.
matches, err := filepath.Glob(arg)
if err != nil {
- return nil, nil, err
+ return nil, err
}
if len(matches) == 0 {
- return nil, nil, fmt.Errorf("Don't understand '%s'.", arg)
+ return nil, fmt.Errorf("Don't understand '%s'.", arg)
}
for _, match := range matches {
err := filepath.Walk(match, func(path string, info os.FileInfo, err error) error {
if !info.IsDir() {
- sources = append(sources, path)
+ w.Sources = append(w.Sources, path)
}
return err
})
if err != nil {
- return nil, nil, err
+ return nil, err
}
}
}
- return sources, flags, nil
-}
-
-type work struct {
- Sources []string
- Flags []string
+ return w, nil
}
func main() {
@@ -197,13 +175,24 @@
}
}
-
wg := &sync.WaitGroup{}
var failures int32 = 0
worker := func(queue chan work) {
for w := range queue {
- if !callFM(fm, w.Sources, w.Flags) {
+ start := time.Now()
+
+ args := w.Flags[:]
+ args = append(args, "-s")
+ args = append(args, w.Sources...)
+
+ cmd := exec.Command(fm, args...)
+ output, err := cmd.CombinedOutput()
+
+ status := "#done"
+ if err != nil {
+ status = fmt.Sprintf("#failed (%v)", err)
+
if len(w.Sources) == 1 {
// If a source ran alone and failed, that's just a failure.
atomic.AddInt32(&failures, 1)
@@ -215,6 +204,12 @@
}
}
}
+
+ if !*quiet || (err != nil && len(w.Sources) == 1) {
+ log.Printf("\n%v %v in %v:\n%s",
+ strings.Join(cmd.Args, " "), status, time.Since(start), output)
+ }
+
wg.Done()
}
}
@@ -234,7 +229,8 @@
if len(job) == 0 {
continue
}
- sources, flags, err := sourcesAndFlags(job, gms)
+
+ w, err := parseWork(job, gms)
if err != nil {
log.Fatal(err)
}
@@ -242,9 +238,9 @@
// Determine if this is CPU-bound or GPU-bound work, conservatively assuming GPU.
queue, limit := gpu, *gpuLimit
backend := ""
- for i, flag := range flags {
+ for i, flag := range w.Flags {
if flag == "-b" || flag == "--backend" {
- backend = flags[i+1]
+ backend = w.Flags[i+1]
}
}
whitelisted := map[string]bool{
@@ -257,23 +253,23 @@
}
if *random {
- rand.Shuffle(len(sources), func(i, j int) {
- sources[i], sources[j] = sources[j], sources[i]
+ rand.Shuffle(len(w.Sources), func(i, j int) {
+ w.Sources[i], w.Sources[j] = w.Sources[j], w.Sources[i]
})
}
// Round up so there's at least one source per batch.
- sourcesPerBatch := (len(sources) + limit - 1) / limit
+ sourcesPerBatch := (len(w.Sources) + limit - 1) / limit
- for i := 0; i < len(sources); i += sourcesPerBatch {
+ for i := 0; i < len(w.Sources); i += sourcesPerBatch {
end := i + sourcesPerBatch
- if end > len(sources) {
- end = len(sources)
+ if end > len(w.Sources) {
+ end = len(w.Sources)
}
- batch := sources[i:end]
+ batch := w.Sources[i:end]
wg.Add(1)
- queue <- work{batch, flags}
+ queue <- work{batch, w.Flags}
}
}
