Skip to content

tools: improve benchmark build cache reuse - #65859

Open
panva wants to merge 1 commit into
nodejs:mainfrom
panva:share-benchmark-cache
Open

tools: improve benchmark build cache reuse#65859
panva wants to merge 1 commit into
nodejs:mainfrom
panva:share-benchmark-cache

Conversation

@panva

@panva panva commented Sep 6, 2026

Copy link
Copy Markdown
Member

This ought to speed up benchmark runs by reusing the Perfetto-enabled V8 build already cached by shared-library CI on Linux x64. It also fixes the sccache setup while keeping cache access read-only and preserving incremental PR rebuilds.

Match Linux x64 benchmark builds to the Perfetto-enabled V8 configuration already cached by shared-library CI.

Enable the GHA sccache backend in read-only mode for base builds. Retain the compiler wrapper for incremental PR builds, but stop the remote-backed server and use a read-only local cache for PR code.

Assisted-by: GitHub Copilot

Match Linux x64 benchmark builds to the Perfetto-enabled V8
configuration already cached by shared-library CI.

Enable the GHA sccache backend in read-only mode for base builds.
Retain the compiler wrapper for incremental PR builds, but stop the
remote-backed server and use a read-only local cache for PR code.

Assisted-by: GitHub Copilot
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva
panva requested a review from aduh95 September 6, 2026 15:54
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Sep 6, 2026
Comment on lines +130 to +132
set -e
make build-ci -j4 V=1
sccache --stop-server

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
set -e
make build-ci -j4 V=1
sccache --stop-server
make build-ci -j4 V=1 && sccache --stop-server

--arg useSeparateDerivationForV8 true \
--arg withPerfetto ${{ matrix.perfetto || false }} \
--arg loadJSBuiltinsDynamically false \
--arg ccache "${NIX_SCCACHE:-null}" \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use sscache in the --run, we shouldn't default to null. I suggest we remove NIX_SCCACHE

Suggested change
--arg ccache "${NIX_SCCACHE:-null}" \
--arg ccache '(import <nixpkgs> {}).sccache' \

Alternatively, we could add a Bash assertion

Suggested change
--arg ccache "${NIX_SCCACHE:-null}" \
--arg ccache "${NIX_SCCACHE:?}" \

--arg withPerfetto ${{ matrix.perfetto || false }} \
--arg loadJSBuiltinsDynamically false \
--arg ccache 'null' \
--arg ccache "${NIX_SCCACHE:-null}" \

@aduh95 aduh95 Sep 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here (although we don't explicitly call sscache here, so I guess it could stay – but I reckon we'd prefer to be consistent).

Is there any point to have sccache for this step? Either the object is already compiled locally (in which case, no need for sccache, Ninja will simply reuse it), either it's not and there's no reason to believe it would be in the cache either (especially the local one), or am I forgetting something?

- runner: macos-latest
system: aarch64-darwin
name: '${{ matrix.system }}: with shared libraries'
name: '${{ matrix.system }}: with shared libraries${{ matrix.perfetto && '' and perfetto'' || '''' }}'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this is going away in #65794

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Issues and PRs related to the general management of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants