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.

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

Allow more execArgv flags to be passed to workers #4382

Closed
4 tasks done
adriencaccia opened this issue Oct 27, 2023 · 2 comments · Fixed by #4383
Closed
4 tasks done

Allow more execArgv flags to be passed to workers #4382

adriencaccia opened this issue Oct 27, 2023 · 2 comments · Fixed by #4383

Comments

@adriencaccia
Copy link
Contributor

adriencaccia commented Oct 27, 2023

Clear and concise description of the problem

I need to pass execution flags to the workers to fully support vitest on CodSpeed (more details below). At this time, execution flags are all filtered out except --cpu-prof and --heap-prof.
The flags are described here: https://github.com/CodSpeedHQ/codspeed-node/blob/707a076f6178125fa1042f4a4decd51d11b03b56/packages/core/src/introspection.ts#L5-L21

Suggested solution

It is a touchy subject that can lead to bugs as already stated in the code comment below and in #2702.
I would propose some kind of escape hatch for library authors where we can remove the filtering entirely. Using a specific env variable for example.

The change would need to be done in

// Instead of passing whole process.execArgv to the workers, pick allowed options.
// Some options may crash worker, e.g. --prof, --title. nodejs/node#41103
const execArgv = process.execArgv.filter(execArg =>
execArg.startsWith('--cpu-prof') || execArg.startsWith('--heap-prof'),
)

Alternative

Expose an option directly in vitest config, to specify the flags to be passed to the workers.

Additional context

I am working on adding first class vitest support to CodSpeed, I started the work at CodSpeedHQ/codspeed-node#26. In order to have automated flame graphs with each benchmark, we need to pass a few additional v8 args to the workers.
I managed to get it working with a patch of vitest removing the filtering.

With pool: 'forks', all the flags needed by CodSpeed are working, as this job is working https://github.com/CodSpeedHQ/codspeed-node/actions/runs/6668657521/job/18124687199.

However, I tested it with the default pool: 'threads' and got an error because one of the flags was not supported, highlighting that allowing all flags for every pool method is definitely not the way to go.

I would gladly do a PR if and when we agree on what API to expose 😃

Validations

@sheremet-va
Copy link
Member

Maybe we can expose a way to pass down execArgs in poolOptions.{threads,forks,vmThreads}?

@adriencaccia
Copy link
Contributor Author

Yep that would work for me!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants