From c48bf3a34b765f5beabe8d79656df8f2d6cca800 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Mon, 15 Apr 2019 15:59:11 -0500 Subject: [PATCH] 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 --- tools/fm/fm_bot.go | 94 ++++++++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 49 deletions(-) diff --git a/tools/fm/fm_bot.go b/tools/fm/fm_bot.go index 9c1b69bcbc..3c1e069c09 100644 --- a/tools/fm/fm_bot.go +++ b/tools/fm/fm_bot.go @@ -63,30 +63,13 @@ func listAllGMs(fm string) (gms []string, err error) { 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 @@ func sourcesAndFlags(args []string, gms []string) ([]string, []string, error) { 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 @@ func sourcesAndFlags(args []string, gms []string) ([]string, []string, error) { // 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 @@ func sourcesAndFlags(args []string, gms []string) ([]string, []string, error) { } f += parts[0] - flags = append(flags, f, parts[1]) + w.Flags = append(w.Flags, f, parts[1]) continue } @@ -124,7 +107,7 @@ func sourcesAndFlags(args []string, gms []string) ([]string, []string, error) { 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 @@ func sourcesAndFlags(args []string, gms []string) ([]string, []string, error) { // 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 @@ func main() { } } - 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 @@ func main() { } } } + + 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 @@ func main() { 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 @@ func main() { // 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 @@ func main() { } 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} } }