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

os: add availableParallelism() #45895

Merged
merged 2 commits into from Dec 21, 2022
Merged

os: add availableParallelism() #45895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
288c421
os: add availableParallelism()
cjihrig Dec 17, 2022
dfbe36f
doc: add parallelism note to os.cpus()
cjihrig Dec 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions doc/api/os.md
View file
Open in desktop
Expand Up @@ -26,6 +26,19 @@ The operating system-specific end-of-line marker.
* `\n` on POSIX
* `\r\n` on Windows

## `os.availableParallelism()`
cjihrig marked this conversation as resolved.
Show resolved Hide resolved

<!-- YAML
added: REPLACEME
-->

* Returns: {integer}

Returns an estimate of the default amount of parallelism a program should use.
Always returns a value greater than zero.
Copy link
Member

Choose a reason for hiding this comment

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

Some explanation about how that estimate is reached would be helpful here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the PR that added this functionality to libuv - libuv/libuv@f250c6c. If you have some exact text you'd like to see based on that, I'm happy to add it. The CI has been so flaky (and frustrating) that I don't want to go back and forth with pushing changes, running the CI, making more changes, etc. - I'd rather just ship whatever suggestions you have.

Alternatively, a docs update could be added after the fact since the CI requirements for docs only changes are much lower.

Copy link
Member

Choose a reason for hiding this comment

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

The implementation of uv_available_parallelism() will likely evolve over time so the docs here shouldn't be too specific, or they'll become outdated.

My suggestions are to a) link to the libuv documentation, b) leave it out, or c) say we consult the animating spirits inside the computer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to see how option C plays out.

Copy link
Member

Choose a reason for hiding this comment

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

Returns an estimate of the default amount of parallelism a program should use. Always returns a value greater than zero.

On Linux, inspects the calling thread’s CPU affinity mask to determine if it has been pinned to specific CPUs.

On Windows, the available parallelism may be underreported on systems with more than 64 logical CPUs.

On other platforms, reports the number of CPUs that the operating system considers to be online.

The heuristics used are expected to evolve over time and may vary across operating systems.

Or, if nothing else, link to the libuv documentation.

Copy link

Choose a reason for hiding this comment

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

Does not answer the question of what it does inside of a cgroup, which is the primary problem with cpus()

Copy link
Member

Choose a reason for hiding this comment

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

It's not mentioned because it's oblivious to the presence of cgroups.

Copy link

@jdmarshall jdmarshall Jan 6, 2023 •
edited

Choose a reason for hiding this comment

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

I'm not a fan of adding new features that replicate open bugs in existing functionality.

Also #28855 specifically asks for this problem (cgoups causing process count to be wrong) to be fixed, which is what my question is asking. Obliviousness is exactly the problem. cpus().length is oblivious to cgroups and gives the processor count of the host, not what's available to the container.

If the answer is 'no', we still don't have a solution to the issue that was marked closed.

Copy link

@jdmarshall jdmarshall Jan 6, 2023 •
edited

Choose a reason for hiding this comment

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

I'm lousy at reading C code, but I think this is saying:

libuv/libuv@f250c6c
https://www.cygwin.com/bugzilla/show_bug.cgi?id=27645

That sysconf() works on musl (eg, Alpine Linux?) but not on glibc (eg, Ubuntu, CoreOS). But the extra call in the conditional block in the libuv commit above emulates the musl behavior. If I've followed that right, then the answer is 'yes' and it's not oblivious to cgroups.

Copy link
Member

Choose a reason for hiding this comment

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

It's oblivious in the "cache oblivious algorithm" sense: libuv doesn't have to care whether cgroups are active or not; if the container is set up properly, it'll produce the right answer.


This function is a small wrapper about libuv's [`uv_available_parallelism()`][].

## `os.arch()`

<!-- YAML
Expand Down Expand Up @@ -127,6 +140,10 @@ The properties included on each object include:
`nice` values are POSIX-only. On Windows, the `nice` values of all processors
are always 0.

`os.cpus().length` should not be used to calculate the amount of parallelism
available to an application. Use
[`os.availableParallelism()`](#osavailableparallelism) for this purpose.

## `os.devNull`

<!-- YAML
Expand Down Expand Up @@ -1340,3 +1357,4 @@ The following process scheduling constants are exported by
[`process.arch`]: process.md#processarch
[`process.platform`]: process.md#processplatform
[`uname(3)`]: https://linux.die.net/man/3/uname
[`uv_available_parallelism()`]: https://docs.libuv.org/en/v1.x/misc.html#c.uv_available_parallelism
3 changes: 3 additions & 0 deletions lib/os.js
View file
Open in desktop
Expand Up @@ -45,6 +45,7 @@ const {
const { validateInt32 } = require('internal/validators');

const {
getAvailableParallelism,
getCPUs,
getFreeMem,
getHomeDirectory: _getHomeDirectory,
Expand Down Expand Up @@ -100,6 +101,7 @@ const getOSVersion = () => version;
*/
const getMachine = () => machine;

getAvailableParallelism[SymbolToPrimitive] = () => getAvailableParallelism();
getFreeMem[SymbolToPrimitive] = () => getFreeMem();
getHostname[SymbolToPrimitive] = () => getHostname();
getOSVersion[SymbolToPrimitive] = () => getOSVersion();
Expand Down Expand Up @@ -364,6 +366,7 @@ function userInfo(options) {

module.exports = {
arch,
availableParallelism: getAvailableParallelism,
cpus,
endianness,
freemem: getFreeMem,
Expand Down
7 changes: 7 additions & 0 deletions src/node_os.cc
View file
Open in desktop
Expand Up @@ -380,6 +380,10 @@ static void GetPriority(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(priority);
}

static void GetAvailableParallelism(const FunctionCallbackInfo<Value>& args) {
unsigned int parallelism = uv_available_parallelism();
args.GetReturnValue().Set(parallelism);
cjihrig marked this conversation as resolved.
Show resolved Hide resolved
}

void Initialize(Local<Object> target,
Local<Value> unused,
Expand All @@ -397,6 +401,8 @@ void Initialize(Local<Object> target,
SetMethod(context, target, "getUserInfo", GetUserInfo);
SetMethod(context, target, "setPriority", SetPriority);
SetMethod(context, target, "getPriority", GetPriority);
SetMethod(
context, target, "getAvailableParallelism", GetAvailableParallelism);
SetMethod(context, target, "getOSInformation", GetOSInformation);
target
->Set(context,
Expand All @@ -417,6 +423,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(GetUserInfo);
registry->Register(SetPriority);
registry->Register(GetPriority);
registry->Register(GetAvailableParallelism);
registry->Register(GetOSInformation);
}

Expand Down
4 changes: 4 additions & 0 deletions test/parallel/test-os.js
View file
Open in desktop
Expand Up @@ -255,6 +255,8 @@ if (!common.isIBMi) {
is.number(os.uptime(), 'uptime');
}

is.number(+os.availableParallelism, 'availableParallelism');
is.number(os.availableParallelism(), 'availableParallelism');
is.number(+os.freemem, 'freemem');
is.number(os.freemem(), 'freemem');

Expand All @@ -264,3 +266,5 @@ if (common.isWindows) {
} else {
assert.strictEqual(devNull, '/dev/null');
}

assert.ok(os.availableParallelism() > 0);