Skip to content

perf(SDK-6463): prune excluded dirs from zip/md5 walks — minutes-long 'Creating tests.zip' stall on monorepos #1142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hamza-browserstack merged 3 commits into from
Jul 9, 2026
Merged

perf(SDK-6463): prune excluded dirs from zip/md5 walks — minutes-long 'Creating tests.zip' stall on monorepos #1142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
613a136
perf(upload): prune excluded directories from zip/md5 walks + cap tel…
Bhargavi-BS Jul 5, 2026
dcac4df
chore: remove ticket identifiers from code comments
Bhargavi-BS Jul 6, 2026
609d50d
perf(upload): compute node_modules telemetry size concurrently with t…
Bhargavi-BS Jul 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion bin/helpers/archiver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ const archiveSpecs = (runSettings, filePath, excludeFiles, md5data) => {

let ignoreFiles = utils.getFilesToIgnore(runSettings, excludeFiles);
logger.debug(`Patterns ignored during zip ${ignoreFiles}`);
archive.glob(`**/*.+(${Constants.allowedFileTypes.join("|")})`, { cwd: cypressFolderPath, matchBase: true, ignore: ignoreFiles, dot:true });
// SDK-6463 (perf): `ignore` filters entries only AFTER the walk visits them, so the
Comment thread
Bhargavi-BS marked this conversation as resolved.
Outdated
// globber still descends into node_modules/.git/etc. `skip` prunes those directories
// from the traversal entirely — on large monorepos this cuts zip creation from
// minutes to seconds without changing the archive contents.
let skipDirectories = utils.getDirectorySkipPatterns(ignoreFiles);
archive.glob(`**/*.+(${Constants.allowedFileTypes.join("|")})`, { cwd: cypressFolderPath, matchBase: true, ignore: ignoreFiles, skip: skipDirectories, dot:true });

let packageJSON = {};

Expand Down
3 changes: 3 additions & 0 deletions bin/helpers/checkUploaded.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ const checkSpecsMd5 = (runSettings, args, instrumentBlocks) => {
let options = {
cwd: cypressFolderPath,
ignore: ignoreFiles,
// SDK-6463 (perf): prune ignored directories from the md5 walk instead of
// filtering entries after descending into them (see utils.getDirectorySkipPatterns).
skip: utils.getDirectorySkipPatterns(ignoreFiles),
pattern: `**/*.+(${Constants.allowedFileTypes.join("|")})`
};
hashHelper.hashWrapper(options, instrumentBlocks).then(function (data) {
Expand Down
34 changes: 30 additions & 4 deletions bin/helpers/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,20 @@ exports.getFilesToIgnore = (runSettings, excludeFiles, logging = true) => {
return ignoreFiles;
}

// SDK-6463 (perf): derive directory-pruning patterns from the ignore list.
Comment thread
Bhargavi-BS marked this conversation as resolved.
Outdated
// readdir-glob (used both by archiver.glob for tests.zip and by hashUtil for the
// spec md5 check) applies `ignore` per-entry AFTER walking, so it still descends
// into node_modules/.git/dist etc. On large monorepos that walk alone takes
// minutes ("Creating tests.zip" stalls). Its `skip` option instead prevents
// DESCENDING into matching directories. Any ignore pattern ending in '/**' is
// safe to reuse as a skip: if a directory matches '<x>/**' then every descendant
// also matches it, so pruning cannot change which files end up included.
exports.getDirectorySkipPatterns = (ignoreFiles) => {
return (ignoreFiles || []).filter((pattern) =>
typeof pattern === 'string' && pattern.endsWith('/**')
);
}

exports.getNumberOfSpecFiles = (bsConfig, args, cypressConfig, turboScaleSession=false) => {
let defaultSpecFolder
let testFolderPath
Expand Down Expand Up @@ -1722,13 +1736,20 @@ exports.fetchZipSize = (fileName) => {
}
}

const getDirectorySize = async function(dir) {
const getDirectorySize = async function(dir, deadline) {
try{
// SDK-6463 (perf): this telemetry-only walk recursively stats every file (it is
// pointed at node_modules in runs.js). On large monorepos it blocked the pipeline
// between archiving and uploading for tens of seconds. Stop descending once the
// deadline passes — folder size is best-effort telemetry, never worth stalling a run.
if (deadline && Date.now() > deadline) {
return 0;

@amaanbs amaanbs Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any downstream effects from this?

@Bhargavi-BS Bhargavi-BS Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Verified the full flow — no functional downstream effects:

  • fetchFolderSize has exactly one caller (runs.js → node_modules_size), and that value has exactly one sink: the dataToSend instrumentation object, merged into buildReportData and sent via sendUsageReport. It is telemetry-only — nothing branches on it (no upload decisions, no timeouts, no user-facing output).
  • getDirectorySize is module-private (not exported), so there are no other consumers.

Behavior when the 5s deadline fires: the reported node_modules_size is partial (under-reported) for that run, with a debug log noting it. Worth noting the field was already best-effort — the existing catch returns 0 on any readdir/stat error, so consumers already tolerate inaccurate values. On repos where the walk finishes within 5s (the common case) the value is unchanged.

The trade: bounded, slightly-lossy telemetry vs. blocking the archive→upload pipeline for tens of seconds on large monorepos (the reported case). Happy to raise the cap or make it env-configurable if instrumentation accuracy is a concern.

@Bhargavi-BS Bhargavi-BS Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pushed an improvement on this in 609d50d that removes the concern entirely: the walk no longer blocks anything. fetchFolderSize is now started before the upload (not awaited) and awaited only after the upload completes, right before the value is consumed for dataToSend. In the common case the walk resolves while the upload is in flight, so it adds zero wall-clock and reports the full, accurate size. The 5s deadline stays as a backstop for pathological trees whose walk outlives the upload (worst case: partial size in the usage report, never a stalled run). fetchFolderSize never rejects, so the floating promise is safe.

@Bhargavi-BS Bhargavi-BS Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Verified both modes, structurally and with real runs.

Structure — the await sits on the shared path before the modes ever diverge:

runs.js:286  walk starts (not awaited)      ─┐
runs.js:314  await nodeModulesSizePromise    ├─ shared path, both modes
runs.js:341  createBuild(...)               ─┘
runs.js:378  if (args.sync)  → pollBuildStatus     ← first sync/async divergence
runs.js:437  if (!args.sync) → exit message
runs.js:450  dataToSend.node_modules_size          ← plain resolved local, both modes

The value is awaited once, at a single point common to both modes, before createBuild — neither mode can observe an unresolved value, and neither mode's control flow changed. (fetchFolderSize never rejects, so the floating promise can't produce an unhandled rejection either.)

Empirical — real runs on a 144k-file synthetic monorepo (customer-style home_directory: "./" + their 130 exclude patterns), this branch @ 609d50d:

Mode Zip creation Upload Build created After build
async (default) <1s 6s ✅ 5df4c606… usage report sent, clean exit 0
--sync <1s 4s ✅ c45b0044… polling started ("BrowserStack machines are now setting up…") and followed the build ✓

For reference, published 1.36.12 on the same repo: 5s zip creation and 13s in the pre-zip md5 phase — this branch reduces both to ~0 (and on the reporting customer's Windows machine with a real NX tree, the zip phase alone was 86s).

}
const subdirs = (await readdir(dir));
const files = await Promise.all(subdirs.map(async (subdir) => {
const res = path.resolve(dir, subdir);
const s = (await stat(res));
return s.isDirectory() ? getDirectorySize(res) : (s.size);
return s.isDirectory() ? getDirectorySize(res, deadline) : (s.size);
}));
return files.reduce((a, f) => a+f, 0);
}catch(e){
Expand All @@ -1738,10 +1759,15 @@ const getDirectorySize = async function(dir) {
}
};

exports.fetchFolderSize = async (dir) => {
exports.fetchFolderSize = async (dir, timeoutMs = 5000) => {
try {
if(fs.existsSync(dir)){
return (await getDirectorySize(dir) / 1024 / 1024);
const deadline = Date.now() + timeoutMs;
const size = (await getDirectorySize(dir, deadline) / 1024 / 1024);
if (Date.now() > deadline) {
logger.debug(`Folder size calculation for ${dir} exceeded ${timeoutMs}ms; reporting partial size.`);
}
return size;
}
return 0;
} catch (error) {
Expand Down
28 changes: 28 additions & 0 deletions test/unit/bin/helpers/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,34 @@ describe('utils', () => {
});
});

// SDK-6463 (perf): directory-shaped ignore patterns are reused as readdir-glob `skip`
// patterns so the zip/md5 walks prune excluded trees instead of descending into them.
describe('getDirectorySkipPatterns', () => {
it('keeps only patterns ending in /** (safe to prune)', () => {
const ignore = [
'**/node_modules/**',
'node_modules/**',
'dist/**',
'apps/admin-portal/**',
'package.json', // file pattern — must NOT be used for pruning
'.env',
'**/README.md',
];
chai.expect(utils.getDirectorySkipPatterns(ignore)).to.be.eql([
'**/node_modules/**',
'node_modules/**',
'dist/**',
'apps/admin-portal/**',
]);
});

it('handles empty/undefined input and non-string entries', () => {
chai.expect(utils.getDirectorySkipPatterns(undefined)).to.be.eql([]);
chai.expect(utils.getDirectorySkipPatterns([])).to.be.eql([]);
chai.expect(utils.getDirectorySkipPatterns([null, 42, 'x/**'])).to.be.eql(['x/**']);
});
});

describe('setTestEnvs', () => {
it('set env only from args', () => {
let argsEnv = 'env3=value3, env4=value4';
Expand Down
Loading