Skip to content

Commit a2b9095

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 02fbff4 commit a2b9095

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
@@ -38,7 +38,7 @@ const {
3838
ERR_SOURCE_MAP_MISSING_SOURCE,
3939
},
4040
}=require('internal/errors');
41-
const{matchGlobPattern}=require('internal/fs/glob');
41+
const{createMatcher}=require('internal/fs/glob');
4242
const{constants: { kMockSearchParam }}=require('internal/test_runner/mock/loader');
4343

4444
constkCoverageFileRegex=/^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
@@ -87,6 +87,8 @@ class TestCoverage {
8787
#sourceLines =newSafeMap();
8888
#typeScriptLines =newSafeSet();
8989
#skipCache =newSafeMap();
90+
#excludeMatchers =null;
91+
#includeMatchers =null;
9092

9193
getLines(fileUrl,source){
9294
// Split the file source into lines. Make sure the lines maintain their
@@ -551,28 +553,27 @@ class TestCoverage {
551553

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

559566
// This check filters out files that match the exclude globs.
560-
if(excludeGlobs?.length>0){
561-
for(leti=0;i<excludeGlobs.length;++i){
562-
if(
563-
matchGlobPattern(relativePath,excludeGlobs[i])||
564-
matchGlobPattern(absolutePath,excludeGlobs[i])
565-
)returntrue;
566-
}
567+
for(leti=0;i<this.#excludeMatchers.length;++i){
568+
constmatcher=this.#excludeMatchers[i];
569+
if(matcher.match(relativePath)||matcher.match(absolutePath))returntrue;
567570
}
568571

569572
// This check filters out files that do not match the include globs.
570-
if(includeGlobs?.length>0){
571-
for(leti=0;i<includeGlobs.length;++i){
572-
if(
573-
matchGlobPattern(relativePath,includeGlobs[i])||
574-
matchGlobPattern(absolutePath,includeGlobs[i])
575-
)returnfalse;
573+
if(this.#includeMatchers.length>0){
574+
for(leti=0;i<this.#includeMatchers.length;++i){
575+
constmatcher=this.#includeMatchers[i];
576+
if(matcher.match(relativePath)||matcher.match(absolutePath))returnfalse;
576577
}
577578
returntrue;
578579
}

0 commit comments

Comments
 (0)