bazelbuild / vscode-bazel

Bazel support for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=BazelBuild.vscode-bazel
Apache License 2.0
232 stars 76 forks source link

perf: reduces bazel query output #308

Closed iamricard closed 7 months ago

iamricard commented 8 months ago

I spent a little bit of time digging into why this extension kept crashing in our codebase. After some investigation, I noticed the bazel query helper requests the output as protobufs. The output is very useful for tools, but it does come with a few default options that bloat the query's output:

I dug through the codebase and it seems like none of the values populated are used anywhere in the extension. For smaller projects this is probably fine. For the codebases we (DataDog) work with, the in-memory representation of the entire graph is far too large. One of our repos has close to 900k targets. Ran a small benchmark script to capture how much memory processing the output takes before and after. Ran with: time node --max-old-space-size=20000 bench.mjs:

Benchmark codebase:

// <large-repo>/bench.mjs
import protos from "/home/user/.vscode/extensions/bazelbuild.vscode-bazel-0.7.0/out/src/protos/protos.js";
import { exec } from "node:child_process";

const before_all = process.memoryUsage().heapUsed / 1024 / 1024;
const go = new Promise((resolve, reject) => exec(
    // swapped with the new query
    "bzl query \"kind('.* rule', ...)\" --output proto",
    { cwd: "/Users/ricard.sole/dd/dd-source", encoding: null, maxBuffer: Number.MAX_SAFE_INTEGER },
    (error, stdout, _) => {
        if (error) {
            reject(null);
        } else {
            const after_exec_sync = process.memoryUsage().heapUsed / 1024 / 1024;
            protos.blaze_query.QueryResult.decode(stdout);
            const after_decode = process.memoryUsage().heapUsed / 1024 / 1024;
            console.log(`\
* before_all=${before_all}
* after_exec_sync=${after_exec_sync}
* after_decode=${after_decode}`);
            resolve(stdout);
        }
    },
));

await go;

Should fix bazelbuild/vscode-bazel#296 in most setups. Ideally there would be a longer-term rework to lazily load and process the query output but that's quite a large undertaking.

iamricard commented 8 months ago

CI just seems to be kind of busted 👀

SyntaxError: Unexpected token '??='
--
  | at wrapSafe (internal/modules/cjs/loader.js:1029:16)
  | at Module._compile (internal/modules/cjs/loader.js:1078:27)
  | at Object.Module._extensions..js (internal/modules/cjs/loader.js:1143:10)
  | at Module.load (internal/modules/cjs/loader.js:979:32)
  | at Function.Module._load (internal/modules/cjs/loader.js:819:12)
  | at Module.require (internal/modules/cjs/loader.js:1003:19)
  | at require (internal/modules/cjs/helpers.js:107:18)
  | at Object.<anonymous> (/usr/lib/node_modules/npm/node_modules/@npmcli/agent/lib/index.js:7:15)
  | at Module._compile (internal/modules/cjs/loader.js:1114:14)
  | at Object.Module._extensions..js (internal/modules/cjs/loader.js:1143:10)
iamricard commented 7 months ago

Thanks, LGTM! #312 should fix the CI failures. Once that merges, can you rebase on top?

yup sounds good!

iamricard commented 7 months ago

Thanks, LGTM! #312 should fix the CI failures. Once that merges, can you rebase on top?

done!

iamricard commented 7 months ago

thanks for merging @coeuvre 🙇🏼 do you have an idea of when y'all are likely to cut a new release? i'd like to let folks know at datadog they can go back to using the published extension 🎉

coeuvre commented 7 months ago

I am not sure. @jfirebaugh has been managing the releases. John, what do you think?

daivinhtran commented 6 months ago

@iamricard I'm afraid this optimization is breaking CodeLens (or maybe only on the bazel version my repo is using).

I tried reverting the proto flags and CodeLens worked as normal. See the screencast when the flags are used. A few obvious bugs: