Skip to content

Commit

Permalink
Fix memory leak around global.process
Browse files Browse the repository at this point in the history
Summary: This memory leak was discovered in #584 and has always been present in jest. It likely affects the runtime performance of jest on low-memory machines *a lot* and we don't notice it much on devservers. I will tag and release 0.7.0 with this fix (because 0.7.0 also removes a bunch of APIs, see the changelog, it cannot be a patch release).

Two samples from before and after:

* P20276376 (150 mb vs 47 mb on 34 Relay tests)
* P20276601 (54 mb vs 38 mb flat on the functional tests)

I also added a way to pretty print the output (see the second paste). By adding `--logHeapUsage` to jest it adds these statistics after every test run. When node's global `gc` function is exposed, it calls it beforehand to make sure we are logging accurate data. I am going to think about whether it makes sense to expose this more easily (adding `--runInBand` etc. is slightly confusing) if I am confident it isn't bad for performance. On automated test runs you might want to log this and be able to log the final memory usage to whatever logging system you use to be able to track memory usage of test runs (and applications) over time.

Note: It is time we rewrite jest's configs *completely*. I have plans for this and it is a discussion for a separate diff. For now I have to copy-pasta some code to make everything work in the internal and external runners and tack on the argv data to the config objects.

public

Reviewed By: DmitrySoshnikov

Differential Revision: D2608363

fb-gh-sync-id: ab3bda143c22ef06e2e1b32a4c92d8ad0a2151cb
  • Loading branch information
cpojer authored and facebook-github-bot-3 committed Nov 3, 2015
1 parent 615789a commit 4e12731
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 27 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,9 @@
## 0.7.0

