Skip to content

Commit

Permalink
Fix double-gzipped timing profile (#4652)
Browse files Browse the repository at this point in the history
  • Loading branch information
bduffany authored Aug 28, 2023
1 parent 75bb416 commit c488ae7
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 30 deletions.
2 changes: 0 additions & 2 deletions app/invocation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
24 changes: 11 additions & 13 deletions app/invocation/invocation_timing_card.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -87,21 +86,20 @@ export default class InvocationTimingCardComponent extends React.Component<Props
if (this.state.profile) return;

let profileFile = this.getProfileFile();
let compressionOption = this.props.model.optionsMap.get("json_trace_compression");
let isGzipped =
compressionOption === undefined ? (profileFile?.name ?? "").endsWith(".gz") : compressionOption == "1";

if (!profileFile?.uri) return;

let compressionOption = this.props.model.optionsMap.get("json_trace_compression");
let storedEncoding: FileEncoding = "";
if (compressionOption === "1" || (profileFile?.name ?? "").endsWith(".gz")) {
storedEncoding = "gzip";
}

this.setState({ loading: true });
// Note: we use responseType "text" instead of "json" since the profile is
// not always valid JSON (the trailing "]}" may be missing).
rpcService
.fetchBytestreamFile(profileFile.uri, this.props.model.getInvocationId(), isGzipped ? "arraybuffer" : "json")
.then((contents: any) => {
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 }));
}
Expand Down
29 changes: 26 additions & 3 deletions app/service/rpc_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ type ExtendedBuildBuddyService = CancelableService<buildbuddy.service.BuildBuddy
*/
export type BuildBuddyServiceRpcName = RpcMethodNames<buildbuddy.service.BuildBuddyService>;

export type FileEncoding = "gzip" | "";

export type XMLHttpResponseType = XMLHttpRequest["responseType"];

class RpcService {
service: ExtendedBuildBuddyService;
events: Subject<string>;
Expand Down Expand Up @@ -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<string> {
fetchFile(
fileURL: string,
responseType: XMLHttpResponseType,
{ storedEncoding }: { storedEncoding?: FileEncoding } = {}
): Promise<any> {
return new Promise((resolve, reject) => {
var request = new XMLHttpRequest();
request.responseType = responseType;
Expand All @@ -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();
});
}
Expand Down
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
9 changes: 9 additions & 0 deletions server/http/interceptors/interceptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
10 changes: 0 additions & 10 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit c488ae7

Please sign in to comment.