tools/perfmon: Don't stop on errors
Don't terminate on first error. Sleep a bit and try again.
Post a message to a gerrit change if it cannot be built. The fact the PS has a message from perfmon will prevent it from retrying the same change.
Remove trailing newlines from log.Printf() messages, they're automatically added.
Bug: tint:1383
Change-Id: I78a627c53c492e7da33a74470d5a064e90a7a753
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78783
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
diff --git a/tools/src/cmd/perfmon/main.go b/tools/src/cmd/perfmon/main.go
index 21f1057..f01fcff 100644
--- a/tools/src/cmd/perfmon/main.go
+++ b/tools/src/cmd/perfmon/main.go
@@ -112,50 +112,16 @@
}
for true {
- {
- log.Println("scanning for review changes to benchmark...")
- change, err := e.findGerritChangeToBenchmark()
- if err != nil {
- return err
- }
- if change != nil {
- if err := e.benchmarkGerritChange(*change); err != nil {
- return err
- }
- continue
- }
+ didSomething, err := e.doSomeWork()
+ if err != nil {
+ log.Printf("ERROR: %v", err)
+ time.Sleep(time.Minute * 10)
+ continue
}
-
- {
- log.Println("scanning for submitted changes to benchmark...")
- changesToBenchmark, err := e.changesToBenchmark()
- if err != nil {
- return err
- }
-
- if len(changesToBenchmark) > 0 {
- log.Printf("benchmarking %v changes...\n", len(changesToBenchmark))
- for i, c := range changesToBenchmark {
- log.Printf("benchmarking %v/%v....\n", i+1, len(changesToBenchmark))
- benchRes, err := e.benchmarkTintChange(c)
- if err != nil {
- return err
- }
- commitRes, err := e.benchmarksToCommitResults(c, *benchRes)
- if err != nil {
- return err
- }
- log.Printf("pushing results...\n")
- if err := e.pushUpdatedResults(*commitRes); err != nil {
- return err
- }
- }
- continue
- }
+ if !didSomething {
+ log.Println("nothing to do. Sleeping...")
+ time.Sleep(time.Minute * 5)
}
-
- log.Println("nothing to do. Sleeping...")
- time.Sleep(time.Minute * 5)
}
return nil
@@ -281,6 +247,53 @@
benchmarkCache map[git.Hash]*bench.Run
}
+// doSomeWork scans gerrit for changes up for review and submitted changes to
+// benchmark. If something was found to do, then returns true.
+func (e env) doSomeWork() (bool, error) {
+ {
+ log.Println("scanning for review changes to benchmark...")
+ change, err := e.findGerritChangeToBenchmark()
+ if err != nil {
+ return true, err
+ }
+ if change != nil {
+ if err := e.benchmarkGerritChange(*change); err != nil {
+ return true, err
+ }
+ return true, nil
+ }
+ }
+
+ {
+ log.Println("scanning for submitted changes to benchmark...")
+ changesToBenchmark, err := e.changesToBenchmark()
+ if err != nil {
+ return true, err
+ }
+
+ if len(changesToBenchmark) > 0 {
+ log.Printf("benchmarking %v changes...", len(changesToBenchmark))
+ for i, c := range changesToBenchmark {
+ log.Printf("benchmarking %v/%v....", i+1, len(changesToBenchmark))
+ benchRes, err := e.benchmarkTintChange(c)
+ if err != nil {
+ return true, err
+ }
+ commitRes, err := e.benchmarksToCommitResults(c, *benchRes)
+ if err != nil {
+ return true, err
+ }
+ log.Printf("pushing results...")
+ if err := e.pushUpdatedResults(*commitRes); err != nil {
+ return true, err
+ }
+ }
+ return true, nil
+ }
+ }
+ return false, nil
+}
+
// changesToBenchmark fetches the list of changes that do not currently have
// benchmark results, which should be benchmarked.
func (e env) changesToBenchmark() ([]git.Hash, error) {
@@ -319,11 +332,11 @@
// dependencies, builds tint, then runs the benchmarks, returning the results.
func (e env) benchmarkTintChange(hash git.Hash) (*bench.Run, error) {
if cached, ok := e.benchmarkCache[hash]; ok {
- log.Printf("reusing cached benchmark results of '%v'...\n", hash)
+ log.Printf("reusing cached benchmark results of '%v'...", hash)
return cached, nil
}
- log.Printf("checking out tint at '%v'...\n", hash)
+ log.Printf("checking out tint at '%v'...", hash)
if err := checkout(hash, e.tintRepo); err != nil {
return nil, err
}
@@ -345,6 +358,8 @@
return run, nil
}
+// benchmarksToCommitResults converts the benchmarks in the provided bench.Run
+// to a CommitResults.
func (e env) benchmarksToCommitResults(hash git.Hash, results bench.Run) (*CommitResults, error) {
commits, err := e.tintRepo.Log(&git.LogOptions{
From: hash.String(),
@@ -502,8 +517,7 @@
if !reflect.DeepEqual(res.System, e.system) {
log.Printf(`WARNING: results file '%v' has different system information!
File: %+v
-System: %+v
-`, path, res.System, e.system)
+System: %+v`, path, res.System, e.system)
}
return res, nil
@@ -546,14 +560,24 @@
"-DTINT_BUILD_WGSL_WRITER=1",
"-DTINT_BUILD_BENCHMARKS=1",
); err != nil {
- return fmt.Errorf("failed to generate tint build config:\n %w", err)
+ return errFailedToBuild{fmt.Errorf("failed to generate tint build config:\n %w", err)}
}
if _, err := call(tools.ninja, e.buildDir, e.cfg.Timeouts.Build); err != nil {
- return fmt.Errorf("failed to build tint:\n %w", err)
+ return errFailedToBuild{err}
}
return nil
}
+// errFailedToBuild is the error returned by buildTint() if the build failed
+type errFailedToBuild struct {
+ // The reason
+ reason error
+}
+
+func (e errFailedToBuild) Error() string {
+ return fmt.Sprintf("failed to build: %v", e.reason)
+}
+
// benchmarkTint runs the tint benchmarks, returning the results.
func (e env) benchmarkTint() (*bench.Run, error) {
exe := filepath.Join(e.buildDir, "tint-benchmark")
@@ -666,7 +690,7 @@
})
if len(candidates) > 0 {
- log.Printf("%d gerrit changes to benchmark\n", len(candidates))
+ log.Printf("%d gerrit changes to benchmark", len(candidates))
return &candidates[0].change, nil
}
return nil, nil
@@ -675,7 +699,7 @@
// benchmarks the gerrit change, posting the findings to the change
func (e env) benchmarkGerritChange(change gerrit.ChangeInfo) error {
current := change.Revisions[change.CurrentRevision]
- log.Printf("fetching '%v'...\n", current.Ref)
+ log.Printf("fetching '%v'...", current.Ref)
currentHash, err := e.tintRepo.Fetch(current.Ref, &git.FetchOptions{Auth: e.cfg.Tint.Auth})
if err != nil {
return err
@@ -685,8 +709,25 @@
if err != nil {
return fmt.Errorf("failed to parse parent hash '%v':\n %v", parent, err)
}
+
+ postMsg := func(notify, msg string) error {
+ _, _, err = e.gerrit.Changes.SetReview(change.ChangeID, currentHash.String(), &gerrit.ReviewInput{
+ Message: msg,
+ Tag: "autogenerated:perfmon",
+ Notify: notify,
+ })
+ if err != nil {
+ return fmt.Errorf("failed to post message to gerrit change:\n %v", err)
+ }
+ return nil
+ }
+
newRun, err := e.benchmarkTintChange(currentHash)
if err != nil {
+ var ftb errFailedToBuild
+ if errors.As(err, &ftb) {
+ return postMsg("OWNER", fmt.Sprintf("patchset %v failed to build", current.Number))
+ }
return err
}
if _, err := e.tintRepo.Fetch(parent, &git.FetchOptions{Auth: e.cfg.Tint.Auth}); err != nil {
@@ -735,15 +776,7 @@
if len(diff) > 0 {
notify = "OWNER_REVIEWERS"
}
- _, _, err = e.gerrit.Changes.SetReview(change.ChangeID, currentHash.String(), &gerrit.ReviewInput{
- Message: msg.String(),
- Tag: "autogenerated:perfmon",
- Notify: notify,
- })
- if err != nil {
- return fmt.Errorf("failed to post benchmark results to gerrit change:\n %v", err)
- }
- return nil
+ return postMsg(notify, msg.String())
}
// createOrOpenGitRepo creates a new local repo by cloning cfg.URL into