From c488ae77b6415ab2ae91e8556a30f43f24420937 Mon Sep 17 00:00:00 2001 From: Brandon Duffany Date: Mon, 28 Aug 2023 11:28:42 -0400 Subject: [PATCH] Fix double-gzipped timing profile (#4652) --- app/invocation/BUILD | 2 -- app/invocation/invocation_timing_card.tsx | 24 +++++++++---------- app/service/rpc_service.ts | 29 ++++++++++++++++++++--- package.json | 2 -- server/http/interceptors/interceptors.go | 9 +++++++ yarn.lock | 10 -------- 6 files changed, 46 insertions(+), 30 deletions(-) diff --git a/app/invocation/BUILD b/app/invocation/BUILD index 71001e619f2..7e30a879dce 100644 --- a/app/invocation/BUILD +++ b/app/invocation/BUILD @@ -580,10 +580,8 @@ ts_library( "//app/invocation:invocation_suggestion_card", "//app/service:rpc_service", "//proto:build_event_stream_ts_proto", - "@npm//@types/pako", "@npm//@types/react", "@npm//lucide-react", - "@npm//pako", "@npm//react", "@npm//tslib", ], diff --git a/app/invocation/invocation_timing_card.tsx b/app/invocation/invocation_timing_card.tsx index cfb147fc967..7ca9a218cc3 100644 --- a/app/invocation/invocation_timing_card.tsx +++ b/app/invocation/invocation_timing_card.tsx @@ -1,9 +1,8 @@ -import pako from "pako"; import React from "react"; import SetupCodeComponent from "../docs/setup_code"; import FlameChart from "../flame_chart/flame_chart"; import { Profile, parseProfile } from "../flame_chart/profile_model"; -import rpcService from "../service/rpc_service"; +import rpcService, { FileEncoding } from "../service/rpc_service"; import InvocationModel from "./invocation_model"; import Button from "../components/button/button"; import { Clock } from "lucide-react"; @@ -87,21 +86,20 @@ export default class InvocationTimingCardComponent extends React.Component { - if (isGzipped) { - contents = parseProfile(pako.inflate(contents, { to: "string" })); - } - this.updateProfile(contents); - }) + .fetchBytestreamFile(profileFile.uri, this.props.model.getInvocationId(), "text", { storedEncoding }) + .then((contents: string) => this.updateProfile(parseProfile(contents))) .catch((e) => errorService.handleError(e)) .finally(() => this.setState({ loading: false })); } diff --git a/app/service/rpc_service.ts b/app/service/rpc_service.ts index 43d5a2cbe2c..04960511dd2 100644 --- a/app/service/rpc_service.ts +++ b/app/service/rpc_service.ts @@ -21,6 +21,10 @@ type ExtendedBuildBuddyService = CancelableService; +export type FileEncoding = "gzip" | ""; + +export type XMLHttpResponseType = XMLHttpRequest["responseType"]; + class RpcService { service: ExtendedBuildBuddyService; events: Subject; @@ -78,15 +82,29 @@ class RpcService { window.open(this.getBytestreamUrl(bytestreamURL, invocationId, { filename, zip })); } + /** + * Fetches a bytestream resource. + * + * If the resource is known to already be stored in compressed form, + * storedEncoding can be specified to prevent the server from + * double-compressing (since it gzips all resources by default). + */ fetchBytestreamFile( bytestreamURL: string, invocationId: string, - responseType?: "arraybuffer" | "json" | "text" | undefined + responseType?: XMLHttpResponseType, + { storedEncoding }: { storedEncoding?: FileEncoding } = {} ) { - return this.fetchFile(this.getBytestreamUrl(bytestreamURL, invocationId), responseType || ""); + return this.fetchFile(this.getBytestreamUrl(bytestreamURL, invocationId), responseType || "", { + storedEncoding: storedEncoding, + }); } - fetchFile(fileURL: string, responseType: "arraybuffer" | "json" | "text" | ""): Promise { + fetchFile( + fileURL: string, + responseType: XMLHttpResponseType, + { storedEncoding }: { storedEncoding?: FileEncoding } = {} + ): Promise { return new Promise((resolve, reject) => { var request = new XMLHttpRequest(); request.responseType = responseType; @@ -107,6 +125,11 @@ class RpcService { request.onerror = function () { reject("Error loading file (unknown error)"); }; + // If we know the stored content is already gzipped, inform the server so + // that it doesn't double-gzip. + if (storedEncoding === "gzip") { + request.setRequestHeader("X-Stored-Encoding-Hint", "gzip"); + } request.send(); }); } diff --git a/package.json b/package.json index 0cb02faabdc..361da450a0c 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,6 @@ "@types/long": "^4.0.1", "@types/moment": "^2.13.0", "@types/node": "^12.0.2", - "@types/pako": "^1.0.1", "@types/react": "^16.8.17", "@types/react-date-range": "^1.1.6", "@types/react-dom": "^16.8.4", @@ -51,7 +50,6 @@ "moment": "^2.29.4", "monaco-editor": "^0.25.2", "oidc-client": "^1.10.1", - "pako": "^1.0.11", "path-browserify": "^1.0.1", "prettier": "^2.1.1", "protobufjs": "^7.2.4", diff --git a/server/http/interceptors/interceptors.go b/server/http/interceptors/interceptors.go index 12fec8c570d..2fee9c37a1d 100644 --- a/server/http/interceptors/interceptors.go +++ b/server/http/interceptors/interceptors.go @@ -94,6 +94,15 @@ func Gzip(next http.Handler) http.Handler { // is not also set. w.Header().Set("Transfer-Encoding", "chunked") + // If the client is telling us that the stored payload is already + // gzipped, make sure we don't double-gzip. Note that we still set the + // gzip headers above so that the browser can automatically decompress + // the response. + if r.Header.Get("X-Stored-Encoding-Hint") == "gzip" { + next.ServeHTTP(w, r) + return + } + gz := gzPool.Get().(*gzip.Writer) defer gzPool.Put(gz) diff --git a/yarn.lock b/yarn.lock index d9d06095ed4..58934400e93 100644 --- a/yarn.lock +++ b/yarn.lock @@ -284,11 +284,6 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-12.20.4.tgz#73687043dd00fcb6962c60fbf499553a24d6bdf2" integrity sha512-xRCgeE0Q4pT5UZ189TJ3SpYuX/QGl6QIAOAIeDSbAVAd2gX1NxSZup4jNVK7cxIeP8KDSbJgcckun495isP1jQ== -"@types/pako@^1.0.1": - version "1.0.1" - resolved "https://registry.yarnpkg.com/@types/pako/-/pako-1.0.1.tgz#33b237f3c9aff44d0f82fe63acffa4a365ef4a61" - integrity sha512-GdZbRSJ3Cv5fiwT6I0SQ3ckeN2PWNqxd26W9Z2fCK1tGrrasGy4puvNFtnddqH9UJFMQYXxEuuB7B8UK+LLwSg== - "@types/parse-json@^4.0.0": version "4.0.0" resolved "https://registry.yarnpkg.com/@types/parse-json/-/parse-json-4.0.0.tgz#2f8bb441434d163b35fb8ffdccd7138927ffb8c0" @@ -1417,11 +1412,6 @@ p-try@^2.0.0: resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.2.0.tgz#cb2868540e313d61de58fafbe35ce9004d5540e6" integrity sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ== -pako@^1.0.11: - version "1.0.11" - resolved "https://registry.yarnpkg.com/pako/-/pako-1.0.11.tgz#6c9599d340d54dfd3946380252a35705a6b992bf" - integrity sha512-4hLB8Py4zZce5s4yd9XzopqwVv/yGNhV1Bl8NTmCq1763HeK2+EwVTv+leGeL13Dnh2wfbqowVPXCIO0z4taYw== - parent-module@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/parent-module/-/parent-module-1.0.1.tgz#691d2709e78c79fae3a156622452d00762caaaa2"