From c61919a0516d1d3ac4008fbcc7112edc1633085c Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Thu, 28 Mar 2019 18:27:39 -0700 Subject: [PATCH 1/5] Fix coverage memory leak and minor optimizations. --- .../jest-reporters/src/coverage_reporter.ts | 16 +++-- packages/jest-runner/src/runTest.ts | 14 ++-- .../jest-test-result/src/formatTestResults.ts | 64 +++++++++---------- packages/jest-test-result/src/types.ts | 2 +- 4 files changed, 52 insertions(+), 44 deletions(-) diff --git a/packages/jest-reporters/src/coverage_reporter.ts b/packages/jest-reporters/src/coverage_reporter.ts index bb761944b417..6bbb00cbb337 100644 --- a/packages/jest-reporters/src/coverage_reporter.ts +++ b/packages/jest-reporters/src/coverage_reporter.ts @@ -53,29 +53,35 @@ export default class CoverageReporter extends BaseReporter { testResult: TestResult, _aggregatedResults: AggregatedResult, ) { + // Coverage and source maps are only appended to the test result for this + // handler. Delete when processed to preserve memory. + if (testResult.coverage) { this._coverageMap.merge(testResult.coverage); - // Remove coverage data to free up some memory. - delete testResult.coverage; + testResult.coverage = undefined; + } - Object.keys(testResult.sourceMaps).forEach(sourcePath => { + const sourceMaps = testResult.sourceMaps; + if (sourceMaps) { + Object.keys(sourceMaps).forEach(sourcePath => { let inputSourceMap: RawSourceMap | undefined; try { const coverage: FileCoverage = this._coverageMap.fileCoverageFor( sourcePath, ); - ({inputSourceMap} = coverage.toJSON() as any); + inputSourceMap = (coverage.toJSON() as any).inputSourceMap; } finally { if (inputSourceMap) { this._sourceMapStore.registerMap(sourcePath, inputSourceMap); } else { this._sourceMapStore.registerURL( sourcePath, - testResult.sourceMaps[sourcePath], + sourceMaps[sourcePath], ); } } }); + testResult.sourceMaps = undefined; } } diff --git a/packages/jest-runner/src/runTest.ts b/packages/jest-runner/src/runTest.ts index 27eac4a7e75d..63ff7606e826 100644 --- a/packages/jest-runner/src/runTest.ts +++ b/packages/jest-runner/src/runTest.ts @@ -253,14 +253,18 @@ async function runTestInternal( result.perfStats = {end: Date.now(), start}; result.testFilePath = path; - result.coverage = runtime.getAllCoverageInfoCopy(); - result.sourceMaps = runtime.getSourceMapInfo( - new Set(Object.keys(result.coverage || {})), - ); - result.console = testConsole.getBuffer(); result.skipped = testCount === result.numPendingTests; result.displayName = config.displayName; + const coverage = runtime.getAllCoverageInfoCopy(); + if (coverage) { + const coverageKeys = Object.keys(coverage); + if (coverageKeys.length) { + result.coverage = coverage; + result.sourceMaps = runtime.getSourceMapInfo(new Set(coverageKeys)); + } + } + if (globalConfig.logHeapUsage) { if (global.gc) { global.gc(); diff --git a/packages/jest-test-result/src/formatTestResults.ts b/packages/jest-test-result/src/formatTestResults.ts index 161937b7c1dc..7eae5d01eeb7 100644 --- a/packages/jest-test-result/src/formatTestResults.ts +++ b/packages/jest-test-result/src/formatTestResults.ts @@ -16,41 +16,41 @@ import { TestResult, } from './types'; -const formatResult = ( +const formatTestResult = ( testResult: TestResult, - codeCoverageFormatter: CodeCoverageFormatter, - reporter: CodeCoverageReporter, + codeCoverageFormatter?: CodeCoverageFormatter, + reporter?: CodeCoverageReporter, ): FormattedTestResult => { - const now = Date.now(); - const output: FormattedTestResult = { - assertionResults: [], - coverage: {}, - endTime: now, - message: '', - name: testResult.testFilePath, - startTime: now, - status: 'failed', - summary: '', - }; - + const assertionResults = testResult.testResults.map(formatTestAssertion); if (testResult.testExecError) { - output.message = testResult.testExecError.message; - output.coverage = {}; + const now = Date.now(); + return { + assertionResults, + coverage: {}, + endTime: now, + message: testResult.failureMessage + ? testResult.failureMessage + : testResult.testExecError.message, + name: testResult.testFilePath, + startTime: now, + status: 'failed', + summary: '', + }; } else { const allTestsPassed = testResult.numFailingTests === 0; - output.status = allTestsPassed ? 'passed' : 'failed'; - output.startTime = testResult.perfStats.start; - output.endTime = testResult.perfStats.end; - output.coverage = codeCoverageFormatter(testResult.coverage, reporter); - } - - output.assertionResults = testResult.testResults.map(formatTestAssertion); - - if (testResult.failureMessage) { - output.message = testResult.failureMessage; + return { + assertionResults, + coverage: codeCoverageFormatter + ? (testResult.coverage, reporter) + : testResult.coverage, + endTime: testResult.perfStats.end, + message: testResult.failureMessage ? testResult.failureMessage : '', + name: testResult.testFilePath, + startTime: testResult.perfStats.start, + status: allTestsPassed ? 'passed' : 'failed', + summary: '', + }; } - - return output; }; function formatTestAssertion( @@ -72,13 +72,11 @@ function formatTestAssertion( export default function formatTestResults( results: AggregatedResult, - codeCoverageFormatter?: CodeCoverageFormatter | null, + codeCoverageFormatter?: CodeCoverageFormatter, reporter?: CodeCoverageReporter, ): FormattedTestResults { - const formatter = codeCoverageFormatter || (coverage => coverage); - const testResults = results.testResults.map(testResult => - formatResult(testResult, formatter, reporter), + formatTestResult(testResult, codeCoverageFormatter, reporter), ); return {...results, testResults}; diff --git a/packages/jest-test-result/src/types.ts b/packages/jest-test-result/src/types.ts index b24c117d0834..4d3dc7c4257d 100644 --- a/packages/jest-test-result/src/types.ts +++ b/packages/jest-test-result/src/types.ts @@ -126,7 +126,7 @@ export type TestResult = { unmatched: number; updated: number; }; - sourceMaps: { + sourceMaps?: { [sourcePath: string]: string; }; testExecError?: SerializableError; From 8413503a821bfc7b123a09374182534652459353 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Thu, 28 Mar 2019 18:35:54 -0700 Subject: [PATCH 2/5] Restore accidental deletion. --- packages/jest-runner/src/runTest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/jest-runner/src/runTest.ts b/packages/jest-runner/src/runTest.ts index 63ff7606e826..18da0d5de17b 100644 --- a/packages/jest-runner/src/runTest.ts +++ b/packages/jest-runner/src/runTest.ts @@ -253,6 +253,7 @@ async function runTestInternal( result.perfStats = {end: Date.now(), start}; result.testFilePath = path; + result.console = testConsole.getBuffer(); result.skipped = testCount === result.numPendingTests; result.displayName = config.displayName; From 1f9907c839bfdc18c5a429d2617c39e3771ca2f3 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Thu, 28 Mar 2019 18:44:22 -0700 Subject: [PATCH 3/5] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ccf8f3e6c99..93329f978ad7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ - `[jest-core]` Improve performance of SearchSource.findMatchingTests by 15% ([#8184](https://github.com/facebook/jest/pull/8184)) - `[jest-resolve]` Optimize internal cache lookup performance ([#8183](https://github.com/facebook/jest/pull/8183)) - `[jest-core]` Dramatically improve watch mode performance ([#8201](https://github.com/facebook/jest/pull/8201)) +- `[jest-reporters]` Fix memory leak of source map info and minor performance improvements ([#8234](https://github.com/facebook/jest/pull/8234)) ## 24.5.0 From 36b621be8e026d26379c183c3dfaa08b5d2e4595 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Thu, 28 Mar 2019 23:42:04 -0700 Subject: [PATCH 4/5] Review feedback. --- packages/jest-test-result/src/formatTestResults.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jest-test-result/src/formatTestResults.ts b/packages/jest-test-result/src/formatTestResults.ts index 7eae5d01eeb7..47720db16a29 100644 --- a/packages/jest-test-result/src/formatTestResults.ts +++ b/packages/jest-test-result/src/formatTestResults.ts @@ -41,10 +41,10 @@ const formatTestResult = ( return { assertionResults, coverage: codeCoverageFormatter - ? (testResult.coverage, reporter) + ? codeCoverageFormatter(testResult.coverage, reporter) : testResult.coverage, endTime: testResult.perfStats.end, - message: testResult.failureMessage ? testResult.failureMessage : '', + message: testResult.failureMessage || '', name: testResult.testFilePath, startTime: testResult.perfStats.start, status: allTestsPassed ? 'passed' : 'failed', From 69244bff25e5d162fe5c4cbe5b9dd398ad821864 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Fri, 29 Mar 2019 08:43:18 -0700 Subject: [PATCH 5/5] Move memory release to core. --- packages/jest-core/src/ReporterDispatcher.ts | 4 ++++ packages/jest-reporters/src/coverage_reporter.ts | 5 ----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/jest-core/src/ReporterDispatcher.ts b/packages/jest-core/src/ReporterDispatcher.ts index 00aa37f3f287..6e802af5cf71 100644 --- a/packages/jest-core/src/ReporterDispatcher.ts +++ b/packages/jest-core/src/ReporterDispatcher.ts @@ -36,6 +36,10 @@ export default class ReporterDispatcher { reporter.onTestResult && (await reporter.onTestResult(test, testResult, results)); } + + // Release memory if unused later. + testResult.sourceMaps = undefined; + testResult.coverage = undefined; } async onTestStart(test: Test) { diff --git a/packages/jest-reporters/src/coverage_reporter.ts b/packages/jest-reporters/src/coverage_reporter.ts index 6bbb00cbb337..c0832f4daab6 100644 --- a/packages/jest-reporters/src/coverage_reporter.ts +++ b/packages/jest-reporters/src/coverage_reporter.ts @@ -53,12 +53,8 @@ export default class CoverageReporter extends BaseReporter { testResult: TestResult, _aggregatedResults: AggregatedResult, ) { - // Coverage and source maps are only appended to the test result for this - // handler. Delete when processed to preserve memory. - if (testResult.coverage) { this._coverageMap.merge(testResult.coverage); - testResult.coverage = undefined; } const sourceMaps = testResult.sourceMaps; @@ -81,7 +77,6 @@ export default class CoverageReporter extends BaseReporter { } } }); - testResult.sourceMaps = undefined; } }