Skip to content

Commit cca7b1c

Browse files
Han5991aduh95
authored andcommitted
test_runner: avoid recompiling coverage globs for every file
`shouldSkipFileCoverage()` called `matchGlobPattern()` for each covered file, and `matchGlobPattern()` builds a fresh Minimatch (a full glob parse and regexp compile) on every call. The coverage exclude/include globs never change during a run, so the same patterns were recompiled once per file, which dominated the coverage report time. Compile `coverageExcludeGlobs`/`coverageIncludeGlobs` to matchers once per `TestCoverage` instance and reuse them for every file. Expose `createMatcher()` from `internal/fs/glob` for this. On a synthetic 200-test-file project this drops shouldSkipFileCoverage from ~117ms to ~11ms (cpu-prof, isolation=none); the saving scales with files * globs. Refs: #55103 Signed-off-by: sangwook <rewq5991@gmail.com> PR-URL: #63675 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 1ae3849 commit cca7b1c

2 files changed

Lines changed: 20 additions & 18 deletions

File tree

lib/internal/fs/glob.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,5 +947,6 @@ function matchGlobPattern(path, pattern, windows = isWindows) {
947947
module.exports = {
948948
__proto__: null,
949949
Glob,
950+
createMatcher,
950951
matchGlobPattern,
951952
};

lib/internal/test_runner/coverage.js

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const {
3737
ERR_SOURCE_MAP_MISSING_SOURCE,
3838
},
3939
} = require('internal/errors');
40-
const { matchGlobPattern } = require('internal/fs/glob');
40+
const { createMatcher } = require('internal/fs/glob');
4141
const { constants: { kMockSearchParam } } = require('internal/test_runner/mock/loader');
4242

4343
const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
@@ -86,6 +86,8 @@ class TestCoverage {
8686
#sourceLines = new SafeMap();
8787
#typeScriptLines = new SafeSet();
8888
#skipCache = new SafeMap();
89+
#excludeMatchers = null;
90+
#includeMatchers = null;
8991

9092
getLines(fileUrl, source) {
9193
// Split the file source into lines. Make sure the lines maintain their
@@ -550,28 +552,27 @@ class TestCoverage {
550552

551553
const absolutePath = fileURLToPath(url);
552554
const relativePath = relative(this.options.cwd, absolutePath);
553-
const {
554-
coverageExcludeGlobs: excludeGlobs,
555-
coverageIncludeGlobs: includeGlobs,
556-
} = this.options;
555+
// The exclude/include globs are fixed for the lifetime of this
556+
// TestCoverage instance, so compile each glob to a matcher once and reuse
557+
// it for every file. Building a fresh Minimatch per call (the previous
558+
// behavior) dominated the coverage report time, scaling with
559+
// files * globs.
560+
this.#excludeMatchers ??= ArrayPrototypeMap(
561+
this.options.coverageExcludeGlobs ?? [], (pattern) => createMatcher(pattern));
562+
this.#includeMatchers ??= ArrayPrototypeMap(
563+
this.options.coverageIncludeGlobs ?? [], (pattern) => createMatcher(pattern));
557564

558565
// This check filters out files that match the exclude globs.
559-
if (excludeGlobs?.length > 0) {
560-
for (let i = 0; i < excludeGlobs.length; ++i) {
561-
if (
562-
matchGlobPattern(relativePath, excludeGlobs[i]) ||
563-
matchGlobPattern(absolutePath, excludeGlobs[i])
564-
) return true;
565-
}
566+
for (let i = 0; i < this.#excludeMatchers.length; ++i) {
567+
const matcher = this.#excludeMatchers[i];
568+
if (matcher.match(relativePath) || matcher.match(absolutePath)) return true;
566569
}
567570

568571
// This check filters out files that do not match the include globs.
569-
if (includeGlobs?.length > 0) {
570-
for (let i = 0; i < includeGlobs.length; ++i) {
571-
if (
572-
matchGlobPattern(relativePath, includeGlobs[i]) ||
573-
matchGlobPattern(absolutePath, includeGlobs[i])
574-
) return false;
572+
if (this.#includeMatchers.length > 0) {
573+
for (let i = 0; i < this.#includeMatchers.length; ++i) {
574+
const matcher = this.#includeMatchers[i];
575+
if (matcher.match(relativePath) || matcher.match(absolutePath)) return false;
575576
}
576577
return true;
577578
}

0 commit comments

Comments
 (0)