* Fixed a memory leak with test contexts. Jest now properly cleans up
test environments after each test. Added `--logHeapUsage` to log memory
usage after each test. Note: this is option is meant for debugging memory
leaks and might significantly slow down your test run.
* Removed `mock-modules`, `node-haste` and `mocks` virtual modules. This is a
breaking change of undocumented public API. Usage of this API can safely be
automatically updated through an automated codemod:
Expand Down
7 changes: 7 additions & 0 deletions bin/jest.js
Expand Up @@ -165,6 +165,13 @@ const argv = optimist
),
type: 'boolean',
},
logHeapUsage: {
description: _wrapDesc(
'Logs the heap usage after every test. Useful to debug memory ' +
'leaks. Use together with `--runInBand` and `--expose-gc` in node.'
),
type: 'boolean',
},
})
.check(function(argv) {
if (argv.runInBand && argv.hasOwnProperty('maxWorkers')) {
Expand Down
21 changes: 15 additions & 6 deletions src/DefaultTestReporter.js
Expand Up @@ -49,16 +49,25 @@ class DefaultTestReporter {
? (testResult.perfStats.end - testResult.perfStats.start) / 1000
: null;

let testRunTimeString;
let testDetail = [];
if (testRunTime !== null) {
testRunTimeString = '(' + testRunTime + 's)';
if (testRunTime > 2.5) {
testRunTimeString = this._formatMsg(testRunTimeString, FAIL_COLOR);
}
testDetail.push(
testRunTime > 2.5
? this._formatMsg(testRunTime + 's', FAIL_COLOR)
: testRunTime + 's'
);
}

if (testResult.memoryUsage) {
const toMB = bytes => Math.floor(bytes / 1024 / 1024);
testDetail.push(
`${toMB(testResult.memoryUsage)} MB current`,
`${toMB(testResult.maxMemoryUsage)} MB max`
);
}

const resultHeader = this._getResultHeader(allTestsPassed, pathStr, [
testRunTimeString,
(testDetail.length ? '(' + testDetail.join(', ') + ')' : null),
]);

/*
Expand Down
4 changes: 3 additions & 1 deletion src/JSDomEnvironment.js
Expand Up @@ -26,7 +26,9 @@ class JSDomEnvironment {

// Forward some APIs
this.global.Buffer = Buffer;
this.global.process = process;
// `this.global.process` is mutated in FakeTimers. Make a copy of the
// object for the jsdom environment to prevent memory leaks.
this.global.process = Object.assign({}, process);
this.global.setImmediate = setImmediate;
this.global.clearImmediate = clearImmediate;

Expand Down
51 changes: 31 additions & 20 deletions src/TestRunner.js
Expand Up @@ -70,13 +70,14 @@ function optionPathToRegex(p) {
* @param options See DEFAULT_OPTIONS for descriptions on the various options
* and their defaults.
*/

class TestRunner {

constructor(config, options) {
this._config = config;
this._configDeps = null;
this._moduleLoaderResourceMap = null;
// Maximum memory usage if `logHeapUsage` is enabled.
this._maxMemoryUsage = 0;
this._testPathDirsRegExp = new RegExp(
config.testPathDirs
.map(dir => optionPathToRegex(dir))
Expand All @@ -91,6 +92,7 @@ class TestRunner {
.join('|') +
')$'
);

// Map from testFilePath -> time it takes to run the test. Used to
// optimally schedule bigger test runs.
this._testPerformanceCache = null;
Expand Down Expand Up @@ -375,22 +377,27 @@ class TestRunner {

const testExecStats = {start: Date.now()};
return testRunner(config, env, moduleLoader, testFilePath)
.then(results => {
.then(result => {
testExecStats.end = Date.now();

results.perfStats = testExecStats;
results.testFilePath = testFilePath;
results.coverage =
result.perfStats = testExecStats;
result.testFilePath = testFilePath;
result.coverage =
config.collectCoverage
? moduleLoader.getAllCoverageInfo()
: {};

return results;
return result;
});
}).then(
results => Promise.resolve().then(() => {
result => Promise.resolve().then(() => {
env.dispose();
return results;

if (config.logHeapUsage) {
this._addMemoryUsage(result);
}

return result;
}),
err => Promise.resolve().then(() => {
env.dispose();
Expand Down Expand Up @@ -483,12 +490,10 @@ class TestRunner {
const TestReporter = require(config.testReporter);
if (config.useStderr) {
/* eslint-disable fb-www/object-create-only-one-param */
reporter = new TestReporter(
Object.create(
process,
{ stdout: { value: process.stderr } }
)
);
reporter = new TestReporter(Object.create(
process,
{stdout: {value: process.stderr}}
));
/* eslint-enable fb-www/object-create-only-one-param */
} else {
reporter = new TestReporter();
Expand Down Expand Up @@ -563,19 +568,15 @@ class TestRunner {
.then(this._cacheTestResults.bind(this));
}

_createTestRun(
testPaths, onTestResult, onRunFailure
) {
_createTestRun(testPaths, onTestResult, onRunFailure) {
if (this._opts.runInBand || testPaths.length <= 1) {
return this._createInBandTestRun(testPaths, onTestResult, onRunFailure);
} else {
return this._createParallelTestRun(testPaths, onTestResult, onRunFailure);
}
}

_createInBandTestRun(
testPaths, onTestResult, onRunFailure
) {
_createInBandTestRun(testPaths, onTestResult, onRunFailure) {
let testSequence = Promise.resolve();
testPaths.forEach(testPath =>
testSequence = testSequence
Expand Down Expand Up @@ -617,6 +618,16 @@ class TestRunner {
})
))).then(() => workerFarm.end(farm));
}

_addMemoryUsage(result) {
if (global.gc) {
global.gc();
}
const memoryUsage = process.memoryUsage().heapUsed;
this._maxMemoryUsage = Math.max(this._maxMemoryUsage, memoryUsage);
result.maxMemoryUsage = this._maxMemoryUsage;
result.memoryUsage = memoryUsage;
}
}

function _pathStreamToPromise(stream) {
Expand Down
4 changes: 4 additions & 0 deletions src/jest.js
Expand Up @@ -109,6 +109,10 @@ function _promiseConfig(argv, packageRoot) {
config.useStderr = true;
}

if (argv.logHeapUsage) {
config.logHeapUsage = argv.logHeapUsage;
}

config.noStackTrace = argv.noStackTrace;

return config;
Expand Down
1 change: 1 addition & 0 deletions src/lib/utils.js
Expand Up @@ -261,6 +261,7 @@ function normalizeConfig(config) {
case 'moduleFileExtensions':
case 'noHighlight':
case 'noStackTrace':
case 'logHeapUsage':
case 'cache':
case 'verbose':
value = config[key];
Expand Down

0 comments on commit 4e12731

Please sign in to comment.