From e7fcfe6ddc8332675c2533e3a82d8b84b69bba5d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 28 May 2026 16:46:15 +0200 Subject: [PATCH 1/3] tools: refine `v8.nix` source definition Signed-off-by: Antoine du Hamel --- tools/nix/v8.nix | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tools/nix/v8.nix b/tools/nix/v8.nix index fa931092a83f08..3f050b9a8f5e75 100644 --- a/tools/nix/v8.nix +++ b/tools/nix/v8.nix @@ -57,10 +57,24 @@ let ../../tools/icu/icutrim.py ../../tools/icu/no-op.cc ]; + potentiallyAlreadyRemovedFiles = + # Files that are removed in the release tarball (see Makefile $(TARBALL) target) + [ (fileset.difference ../../deps/v8/test ../../deps/v8/test/torque) ] + ++ (builtins.filter builtins.pathExists [ + ../../deps/v8/samples + ../../deps/v8/tools/profviz + ../../deps/v8/tools/run-tests.py + ../../deps/v8/third_party/ittapi + ]); + trackedFiles = + ({ + # This line is being modified by Makefile $(TARBALL) target, any change to it should be sync + fileset = fileset.intersection (fileset.gitTracked root) (fileset.unions files); + }).fileset; in fileset.toSource { inherit root; - fileset = fileset.intersection (fileset.gitTracked root) (fileset.unions files); + fileset = fileset.difference trackedFiles (fileset.unions potentiallyAlreadyRemovedFiles); }; v8Dir = "${src}/deps/v8"; in From a830b12ffbaf82e7d6b6f0f90b5c06de1d268f6e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 28 Aug 2025 13:21:19 +0200 Subject: [PATCH 2/3] tools: add GHA benchmark runner Signed-off-by: Antoine du Hamel --- .github/workflows/benchmark.yml | 206 ++++++++++++++++++++++++++++++++ tools/nix/benchmarkTools.nix | 4 +- 2 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/benchmark.yml diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml new file mode 100644 index 00000000000000..697c4a5318047d --- /dev/null +++ b/.github/workflows/benchmark.yml @@ -0,0 +1,206 @@ +name: Benchmark + +on: + workflow_dispatch: + inputs: + repo: + type: string + description: GitHub repository to fetch from (default to the current repo) + pr_id: + type: number + required: true + description: The PR to test + commit: + required: true + type: string + description: The expect HEAD of the PR + category: + required: true + type: string + description: The category (or categories) of tests to run, for example buffers, cluster etc. Maps to a folders in node/benchmark + filter: + type: string + description: A substring to restrict the benchmarks to run in a category. e.g. `net-c2c` + runs: + type: number + default: 30 + description: How many times to repeat each benchmark + +permissions: + contents: read + +jobs: + build: + strategy: + fail-fast: true + matrix: + include: + - runner: ubuntu-24.04 + system: x86_64-linux + - runner: ubuntu-24.04-arm + system: aarch64-linux + - runner: macos-15-intel + system: x86_64-darwin + - runner: macos-latest + system: aarch64-darwin + name: '${{ matrix.system }}: with shared libraries' + runs-on: ${{ matrix.runner }} + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + repository: ${{ inputs.repo || github.repository }} + ref: refs/pull/${{ inputs.pr_id }}/merge + persist-credentials: false + fetch-depth: 2 + + - name: Validate PR head and roll back to base commit + run: | + [ "$(git rev-parse HEAD^2)" = "$EXPECTED_SHA" ] + git reset HEAD^ --hard + env: + EXPECTED_SHA: ${{ inputs.commit }} + + - uses: cachix/install-nix-action@96951a368ba55167b55f1c916f7d416bac6505fe # v31.10.3 + with: + extra_nix_config: sandbox = true + + - uses: cachix/cachix-action@1eb2ef646ac0255473d23a5907ad7b04ce94065c # v17 + with: + # We do not pass any `authToken` to avoid polluting the cache with potentially untrusted code. + name: nodejs + + - name: Configure sccache + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + script: | + core.exportVariable('SCCACHE_GHA_VERSION', 'on'); + core.exportVariable('ACTIONS_CACHE_SERVICE_V2', 'on'); + core.exportVariable('ACTIONS_RESULTS_URL', process.env.ACTIONS_RESULTS_URL || ''); + core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || ''); + + - name: Build Node.js on the base commit + run: | + nix-shell \ + -I nixpkgs=./tools/nix/pkgs.nix \ + --pure --keep TAR_DIR --keep FLAKY_TESTS \ + --keep SCCACHE_GHA_ENABLED --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN \ + --arg useSeparateDerivationForV8 true \ + --arg loadJSBuiltinsDynamically false \ + --arg ccache "${NIX_SCCACHE:-null}" \ + --arg devTools '[]' \ + --arg benchmarkTools '[]' \ + --run ' + make build-ci -j4 V=1 + ' + mv out/Release/node base_node + + - name: Checkout the merge commit + run: git reset FETCH_HEAD --hard + + - name: Re-build Node.js on the merge commit + # ccache is disabled here to avoid polluting the cache. Local build outputs should make this build relatively quick anyway. + if: ${{ github.event_name == 'workflow_dispatch' }} + run: | + nix-shell \ + -I nixpkgs=./tools/nix/pkgs.nix \ + --pure \ + --arg useSeparateDerivationForV8 true \ + --arg loadJSBuiltinsDynamically false \ + --arg ccache 'null' \ + --arg devTools '[]' \ + --arg benchmarkTools '[]' \ + --run ' + make -j4 V=1 + ' + + - name: Run benchmark + if: ${{ github.event_name == 'workflow_dispatch' }} + run: | + nix-shell \ + -I nixpkgs=./tools/nix/pkgs.nix \ + --pure --keep FILTER --keep LC_ALL --keep LANG \ + --arg loadJSBuiltinsDynamically false \ + --arg ccache 'null' \ + --arg icu 'null' \ + --arg sharedLibDeps '{}' \ + --arg devTools '[]' \ + --run ' + set -o pipefail + ./base_node benchmark/compare.js \ + --filter "$FILTER" \ + --runs ${{ inputs.runs }} \ + --old ./base_node --new ./node \ + -- ${{ inputs.category }} \ + | tee /dev/stderr \ + > ${{ matrix.system }}.csv + echo "> [!WARNING] " + echo "> Do not take GHA benchmark results as face value, always confirm them" + echo "> using a dedicated machine, e.g. Jenkins CI." + echo + echo "Benchmark results:" + echo + echo '"'"'```'"'"' + Rscript benchmark/compare.R < ${{ matrix.system }}.csv + echo '"'"'```'"'"' + echo + echo "> [!WARNING] " + echo "> Do not take GHA benchmark results as face value, always confirm them" + echo "> using a dedicated machine, e.g. Jenkins CI." + ' | tee /dev/stderr >> "$GITHUB_STEP_SUMMARY" + env: + FILTER: ${{ inputs.filter }} + + - name: Upload raw benchmark results + if: ${{ github.event_name == 'workflow_dispatch' }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: csv-${{ matrix.system }} + path: ${{ matrix.system }}.csv + + aggregate-benchmark-results: + needs: build + name: Aggregate benchmark results + if: ${{ github.event_name == 'workflow_dispatch' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + sparse-checkout: | + benchmark/*.R + tools/nix/*.nix + *.nix + sparse-checkout-cone-mode: false + + - name: Download benchmark raw results + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + pattern: csv-* + merge-multiple: true + path: raw-results + + - uses: cachix/install-nix-action@96951a368ba55167b55f1c916f7d416bac6505fe # v31.10.3 + with: + extra_nix_config: sandbox = true + + - name: Benchmark results + run: | + nix-shell \ + -I nixpkgs=./tools/nix/pkgs.nix \ + --pure --keep LC_ALL --keep LANG \ + -E '(import {}).mkShell { buildInputs = import ./tools/nix/benchmarkTools.nix { withHttpBenchmarkDeps = false; }; }' \ + --run ' + echo "> [!WARNING] " + echo "> Do not take GHA benchmark results as face value, always confirm them" + echo "> using a dedicated machine, e.g. Jenkins CI." + echo + echo "Benchmark results:" + echo + echo '"'"'```'"'"' + awk "FNR==1 && NR!=1{next;}{print}" raw-results/*.csv | Rscript benchmark/compare.R + echo '"'"'```'"'"' + echo + echo "> [!WARNING] " + echo "> Do not take GHA benchmark results as face value, always confirm them" + echo "> using a dedicated machine, e.g. Jenkins CI." + ' | tee /dev/stderr >> "$GITHUB_STEP_SUMMARY" diff --git a/tools/nix/benchmarkTools.nix b/tools/nix/benchmarkTools.nix index 62c744a552b7d1..7c37c423c2530a 100644 --- a/tools/nix/benchmarkTools.nix +++ b/tools/nix/benchmarkTools.nix @@ -1,9 +1,11 @@ { pkgs ? import ./pkgs.nix { }, + withHttpBenchmarkDeps ? true, }: [ pkgs.R pkgs.rPackages.ggplot2 pkgs.rPackages.plyr - pkgs.wrk ] +++ pkgs.lib.optional withHttpBenchmarkDeps pkgs.wrk +++ pkgs.lib.optional pkgs.stdenv.buildPlatform.isLinux pkgs.glibcLocales From 47db1ce66ea079d03ef22b6b335ddd67ac72c74b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 28 May 2026 12:51:47 +0200 Subject: [PATCH 3/3] inspect: use `ArrayPrototypeJoin` instead of custom implementation --- lib/internal/util.js | 16 ---------------- lib/internal/util/inspect.js | 9 ++++----- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 34af9ca6f61a6f..00ff514d3e2cf0 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -536,21 +536,6 @@ function promisify(original) { promisify.custom = kCustomPromisifiedSymbol; -// The built-in Array#join is slower in v8 6.0 -function join(output, separator) { - let str = ''; - if (output.length !== 0) { - const lastIndex = output.length - 1; - for (let i = 0; i < lastIndex; i++) { - // It is faster not to use a template string here - str += output[i]; - str += separator; - } - str += output[lastIndex]; - } - return str; -} - // As of V8 6.6, depending on the size of the array, this is anywhere // between 1.5-10x faster than the two-arg version of Array#splice() function spliceOne(list, index) { @@ -1005,7 +990,6 @@ module.exports = { isUnderNodeModules, isMacOS, isWindows, - join, lazyDOMException, lazyDOMExceptionClass, normalizeEncoding, diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index c611eb97fc0755..01ac0223502f8d 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -151,7 +151,6 @@ const { const { customInspectSymbol, isError, - join, removeColors, } = require('internal/util'); @@ -2668,7 +2667,7 @@ function reduceToSingleString( const start = output.length + ctx.indentationLvl + braces[0].length + base.length + 10; if (isBelowBreakLength(ctx, output, start, base)) { - const joinedOutput = join(output, ', '); + const joinedOutput = ArrayPrototypeJoin(output, ', '); if (!StringPrototypeIncludes(joinedOutput, '\n')) { return `${base ? `${base} ` : ''}${braces[0]} ${joinedOutput}` + ` ${braces[1]}`; @@ -2679,12 +2678,12 @@ function reduceToSingleString( // Line up each entry on an individual line. const indentation = `\n${StringPrototypeRepeat(' ', ctx.indentationLvl)}`; return `${base ? `${base} ` : ''}${braces[0]}${indentation} ` + - `${join(output, `,${indentation} `)}${indentation}${braces[1]}`; + `${ArrayPrototypeJoin(output, `,${indentation} `)}${indentation}${braces[1]}`; } // Line up all entries on a single line in case the entries do not exceed // `breakLength`. if (isBelowBreakLength(ctx, output, 0, base)) { - return `${braces[0]}${base ? ` ${base}` : ''} ${join(output, ', ')} ` + + return `${braces[0]}${base ? ` ${base}` : ''} ${ArrayPrototypeJoin(output, ', ')} ` + braces[1]; } const indentation = StringPrototypeRepeat(' ', ctx.indentationLvl); @@ -2694,7 +2693,7 @@ function reduceToSingleString( const ln = base === '' && braces[0].length === 1 ? ' ' : `${base ? ` ${base}` : ''}\n${indentation} `; // Line up each entry on an individual line. - return `${braces[0]}${ln}${join(output, `,\n${indentation} `)} ${braces[1]}`; + return `${braces[0]}${ln}${ArrayPrototypeJoin(output, `,\n${indentation} `)} ${braces[1]}`; } function hasBuiltInToString(value) {