Adds tile handler to the perf server

perf/server/src/server/perf.go

Index: perf/server/src/server/perf.go
diff --git a/perf/server/src/server/perf.go b/perf/server/src/server/perf.go
index 69a237f1c2e00b0ae8b2bfc5a73139bb2fcde3c9..18579401596ace7497febd561e20f64ba286ad67 100644
--- a/perf/server/src/server/perf.go
+++ b/perf/server/src/server/perf.go
@@ -18,6 +18,7 @@ import (
 	"regexp"
 	"sort"
 	"strconv"
+        "strings"
 	"time"
 )
 
@@ -30,6 +31,8 @@ import (
 import (
 	"config"
 	"db"
+        "filetilestore"
+        "types"
 )
 
 var (
@@ -44,6 +47,8 @@ var (
 	clustersHandlerPath = regexp.MustCompile(`/clusters/([a-z]*)$`)
 
 	shortcutHandlerPath = regexp.MustCompile(`/shortcuts/([0-9]*)$`)
+
+	tileHandlerPath = regexp.MustCompile(`/tiles/([a-z]*)/([0-9]*)/([-0-9]*)$`)

[jcgregorio, 2014/07/11 19:03:55]

Since there's more than one parameter here use named captures for the parts:
(?P<name>re)

[kelvinly, 2014/07/11 19:42:29]

Would that serve any purpose other than documenting the groups? I can't seem to
find a way to use the captured group names, aside from the cumbersome-looking
regexp.Expand()

[jcgregorio, 2014/07/11 20:06:56]

Oh yeah, the regexp package doesn't handle those very well.
Never mind, just add docs to this variable on what each part means.


On 2014/07/11 19:42:29, kelvinly wrote:

[kelvinly, 2014/07/11 20:17:10]

Done.

 )
 
 // flags
@@ -56,6 +61,8 @@ var (
 
 var (
 	data *Data
+
+        tileStores map[string]types.TileStore
 )
 
 const (
@@ -83,6 +90,11 @@ func Init() {
 
 	indexTemplate = template.Must(template.ParseFiles(filepath.Join(cwd, "templates/index.html")))
 	clusterTemplate = template.Must(template.ParseFiles(filepath.Join(cwd, "templates/clusters.html")))
+
+        tileStores = make(map[string]types.TileStore)
+        for _, name := range config.ALL_DATASET_NAMES {
+            tileStores[string(name)] = filetilestore.NewFileTileStore(*tileDir, string(name))
+        }
 }
 
 // reportError formats an HTTP error response and also logs the detailed error message.
@@ -240,6 +252,159 @@ func annotationsHandler(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func tileHandler(w http.ResponseWriter, r *http.Request) {

[jcgregorio, 2014/07/11 19:03:55]

Add documentation to this function on the format of the URLs it accepts with an
example or two, including what the responses would look like.

[kelvinly, 2014/07/11 19:42:29]

Done.

+        // So this almost doubles the size of this file. Should I move it into its own package?
+        glog.Infof("Tile Handler: %q\n", r.URL.Path)
+        match := tileHandlerPath.FindStringSubmatch(r.URL.Path)
+        if match == nil || len(match) != 4 {
+                http.NotFound(w, r)
+                return
+        }
+        if r.Method == "GET" {

[jcgregorio, 2014/07/11 19:03:55]

Since we will only accept GET on this and to keep the indentation to a minimum
do

if r.Method != "GET" {
   http.NotFound(w, r)
   return
}

[kelvinly, 2014/07/11 19:42:29]

Done.

+                dataset := match[1]
+                tileScale, err := strconv.ParseInt(match[2], 10, 0)
+                if err != nil {
+                        reportError(w, r, fmt.Errorf("Error parsing tile scale: %s", err), "Failed to return traces.")

[jcgregorio, 2014/07/11 19:03:56]

reportError(w, r, err, "Failed parsing tile scale.")

Since reportError already combines err and message in the logs.

Here and below.

[kelvinly, 2014/07/11 19:42:29]

Done.

+                }
+                tileNumber, err := strconv.ParseInt(match[3], 10, 0)
+                if err != nil {
+                        reportError(w, r, fmt.Errorf("Error parsing tile number: %s", err), "Failed to return traces.")
+                        return
+                }
+                // TODO: Use some sort of cache
+                var tile *types.Tile

[jcgregorio, 2014/07/11 19:03:55]

factor out the following 11 lines of code as its own function:

   getTile(dataset, tileScale, tileNumber)

[kelvinly, 2014/07/11 19:42:30]

Done.

+                tileStore, ok := tileStores[dataset]
+                if !ok {
+                        reportError(w, r, fmt.Errorf("Unable to access dataset store for %s", err), "Failed to return traces.")
+                        return
+                }
+                tile, err = tileStore.Get(int(tileScale), int(tileNumber))
+                if err != nil || tile == nil {
+                        reportError(w, r, fmt.Errorf("Unable to get tile from tilestore: ", err), "Failed to return traces.")
+                        return
+                }
+                tracesRequested := strings.Split(r.FormValue("traces"), ",")
+                noCommits := len(r.FormValue("omit_commits")) > 0

[jcgregorio, 2014/07/11 19:03:56]

keep naming consistent, and reverse the logic:

  includeCommits := r.FormValue("include_commits") == "true"

Same for the next two lines.

[kelvinly, 2014/07/11 19:42:29]

Including seems like a good default, so I'm keeping the logic as is, but I'll
rename to be more consistent

+                noParams := len(r.FormValue("omit_params")) > 0
+                noNames := len(r.FormValue("omit_names")) > 0
+                result := types.NewGUITile(int(tileScale), int(tileNumber))
+                paramList, ok := config.KEY_PARAM_ORDER[dataset]
+                if !ok {
+                        reportError(w, r, fmt.Errorf("Unable to read parameter list for dataset: ", err), "Failed to return traces.")
+                        return
+                }
+                for _, keyName := range tracesRequested {
+                        var rawTrace *types.Trace
+                        count := 0
+                        // Unpack trace name and find the trace.
+                        keyParts := strings.Split(keyName, ":")
+                        for _, tileTrace := range tile.Traces {
+                                tracesMatch := true
+                                for i, keyPart := range keyParts {
+                                        if len(keyPart) > 0 {
+                                                if traceParam, exists := tileTrace.Params[paramList[i]]; !exists || traceParam != keyPart {
+                                                        tracesMatch = false
+                                                        break
+                                                }
+                                        // If it doesn't exist in the key, it should also not exist in
+                                        // the trace parameters
+                                        } else if traceParam, exists := tileTrace.Params[paramList[i]]; exists && len(traceParam) <= 0 {
+                                                tracesMatch = false
+                                                break
+                                        }
+                                }
+                                if tracesMatch {
+                                        rawTrace = tileTrace
+                                        // NOTE: Not breaking out of the loop
+                                        // for now to see if there are multiple
+                                        // traces that match any given trace
+                                        count += 1
+                                }
+                        }
+                        // No matches
+                        if count <= 0 || rawTrace == nil {
+                                continue
+                        } else {
+                                glog.Infof("%d matches found for %s\n", count, keyName)

[jcgregorio, 2014/07/11 19:03:55]

Log this as a warning because it indicated a data integrity problem.

[kelvinly, 2014/07/11 19:42:29]

Done. Also changed it to trigger only when count > 1

+                        }
+                        newTraceData := make([][2]float64, 0)
+                        for i, traceVal := range rawTrace.Values {
+                                if traceVal < 1e99 {

[jcgregorio, 2014/07/11 19:03:56]

if traceVal != config.MISSING_DATA_SENTINEL {

[kelvinly, 2014/07/11 19:42:29]

Using < instead of !=

[jcgregorio, 2014/07/11 20:06:56]

Use !=, because that's exactly what is stored there:

 
https://github.com/google/skia-buildbot/blob/master/perf/server/src/server/da...

On 2014/07/11 19:42:29, kelvinly wrote:

[kelvinly, 2014/07/11 20:17:10]

Done. Sorry, I'm a bit wary of using any form of equality on floats in general

+                                        newTraceData = append(newTraceData, [2]float64 {
+                                                traceVal,
+                                                float64(tile.Commits[i].CommitTime),
+                                                // We should have 53 significand bits, so this should work correctly basically forever
+                                        })
+                                }
+                        }
+                        if len(newTraceData) > 0 {
+                                result.Traces = append(result.Traces, types.TraceGUI {
+                                        Data:   newTraceData,
+                                        Key:    keyName,
+                                });
+                        }
+                }
+                if !noCommits {
+                        result.Commits = tile.Commits
+                }
+                if !noNames {
+                        for _, trace := range tile.Traces {
+                                newKey := make([]string, len(paramList))

[jcgregorio, 2014/07/11 19:03:56]

factor out into its own function:

   makeKeyFromParams(paramList, trace.Params)

[kelvinly, 2014/07/11 19:42:29]

Done.

+                                for i, paramName := range paramList {
+                                        if name, ok := trace.Params[paramName]; ok {
+                                                newKey[i] = name
+                                        } else {
+                                                newKey[i] = ""
+                                        }
+                                }
+                                result.NameList = append(result.NameList, strings.Join(newKey, ":"))
+                        }
+                }
+                if !noParams {
+                        // NOTE: When constructing ParamSet, we need to make sure there are empty strings 
+                        // where there's at least one key missing that parameter
+                        // TODO: Fix this on tile generation rather than here
+                        result.ParamSet = make([][]string, len(paramList))
+                        for i := range result.ParamSet {
+                                if readableName, ok := config.HUMAN_READABLE_PARAM_NAMES[paramList[i]]; !ok {
+                                        reportError(w, r, fmt.Errorf("%s does not exist in the readable parameter names list", paramList[i]), "Unable to send traces.")

[jcgregorio, 2014/07/11 19:03:55]

Does this have to be an error, how about falling back to using the raw param
name but logging it as a warning?

[kelvinly, 2014/07/11 19:42:29]

Done.

+                                        return
+                                } else {
+                                        result.ParamSet[i] = append(make([]string, 0), readableName)

[jcgregorio, 2014/07/11 19:03:55]

result.ParamSet[i] = []string{readableName}

[kelvinly, 2014/07/11 19:42:29]

Done.

+                                }
+                        }
+                        for _, trace := range tile.Traces {
+                                for i := range result.ParamSet {
+                                        traceValue, ok := trace.Params[paramList[i]];
+                                        if !ok {
+                                                traceValue = ""
+                                        }
+                                        traceValueIsInParamSet := false
+                                        for _, param := range []string(result.ParamSet[i]) {
+                                                if param == traceValue {
+                                                        traceValueIsInParamSet = true
+                                                }
+                                        }
+                                        if !traceValueIsInParamSet {
+                                                result.ParamSet[i] = append(result.ParamSet[i], traceValue)
+                                        }
+                                }
+                        }
+                }
+                // Marshal and send
+                marshaledResult, err := json.Marshal(result)
+                if err != nil {
+                        reportError(w, r, fmt.Errorf("Failed to marshal JSON: ", err), "Unable to return traces.");
+                        return
+                }
+                w.Header().Set("Content-Type", "application/json")
+                _, err = w.Write(marshaledResult)
+                if err != nil {
+                        reportError(w, r, err, "Error while marshalling results.");
+                }
+        }
+}
+
 // jsonHandler handles the GET for the JSON requests.
 func jsonHandler(w http.ResponseWriter, r *http.Request) {
 	glog.Infof("JSON Handler: %q\n", r.URL.Path)
@@ -290,6 +455,7 @@ func main() {
 	http.HandleFunc("/", autogzip.HandleFunc(mainHandler))
 	http.HandleFunc("/json/", jsonHandler) // We pre-gzip this ourselves.
 	http.HandleFunc("/shortcuts/", shortcutHandler)
+        http.HandleFunc("/tiles/", tileHandler)
 	http.HandleFunc("/clusters/", autogzip.HandleFunc(clusterHandler))
 	http.HandleFunc("/annotations/", autogzip.HandleFunc(annotationsHandler))