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>
This commit is contained in:
Mike Klein 2019-04-15 15:59:11 -05:00
parent 4aa13e7da3
commit c48bf3a34b

View File

@ -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}
}
}