perf: make large diffs much cheaper to process - #2934
Open
diogokiss wants to merge 2 commits into
Open
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 127 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
diogokiss
force-pushed
the
perf/large-diff-performance
branch
from
August 17, 2026 13:17
0a92df4 to
150dc0f
Compare
`@actions/core.debug()` always writes to stdout, because it never calls
`isDebug()` itself. `RUNNER_DEBUG` therefore only decides whether the runner
displays a line, not whether the action sends it. The debug calls here are
built from template strings holding whole file lists, so `JSON.stringify` also
ran on every one of them no matter what.
On a large diff that is a lot of work and a lot of bytes for output nobody
reads. A `debugJson` helper now checks `isDebug()` before serializing
anything, and every debug call that holds a value goes through it.
Measured on a generated repository, running the built action the way the
runner does, with one filter matching every changed file and `RUNNER_DEBUG`
unset:
files stdout before stdout after time before time after
20,000 5.20 MiB 0 bytes 5.38s 5.79s
40,000 10.51 MiB 0 bytes 22.64s 23.38s
80,000 21.33 MiB 0 bytes 91.70s 74.61s
The bytes are the point. This is what fills the stdout pipe on a large diff,
and the pipe filling up is what produces `write ENOBUFS`. The remaining time
growth has a separate cause and is dealt with in the next commit.
Debug output itself is unchanged. With `RUNNER_DEBUG=1` the build emits the
same 48 debug lines, with the same labels, as before.
References:
https://github.com/actions/toolkit/blob/%40actions/core%402.0.2/packages/core/src/core.ts
https://github.com/tj-actions/changed-files/blob/934b2d2c7e653bb8c968afed5a0428617f09aa24/src/changedFilesOutput.ts#L39
`other_changed_files`, `other_modified_files` and `other_deleted_files` are
the paths in the diff that the filter did not match. Each was worked out by
calling `Array.includes` inside `Array.filter`, which rescans the whole list
of matched paths for every path in the diff. The cost of that grows with the
square of the number of changed files, and all three run once per filter key.
Each one now tests against a `Set` built once.
Measured on a generated repository, running the built action the way the
runner does, with one filter matching every changed file:
files before after
20,000 5.79s 1.91s
40,000 23.38s 1.78s
80,000 74.61s 2.61s
The "before" column is the previous commit, so these numbers isolate this
change. Time was growing about four times for every doubling of the diff and
is now close to flat.
Behavior is unchanged. On a 20,000 file run every file written to
`output_dir` is byte for byte identical to the same run before either commit.
The new tests cover all three lists, including a filter that matches
everything, a filter that matches nothing, a path reported by more than one
change type, and renamed paths. They pass against the original code as well,
so they describe the existing behavior rather than this change.
References:
https://github.com/tj-actions/changed-files/blob/934b2d2c7e653bb8c968afed5a0428617f09aa24/src/changedFilesOutput.ts#L284-L286
https://github.com/tj-actions/changed-files/blob/934b2d2c7e653bb8c968afed5a0428617f09aa24/src/changedFilesOutput.ts#L374-L376
https://github.com/tj-actions/changed-files/blob/934b2d2c7e653bb8c968afed5a0428617f09aa24/src/changedFilesOutput.ts#L474-L476
diogokiss
force-pushed
the
perf/large-diff-performance
branch
from
August 17, 2026 13:29
150dc0f to
72ba182
Compare
This was referenced Aug 18, 2026
diogokiss
marked this pull request as ready for review
August 18, 2026 12:39
Author
|
Hi, @jackton1 ! 🙋🏻 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This Pull Request was opened by an AI agent, which has been previously reviewed and green-lit by a human, @diogokiss.
Fixes causes 1 and 2 of #2933. Cause 3 needs a new input, so it is left for a separate Pull Request
once the name and shape are agreed.
Two commits, one per cause. They fix different symptoms, so they are easier to judge apart:
perf: skip debug serialization when debug logging is off— removes the bytes.perf: build the other_* file lists with a Set— removes the time.Commit 1 — debug serialization
@actions/core.debug()always writes to stdout, because it never callsisDebug()itself, soRUNNER_DEBUGonly decides whether the runner displays a line and not whether the action sends it.The calls here are built from template strings holding whole file lists, so
JSON.stringifyalso ranon every one of them regardless. A
debugJsonhelper now checksisDebug()before doing any of thatwork, and every debug call that holds a value goes through it.
Commit 2 — the
other_*listsother_changed_files,other_modified_filesandother_deleted_fileswere each built by callingArray.includesinsideArray.filter, which rescans the whole list of matched paths for every pathin the diff. All three run once per filter key. Each now tests against a
Setbuilt once.Measurements
Generated repository, one filter key matching every changed file,
RUNNER_DEBUGunset, running thebuilt
dist/index.jsthe way the runner does. The script is in #2933 so you can reproduce it. Eachcolumn is measured at that commit, so the two effects are separated:
mainWorth reading those two columns separately:
is what produces
write ENOBUFS, so this is the part that stops the crash.diff.
Below 20,000 files both builds sit around 1.5 s, so the difference only shows up as the diff grows.
Nothing about the results changes
output_dirare byte for byte identical to the samerun against upstream
main, and so is the whole ofGITHUB_OUTPUT.RUNNER_DEBUG=1, the build emits the same 48 debug lines with the same labels. The guard onlyskips work when nobody is going to read the output.
src/__tests__/changedFilesOutput.test.tscover the threeother_*lists,including a filter that matches everything, a filter that matches nothing, a path reported by more
than one change type, and renamed paths. They pass against the original code as well, so they
describe existing behavior rather than the change.
GITHUB_OUTPUTis untouched by this Pull Request. That is cause 3 in #2933.Checks
yarn allpasses at both commits: build, prettier, eslint with--max-warnings 0, ncc package, andjest with coverage (71 tests, 5 suites).
dist/is rebuilt in each commit, so the bundle matches thesources at every point in the history.