Skip to content
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

Jump to bottom

Add cpu profiling #2536

Closed
wants to merge 4 commits into from
Closed

Add cpu profiling #2536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de5a46f
Make sure child processes exit when disposed
Lakitna Oct 7, 2020
a4f2bbf
Always run in profiling mode
Lakitna Oct 7, 2020
76944f7
Only profile when the environment variable `STRYKER_PROFILE` is set
Lakitna Oct 7, 2020
6a3825a
Made CPU profiling flag-based instead of env var based
Lakitna Oct 15, 2020
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
5 changes: 5 additions & 0 deletions packages/api/schema/stryker-core.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,11 @@
}
],
"description": "Enable or disable certain warnings"
},
"cpuProf": {
"description": "TODO: Describe",
"default": false,
"type": "boolean"
}
}
}
1 change: 1 addition & 0 deletions packages/core/src/StrykerCli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export default class StrykerCli {
this.command = cmd;
this.strykerConfig = config;
})
.option('--cpu-prof', 'TODO: Write description. Note that this will not profile the main process!')
.option(
'-f, --files <allFiles>',
'A comma separated list of globbing expression used for selecting all files needed to run the tests. For a more detailed way of selecting input files, please use a configFile. Example: node_modules/a-lib/**/*.js,src/**/*.js,!src/index.js,a.js,test/**/*.js',
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/child-proxy/ChildProcessProxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const BROKEN_PIPE_ERROR_CODE = 'EPIPE';
const IPC_CHANNEL_CLOSED_ERROR_CODE = 'ERR_IPC_CHANNEL_CLOSED';
const TIMEOUT_FOR_DISPOSE = 2000;

let n = 0;
Copy link
Member

Choose a reason for hiding this comment

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

What is this doing here? n is never updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

n is updated on 56 and used on 55. Basically, I use it to enumerate the child process output files to check how many of them are missing. Another naming scheme is fine with me.

Copy link
Member

Choose a reason for hiding this comment

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

Should we not reuse this n? For example, with --concurrency 4, we might want to use 1, 2, 3, and 4, but never 5. Whenever a child process crashes, it can reuse the number.

Copy link
Contributor Author

@Lakitna Lakitna Oct 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

No, that'll result in overwritten .cpuprofile files.

n here is not a good solution by any means, but it is a human-readable approach to unique file names. Doing it sequenced like this allows a human to see how many processes crashed, as crashed processes have no .cpuprofile file.

For example:

image

We can see that 1, 3, 6-10, 12-14, and 17 are missing here. These processes must have crashed for some reason.

This insight is the only reason why we need n.

Edit: The last file of the run is child-process-278.cpuprofile. I have a feeling that there are many more processes than you're expecting.

export default class ChildProcessProxy<T> implements Disposable {
public readonly proxy: Promisified<T>;

Expand All @@ -48,7 +49,14 @@ export default class ChildProcessProxy<T> implements Disposable {
additionalInjectableValues: unknown,
workingDirectory: string
) {
this.worker = fork(require.resolve('./ChildProcessProxyWorker'), [autoStart], { silent: true, execArgv: [] });
const execArgv = [];
if (options.cpuProf) {
execArgv.push('--cpu-prof');
execArgv.push(`--cpu-prof-name=child-process-${n}.cpuprofile`);
n++;
}

this.worker = fork(require.resolve('./ChildProcessProxyWorker'), [autoStart], { silent: true, execArgv: execArgv });
this.initTask = new Task();
this.log.debug('Starting %s in child process %s', requirePath, this.worker.pid);
this.send({
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/child-proxy/ChildProcessProxyWorker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ export default class ChildProcessProxyWorker {
const sendCompleted = () => {
this.send({ kind: ParentMessageKind.DisposeCompleted });
};
LogConfigurator.shutdown().then(sendCompleted).catch(sendCompleted);
LogConfigurator.shutdown()
.then(sendCompleted)
.catch(sendCompleted)
// Exit gracefully so we can generate CPU profiles for these processes.
.finally(() => process.exit(0));
break;
}
}
Expand Down