Improvements to commits page - Pre-load all commits on the server. This takes about 2 minutes. - Add "load more" functionality to the commits page. - Fix bug for the case when a branch head is not in the commits window. BUG=skia: Review URL: https://codereview.chromium.org/784103004
diff --git a/monitoring/go/alertserver/main.go b/monitoring/go/alertserver/main.go index a8a9c8c..680ee6b 100644 --- a/monitoring/go/alertserver/main.go +++ b/monitoring/go/alertserver/main.go
@@ -14,6 +14,7 @@ "os/user" "path/filepath" "runtime" + "strconv" "strings" "time" ) @@ -33,12 +34,14 @@ "skia.googlesource.com/buildbot.git/go/skiaversion" "skia.googlesource.com/buildbot.git/go/util" "skia.googlesource.com/buildbot.git/monitoring/go/alerting" + "skia.googlesource.com/buildbot.git/monitoring/go/commit_cache" ) const ( COOKIESALT_METADATA_KEY = "cookiesalt" CLIENT_ID_METADATA_KEY = "client_id" CLIENT_SECRET_METADATA_KEY = "client_secret" + DEFAULT_COMMITS_TO_LOAD = 35 INFLUXDB_NAME_METADATA_KEY = "influxdb_name" INFLUXDB_PASSWORD_METADATA_KEY = "influxdb_password" GMAIL_CLIENT_ID_METADATA_KEY = "gmail_clientid" @@ -48,8 +51,9 @@ ) var ( - alertManager *alerting.AlertManager = nil - gitInfo *gitinfo.GitInfo = nil + alertManager *alerting.AlertManager = nil + gitInfo *gitinfo.GitInfo = nil + commitCache *commit_cache.CommitCache = nil ) // flags @@ -77,6 +81,19 @@ return false } +func getIntParam(name string, r *http.Request) (*int, error) { + raw, ok := r.URL.Query()[name] + if !ok { + return nil, nil + } + v64, err := strconv.ParseInt(raw[0], 10, 32) + if err != nil { + return nil, fmt.Errorf("Invalid value for parameter %q: %s -- %v", name, raw, err) + } + v32 := int(v64) + return &v32, nil +} + func alertJsonHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") type displayAlert struct { @@ -174,31 +191,40 @@ func commitsJsonHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - gitInfo.Update(true, true) - commitHashes := gitInfo.From(time.Now().AddDate(0, 0, -45)) - branchHeads, err := gitInfo.GetBranches() + // Case 1: Requesting specific commit range by index. + startIdx, err := getIntParam("start", r) if err != nil { - util.ReportError(w, r, err, fmt.Sprintf("Failed to read branch information from the repo: %s", err)) + util.ReportError(w, r, err, fmt.Sprintf("Invalid parameter: %v", err)) return } - commits := make([]*gitinfo.LongCommit, len(commitHashes)) - for i, h := range commitHashes { - c, err := gitInfo.Details(h) + if startIdx != nil { + endIdx := commitCache.NumCommits() + end, err := getIntParam("end", r) if err != nil { - util.ReportError(w, r, err, fmt.Sprintf("Failed to obtain commit details for %s: %s", h, err)) + util.ReportError(w, r, err, fmt.Sprintf("Invalid parameter: %v", err)) return } - commits[i] = c + if end != nil { + endIdx = *end + } + if err := commitCache.RangeAsJson(w, *startIdx, endIdx); err != nil { + util.ReportError(w, r, err, fmt.Sprintf("Failed to load commit range from cache: %v", err)) + return + } + return } - data := struct { - Commits []*gitinfo.LongCommit `json:"commits"` - BranchHeads []*gitinfo.GitBranch `json:"branch_heads"` - }{ - Commits: commits, - BranchHeads: branchHeads, + // Case 2: Requesting N (or the default number) commits. + commitsToLoad := DEFAULT_COMMITS_TO_LOAD + n, err := getIntParam("n", r) + if err != nil { + util.ReportError(w, r, err, fmt.Sprintf("Invalid parameter: %v", err)) + return } - if err := json.NewEncoder(w).Encode(data); err != nil { - util.ReportError(w, r, err, fmt.Sprintf("Failed to encode commit data as JSON: %s", err)) + if n != nil { + commitsToLoad = *n + } + if err := commitCache.LastNAsJson(w, commitsToLoad); err != nil { + util.ReportError(w, r, err, fmt.Sprintf("Failed to load commits from cache: %v", err)) return } } @@ -308,5 +334,16 @@ if err != nil { glog.Fatalf("Failed to check out Skia: %v", err) } + commitCache, err = commit_cache.New(gitInfo) + if err != nil { + glog.Fatalf("Failed to create commit cache: %v", err) + } + go func() { + for _ = range time.Tick(time.Minute) { + if err := commitCache.Update(); err != nil { + glog.Errorf("Failed to update commit cache: %v", err) + } + } + }() runServer(serverURL) }
diff --git a/monitoring/go/commit_cache/commit_cache.go b/monitoring/go/commit_cache/commit_cache.go new file mode 100644 index 0000000..47805ba --- /dev/null +++ b/monitoring/go/commit_cache/commit_cache.go
@@ -0,0 +1,126 @@ +package commit_cache + +import ( + "encoding/json" + "fmt" + "io" + "sync" + "time" + + "github.com/golang/glog" + + "skia.googlesource.com/buildbot.git/go/gitinfo" +) + +/* + Utilities for caching commit data. +*/ + +// CommitCache is a struct used for caching commit data. Stores ALL commits in +// the repository. +type CommitCache struct { + branchHeads []*gitinfo.GitBranch + commits []*gitinfo.LongCommit + repo *gitinfo.GitInfo + mutex sync.RWMutex +} + +// New creates and returns a new CommitCache which watches the given repo. +// The initial Update will load ALL commits from the repository, so expect +// this to be slow. +func New(repo *gitinfo.GitInfo) (*CommitCache, error) { + // Initially load the past week's worth of commits. + c := CommitCache{ + repo: repo, + } + if err := c.Update(); err != nil { + return nil, err + } + return &c, nil +} + +// Update syncs the source code repository and loads any new commits. +func (c *CommitCache) Update() error { + glog.Info("Reloading commits.") + c.mutex.Lock() + defer c.mutex.Unlock() + if err := c.repo.Update(true, true); err != nil { + return fmt.Errorf("Failed to update the repo: %v", err) + } + from := time.Time{} + if len(c.commits) > 0 { + from = c.commits[len(c.commits)-1].Timestamp + } + newCommitHashes := c.repo.From(from) + glog.Infof("Processing %d new commits.", len(newCommitHashes)) + newCommits := make([]*gitinfo.LongCommit, len(newCommitHashes)) + if len(newCommitHashes) > 0 { + for i, h := range newCommitHashes { + d, err := c.repo.Details(h) + if err != nil { + return fmt.Errorf("Failed to obtain commit details for %s: %v", h, err) + } + newCommits[i] = d + } + } + branchHeads, err := c.repo.GetBranches() + if err != nil { + return fmt.Errorf("Failed to read branch information from the repo: %v", err) + } + // Update the cached values all at once at at the end. + c.branchHeads = branchHeads + c.commits = append(c.commits, newCommits...) + return nil +} + +// NumCommits returns the number of commits contained in the cache. +func (c *CommitCache) NumCommits() int { + return len(c.commits) +} + +// asJson writes the given commit range along with the branch heads in JSON +// format to the given Writer. +func (c *CommitCache) asJson(w io.Writer, startIdx, endIdx int) error { + c.mutex.RLock() + defer c.mutex.RUnlock() + data := struct { + Commits []*gitinfo.LongCommit `json:"commits"` + BranchHeads []*gitinfo.GitBranch `json:"branch_heads"` + StartIdx int `json:"startIdx"` + EndIdx int `json:"endIdx"` + }{ + Commits: c.commits[startIdx:endIdx], + BranchHeads: c.branchHeads, + StartIdx: startIdx, + EndIdx: endIdx, + } + return json.NewEncoder(w).Encode(&data) +} + +// LastNAsJson writes the last N commits along with the branch heads in JSON +// format to the given Writer. +func (c *CommitCache) LastNAsJson(w io.Writer, n int) error { + c.mutex.RLock() + end := len(c.commits) + c.mutex.RUnlock() + start := end - n + if start < 0 { + start = 0 + } + return c.asJson(w, start, end) +} + +// RangeAsJson writes the given range of commits along with the branch heads +// in JSON format to the given Writer. +func (c *CommitCache) RangeAsJson(w io.Writer, startIdx, endIdx int) error { + if startIdx < 0 || startIdx > len(c.commits) { + return fmt.Errorf("startIdx is out of range [0, %d]: %d", len(c.commits), startIdx) + } + if endIdx < 0 || endIdx > len(c.commits) { + return fmt.Errorf("endIdx is out of range [0, %d]: %d", len(c.commits), endIdx) + } + if endIdx < startIdx { + return fmt.Errorf("endIdx < startIdx: %d, %d", endIdx, startIdx) + } + return c.asJson(w, startIdx, endIdx) +}
diff --git a/monitoring/res/imp/commits-sk.html b/monitoring/res/imp/commits-sk.html index 3d2262b..337832d 100644 --- a/monitoring/res/imp/commits-sk.html +++ b/monitoring/res/imp/commits-sk.html
@@ -38,6 +38,10 @@ a { color: inherit; } + #moreButton { + width: 40px; + height: 40px; + } </style> <div vertical layout flex> <div horizontal layout center id="loadstatus"> @@ -55,7 +59,7 @@ <table class="commitList"> <template repeat="{{commit in commits}}"> <tr class="commit" style="color: {{getCommitColor(displayCommits[commit.hash])}}"> - <td><a href="https://skia.googlesource.com/skiabot-test/+/{{commit.hash}}" target="_blank">{{commit.hash|shortCommit}}</a></td> + <td><a href="https://skia.googlesource.com/skia/+/{{commit.hash}}" target="_blank">{{commit.hash|shortCommit}}</a></td> <td>{{commit.author|shortAuthor}}</td> <td>{{commit.subject|shortSubject}}</td> </tr> @@ -63,15 +67,18 @@ </table> </div> </div> + <!-- TODO(borenet): Automatically loadMore when the user scrolls to the bottom? --> + <core-icon-button id="moreButton" icon="add" on-click="{{loadMore}}"></core-icon-button> </div> </template> <script> (function() { - var commitY = 20; // Vertical pixels used by each commit. - var paddingX = 10; // Left-side padding pixels. - var paddingY = 20; // Top padding pixels. - var radius = 3; // Radius of commit dots. - var columnWidth = commitY; // Pixel width of per-branch colums. + var defaultCommitsToLoad = 35; // Default number of commits to load. + var commitY = 20; // Vertical pixels used by each commit. + var paddingX = 10; // Left-side padding pixels. + var paddingY = 20; // Top padding pixels. + var radius = 3; // Radius of commit dots. + var columnWidth = commitY; // Pixel width of per-branch colums. // Colors used for the branches. Obtained from // http://blog.mollietaylor.com/2012/10/color-blindness-and-palette-choice.html var palette = [ @@ -282,7 +289,12 @@ for (var b = 0; b < branches.length; b++) { // Add a label to commits at branch heads. var hash = branches[b].head - displayCommits[branches[b].head].label.push(branches[b].name); + // The branch might have scrolled out of the time window. If so, just + // skip it. + if (!displayCommits[hash]) { + continue + } + displayCommits[hash].label.push(branches[b].name); if (traceCommits(displayCommits, commits, remaining, hash, column)) { column++; } @@ -308,7 +320,7 @@ value: null, reflect: true, }, - dispayCommits: { + displayCommits: { value: null, reflect: true, }, @@ -329,10 +341,12 @@ created: function() { this.commits = []; this.branchHeads = []; + this.startIdx = null; + this.endIdx = null; this.reloadCommits(); var that = this; window.addEventListener("resize", function() { - that.draw(); + that.draw(that.commits, that.branchHeads); }, true); }, @@ -376,28 +390,98 @@ } if (this.reload > 0) { var that = this; - this.timeout = window.setTimeout(function() { that.reloadCommits(); }, this.reload * 1000); + this.timeout = window.setTimeout(function() { + that.reloadCommits(that.endIdx); + }, this.reload * 1000); } }, - reloadCommits: function() { + loadMore: function() { + this.reloadCommits(this.startIdx - defaultCommitsToLoad, this.startIdx); + }, + + // Reload the commits. If the startIdx and endIdx parameters are given, + // loads the commits in that range. If not, load the most recent N + // commits, where N is the default number returned by the server. + reloadCommits: function(startIdx, endIdx) { console.log("Loading commits."); + if (this.$) { + this.$.moreButton.disabled = true; + } + var url = "/json/commits"; + if (startIdx) { + url += "?start=" + startIdx; + if (endIdx) { + url += "&end=" + endIdx; + } + } + console.log("GET " + url); var that = this; - sk.get("/json/commits").then(JSON.parse).then(function(json) { - that.commits = json.commits; - that.commits.reverse(); - that.branchHeads = json.branch_heads; - that.lastLoaded = new Date().toLocaleTimeString(); - that.resetTimeout(); - console.log("Done loading commits."); - that.draw(); + sk.get(url).then(JSON.parse).then(function(json) { + try { + json.commits.reverse(); + that.lastLoaded = new Date().toLocaleTimeString(); + + // Merge the new commits into the existing set. + // Ensure that the new commits line up exactly with the existing ones. + if (json.endIdx - json.startIdx != json.commits.length) { + console.error("Server returned invalid number of commits."); + return; + } + var commits = null; + // Case 1: Loading initial set of commits. + if (!that.startIdx || !that.endIdx) { + commits = json.commits; + that.startIdx = json.startIdx; + that.endIdx = json.endIdx; + } + // Case 2: Loading earlier commits. + else if (json.startIdx < that.startIdx) { + if (json.endIdx != that.startIdx) { + console.error("Server returned invalid set of commits."); + return; + } + commits = that.commits.concat(json.commits); + that.startIdx = json.startIdx; + } + // Case 3: Loading newer commits. + else if (json.endIdx >= that.endIdx) { + if (json.startIdx != that.endIdx) { + console.error("Server returned invalid set of commits."); + return; + } + if (json.commits.length == 0) { + console.log("No new commits. Skipping draw."); + return; + } + commits = json.commits.concat(that.commits); + that.endIdx = json.endIdx; + } + // ??? + else { + console.error("Server returned invalid data."); + return; + } + // Actually draw the commits. + that.draw(commits, json.branch_heads); + that.commits = commits; + that.branchHeads = json.branch_heads; + } catch(e) { + console.error(e.stack); + return; + } finally { + that.resetTimeout(); + if (that.$) { + that.$.moreButton.disabled = false; + } + } }); }, - draw: function() { + draw: function(commits, branchHeads) { console.log("Drawing."); // Initialize all commits. - var prep = prepareCommitsForDisplay(this.commits, this.branchHeads); + var prep = prepareCommitsForDisplay(commits, branchHeads); this.displayCommits = prep[0]; var numColumns = prep[1]; @@ -408,8 +492,8 @@ var dummyCtx = document.createElement("canvas").getContext("2d"); dummyCtx.font = font; var longestWidth = 0; - for (var i = 0; i < this.commits.length; i++) { - var c = this.displayCommits[this.commits[i].hash]; + for (var i = 0; i < commits.length; i++) { + var c = this.displayCommits[commits[i].hash]; var w = c.labelWidth(dummyCtx); w += commitY * (c.column + 1); if (w > longestWidth) { @@ -422,7 +506,7 @@ var parent = this.shadowRoot.getElementById("canvasContainer"); var canvas = this.shadowRoot.getElementById("commitCanvas"); var w = longestWidth + paddingX; - var h = commitY * this.commits.length; + var h = commitY * commits.length; canvas.style.width = w + "px"; canvas.style.height = h + "px"; canvas.width = w * scale; @@ -433,13 +517,13 @@ ctx.font = font; // Shade an alternating background. - for (var i = 0; i < this.commits.length; i++) { - this.displayCommits[this.commits[i].hash].drawBackground(ctx); + for (var i = 0; i < commits.length; i++) { + this.displayCommits[commits[i].hash].drawBackground(ctx); } // Draw the commits. - for (var i = 0; i < this.commits.length; i++) { - this.displayCommits[this.commits[i].hash].draw(ctx, this.displayCommits); + for (var i = 0; i < commits.length; i++) { + this.displayCommits[commits[i].hash].draw(ctx, this.displayCommits); } }, });