[codesize] Add RPC endpoints for retrieving binary size diffs.
Bug: skia:13535
Change-Id: I83edaf03dde267e97c70bbaba95eb617b8bda524
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/562486
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
diff --git a/codesize/Makefile b/codesize/Makefile
index 0674c5f..444b44c 100644
--- a/codesize/Makefile
+++ b/codesize/Makefile
@@ -16,4 +16,4 @@
$(BAZEL) run //codesize:push_codesizeserver
push: release
- $(BAZEL) run //kube/go/pushk -- codesizeserver
\ No newline at end of file
+ $(BAZEL) run //kube/go/pushk -- codesizeserver
diff --git a/codesize/go/codesizeserver/main.go b/codesize/go/codesizeserver/main.go
index d81ece1..c94cd51 100644
--- a/codesize/go/codesizeserver/main.go
+++ b/codesize/go/codesizeserver/main.go
@@ -213,8 +213,8 @@
// handleFileUploadNotification is called when a new file is uploaded to the GCS bucket.
func (s *server) handleFileUploadNotification(ctx context.Context, path string) error {
sklog.Infof("Received file upload PubSub message: %s", path)
- if strings.HasSuffix(path, ".json") {
- sklog.Infof("Ignoring %s because we index .json files when we see a corresponding .tsv file", path)
+ if strings.HasSuffix(path, ".json") || strings.HasSuffix(path, ".diff.txt") {
+ sklog.Infof("Ignoring %s because we index .json and .diff.txt files when we see a corresponding .tsv file", path)
return nil
}
if err := s.store.Index(ctx, path); err != nil {
@@ -290,6 +290,32 @@
sendJSONResponse(res, w)
}
+func (s *server) binarySizeDiffRPCHandler(w http.ResponseWriter, r *http.Request) {
+ req := rpc.BinarySizeDiffRPCRequest{}
+ if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+ httputils.ReportError(w, err, "Failed to parse request", http.StatusBadRequest)
+ return
+ }
+
+ binary, ok := s.store.GetBinary(req.CommitOrPatchset, req.BinaryName, req.CompileTaskName)
+ if !ok {
+ httputils.ReportError(w, nil, "Binary not found in Store", http.StatusNotFound)
+ return
+ }
+
+ bytes, err := s.store.GetBloatySizeDiffOutputFileContents(r.Context(), binary)
+ if err != nil {
+ httputils.ReportError(w, err, "Failed to retrieve Bloaty output file", http.StatusInternalServerError)
+ return
+ }
+
+ res := rpc.BinarySizeDiffRPCResponse{
+ Metadata: binary.Metadata,
+ RawDiff: string(bytes),
+ }
+ sendJSONResponse(res, w)
+}
+
func (s *server) mostRecentBinariesRPCHandler(w http.ResponseWriter, r *http.Request) {
binaries := s.store.GetMostRecentBinaries(numMostRecentBinaries)
res := rpc.MostRecentBinariesRPCResponse{
@@ -303,6 +329,7 @@
r.HandleFunc("/", s.indexPageHandler).Methods("GET")
r.HandleFunc("/binary", s.binaryPageHandler).Methods("GET")
r.HandleFunc("/rpc/binary/v1", s.binaryRPCHandler).Methods("POST")
+ r.HandleFunc("/rpc/binary_size_diff/v1", s.binarySizeDiffRPCHandler).Methods("POST")
r.HandleFunc("/rpc/most_recent_binaries/v1", s.mostRecentBinariesRPCHandler).Methods("GET")
}
diff --git a/codesize/go/codesizeserver/rpc/types.go b/codesize/go/codesizeserver/rpc/types.go
index d9b5183..946f8cd 100644
--- a/codesize/go/codesizeserver/rpc/types.go
+++ b/codesize/go/codesizeserver/rpc/types.go
@@ -20,6 +20,19 @@
Rows []bloaty.TreeMapDataTableRow `json:"rows" go2ts:"ignorenil"`
}
+type BinarySizeDiffRPCRequest struct {
+ store.CommitOrPatchset
+ BinaryName string `json:"binary_name"`
+ CompileTaskName string `json:"compile_task_name"`
+}
+
+type BinarySizeDiffRPCResponse struct {
+ Metadata common.BloatyOutputMetadata `json:"metadata"`
+
+ // RawDiff is the verbatim Bloaty size diff output in plain-text format.
+ RawDiff string `json:"raw_diff"`
+}
+
type MostRecentBinariesRPCResponse struct {
Binaries []store.BinariesFromCommitOrPatchset `json:"binaries"`
}
diff --git a/codesize/go/common/metadata.go b/codesize/go/common/metadata.go
index b5d0598..915ac49 100644
--- a/codesize/go/common/metadata.go
+++ b/codesize/go/common/metadata.go
@@ -17,10 +17,14 @@
TaskID string `json:"task_id"`
TaskName string `json:"task_name"`
CompileTaskName string `json:"compile_task_name"`
- BinaryName string `json:"binary_name"`
+ // CompileTaskNameNoPatch should only be set for tryjobs.
+ CompileTaskNameNoPatch string `json:"compile_task_name_no_patch,omitempty"`
+ BinaryName string `json:"binary_name"`
BloatyCipdVersion string `json:"bloaty_cipd_version"`
BloatyArgs []string `json:"bloaty_args"`
+ // BloatyDiffArgs should only be set for tryjobs.
+ BloatyDiffArgs []string `json:"bloaty_diff_args,omitempty"`
PatchIssue string `json:"patch_issue"`
PatchServer string `json:"patch_server"`
diff --git a/codesize/go/store/store.go b/codesize/go/store/store.go
index 2e76eee..bff2760 100644
--- a/codesize/go/store/store.go
+++ b/codesize/go/store/store.go
@@ -30,8 +30,9 @@
// Binary represents a single binary that has been analyzed with Bloaty.
type Binary struct {
- Metadata common.BloatyOutputMetadata `json:"metadata"`
- BloatyOutputFileGCSPath string `json:"-"`
+ Metadata common.BloatyOutputMetadata `json:"metadata"`
+ BloatyOutputFileGCSPath string `json:"-"`
+ BloatySizeDiffOutputFileGCSPath string `json:"-"`
// Timestamp should reflect the "timestamp" field in the JSON metadata.
Timestamp time.Time `json:"-"`
@@ -107,11 +108,18 @@
return skerr.Wrap(err)
}
+ // If it exists, the Bloaty size diff output file should be found at this location.
+ bloatySizeDiffOutputFileGCSPath := ""
+ if len(metadata.BloatyDiffArgs) != 0 {
+ bloatySizeDiffOutputFileGCSPath = basePath + ".diff.txt"
+ }
+
// Add a new entry to the index.
binary := Binary{
- Metadata: metadata,
- BloatyOutputFileGCSPath: bloatyOutputFileGCSPath,
- Timestamp: timestamp,
+ Metadata: metadata,
+ BloatyOutputFileGCSPath: bloatyOutputFileGCSPath,
+ BloatySizeDiffOutputFileGCSPath: bloatySizeDiffOutputFileGCSPath,
+ Timestamp: timestamp,
}
commitOrPatchset := CommitOrPatchset{}
if metadata.PatchIssue != "" || metadata.PatchSet != "" {
@@ -166,18 +174,28 @@
// GetBloatyOutputFileContents downloads and returns the raw contents of a Bloaty output file.
func (s *Store) GetBloatyOutputFileContents(ctx context.Context, binary Binary) ([]byte, error) {
+ return s.downloadAndCache(ctx, binary.BloatyOutputFileGCSPath)
+}
+
+// GetBloatySizeDiffOutputFileContents downloads and returns the raw contents of a Bloaty size diff
+// output file.
+func (s *Store) GetBloatySizeDiffOutputFileContents(ctx context.Context, binary Binary) ([]byte, error) {
+ return s.downloadAndCache(ctx, binary.BloatySizeDiffOutputFileGCSPath)
+}
+
+func (s *Store) downloadAndCache(ctx context.Context, path string) ([]byte, error) {
// Read the file from the cache.
- if bytes, ok := s.gcsCache.Get(binary.BloatyOutputFileGCSPath); ok {
+ if bytes, ok := s.gcsCache.Get(path); ok {
return bytes.([]byte), nil
}
// Download the file if it wasn't cached.
- bytes, err := s.downloadFn(ctx, binary.BloatyOutputFileGCSPath)
+ bytes, err := s.downloadFn(ctx, path)
if err != nil {
return nil, skerr.Wrap(err)
}
// Cache the downloaded file and return its contents.
- s.gcsCache.Add(binary.BloatyOutputFileGCSPath, bytes)
+ s.gcsCache.Add(path, bytes)
return bytes, nil
}
diff --git a/codesize/go/store/store_test.go b/codesize/go/store/store_test.go
index b95dcbf..ebb471c 100644
--- a/codesize/go/store/store_test.go
+++ b/codesize/go/store/store_test.go
@@ -136,12 +136,14 @@
Timestamp: "2022-02-16T14:34:25Z",
CompileTaskName: "Build-Foo",
BinaryName: "dm",
+ BloatyDiffArgs: []string{"build/dm", "--", "build_nopatch/dm"},
PatchIssue: "509137",
PatchSet: "11",
Revision: "commit3",
},
- BloatyOutputFileGCSPath: "issue9876/patchset3/job1/Build-Foo/dm.tsv",
- Timestamp: time.Date(2022, time.February, 16, 14, 34, 25, 0, time.UTC),
+ BloatyOutputFileGCSPath: "issue9876/patchset3/job1/Build-Foo/dm.tsv",
+ BloatySizeDiffOutputFileGCSPath: "issue9876/patchset3/job1/Build-Foo/dm.diff.txt",
+ Timestamp: time.Date(2022, time.February, 16, 14, 34, 25, 0, time.UTC),
},
{
Metadata: common.BloatyOutputMetadata{
@@ -349,6 +351,52 @@
assert.Equal(t, "Fake Bloaty output 6", string(bytes))
}
+func TestStore_GetBloatySizeDiffOutputFileContents_NonExistentBinary_Error(t *testing.T) {
+ unittest.SmallTest(t)
+ store := newStoreForTesting()
+
+ _, err := store.GetBloatySizeDiffOutputFileContents(context.Background(), Binary{
+ BloatySizeDiffOutputFileGCSPath: "no-such-file.diff.txt",
+ })
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "not found: no-such-file.diff.txt")
+}
+
+func TestStore_GetBloatySizeDiffOutputFileContents_Success(t *testing.T) {
+ unittest.SmallTest(t)
+ store := newStoreForTesting()
+
+ // Store is initially empty.
+ assert.Empty(t, store.GetMostRecentBinaries(10))
+
+ // Index a bunch of binaries.
+ require.NoError(t, store.Index(context.Background(), "commit1/Build-Foo/dm.tsv"))
+ require.NoError(t, store.Index(context.Background(), "commit1/Build-Foo/fm.tsv"))
+ require.NoError(t, store.Index(context.Background(), "commit1/Build-Bar/dm.tsv"))
+ require.NoError(t, store.Index(context.Background(), "commit2/Build-Foo/dm.tsv"))
+ require.NoError(t, store.Index(context.Background(), "issue9876/patchset3/job1/Build-Foo/dm.tsv"))
+ require.NoError(t, store.Index(context.Background(), "issue9876/patchset3/job2/Build-Bar/fm.tsv"))
+
+ // Binary from a tryjob.
+ bytes, err := store.GetBloatySizeDiffOutputFileContents(context.Background(), Binary{
+ Metadata: common.BloatyOutputMetadata{
+ Version: 1,
+ Timestamp: "2022-02-16T14:34:25Z",
+ CompileTaskName: "Build-Foo",
+ BinaryName: "dm",
+ BloatyDiffArgs: []string{"build/dm", "--", "build_nopatch/dm"},
+ PatchIssue: "509137",
+ PatchSet: "11",
+ Revision: "commit3",
+ },
+ BloatyOutputFileGCSPath: "issue9876/patchset3/job1/Build-Foo/dm.tsv",
+ BloatySizeDiffOutputFileGCSPath: "issue9876/patchset3/job1/Build-Foo/dm.diff.txt",
+ Timestamp: time.Date(2022, time.February, 16, 14, 34, 25, 0, time.UTC),
+ })
+ require.NoError(t, err)
+ assert.Equal(t, "Fake Bloaty diff", string(bytes))
+}
+
// newStoreForTesting returns a Store that "downloads" files from the fake GCS bucket defined by
// the fakeGCSBucket map below.
func newStoreForTesting() Store {
@@ -427,14 +475,20 @@
"timestamp": "2022-02-16T14:34:25Z",
"compile_task_name": "Build-Foo",
"binary_name": "dm",
+ "bloaty_diff_args": [
+ "build/dm",
+ "--",
+ "build_nopatch/dm"
+ ],
"patch_issue": "509137",
"patch_set": "11",
"revision": "commit3"
}
`,
- "issue9876/patchset3/job1/Build-Foo/dm.tsv": "Fake Bloaty output 5",
+ "issue9876/patchset3/job1/Build-Foo/dm.tsv": "Fake Bloaty output 5",
+ "issue9876/patchset3/job1/Build-Foo/dm.diff.txt": "Fake Bloaty diff",
- // Same tryjob as previous file, different compile task and binary.
+ // Same tryjob as previous file, different compile task and binary, missing size diff.
"issue9876/patchset3/job2/Build-Bar/fm.json": `
{
"version": 1,
diff --git a/codesize/go/ts/main.go b/codesize/go/ts/main.go
index 2c1e661..f0564ce 100644
--- a/codesize/go/ts/main.go
+++ b/codesize/go/ts/main.go
@@ -19,6 +19,8 @@
generator := go2ts.New()
generator.Add(rpc.BinaryRPCRequest{})
generator.Add(rpc.BinaryRPCResponse{})
+ generator.Add(rpc.BinarySizeDiffRPCRequest{})
+ generator.Add(rpc.BinarySizeDiffRPCResponse{})
generator.Add(rpc.MostRecentBinariesRPCResponse{})
err := util.WithWriteFile(*outputPath, func(w io.Writer) error {
diff --git a/codesize/modules/rpc_types.ts b/codesize/modules/rpc_types.ts
index 9349cab..85399f4 100644
--- a/codesize/modules/rpc_types.ts
+++ b/codesize/modules/rpc_types.ts
@@ -16,9 +16,11 @@
task_id: string;
task_name: string;
compile_task_name: string;
+ compile_task_name_no_patch?: string;
binary_name: string;
bloaty_cipd_version: string;
bloaty_args: string[] | null;
+ bloaty_diff_args?: string[] | null;
patch_issue: string;
patch_server: string;
patch_set: string;
@@ -40,6 +42,19 @@
rows: TreeMapDataTableRow[];
}
+export interface BinarySizeDiffRPCRequest {
+ binary_name: string;
+ compile_task_name: string;
+ commit: string;
+ patch_issue: string;
+ patch_set: string;
+}
+
+export interface BinarySizeDiffRPCResponse {
+ metadata: BloatyOutputMetadata;
+ raw_diff: string;
+}
+
export interface Binary {
metadata: BloatyOutputMetadata;
}
