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

os.cpus() docs misleading #16279

Closed
lukechilds opened this issue Oct 18, 2017 · 3 comments
Closed

os.cpus() docs misleading #16279

lukechilds opened this issue Oct 18, 2017 · 3 comments
Labels
doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem.

Comments

@lukechilds
Copy link
Contributor

lukechilds commented Oct 18, 2017

The docs for os.cpus() state:

The os.cpus() method returns an array of objects containing information about each CPU/core installed.

It would be more accurate to use the word "threads" rather than "CPU/core". Not sure if this behaviour is intentional or due to how the OS reports cores. Either way, a note in the docs would be helpful to clarify this. Do you want a PR?

Example:

$ sysctl -n machdep.cpu.brand_string
Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz

$ sysctl -n machdep.cpu.core_count
2

$ sysctl -n machdep.cpu.thread_count
4

$ node -e "console.log(require('os').cpus().length)"
4
@joyeecheung joyeecheung added doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem. labels Oct 18, 2017
@joyeecheung
Copy link
Member

joyeecheung commented Oct 18, 2017

The word "threads" seems a bit confusing here. AFAICT os.cpus() returns information about each logical core on Linux, MacOS and Windows, so a better phrasing would be ..about each logical CPU/core installed, or just add a note mentioning it reports stats on logical cores instead of physical ones (maybe + a link explaining what hyperthreading is).

@lukechilds
Copy link
Contributor Author

Logical core seems like a more accurate description 👍

@joyeecheung
Copy link
Member

@lukechilds A PR would be very welcomed. Check out our contributing guide if you want to submit one. :)

lukechilds added a commit to lukechilds/node that referenced this issue Oct 18, 2017
Original text implied it returns physical cpu cores. This is misleading
as `os.cpus().length` is often used to get the CPU core count in
Node.js. In reality that will return the thread count which may not be
what the user intended.

Fixes: nodejs#16279
addaleax pushed a commit to ayojs/ayo that referenced this issue Oct 18, 2017
Original text implied it returns physical cpu cores. This is misleading
as `os.cpus().length` is often used to get the CPU core count in
Node.js. In reality that will return the thread count which may not be
what the user intended.

PR-URL: nodejs/node#16282
Fixes: nodejs/node#16279
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this issue Oct 18, 2017
Original text implied it returns physical cpu cores. This is misleading
as `os.cpus().length` is often used to get the CPU core count in
Node.js. In reality that will return the thread count which may not be
what the user intended.

PR-URL: #16282
Fixes: #16279
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 16, 2017
Original text implied it returns physical cpu cores. This is misleading
as `os.cpus().length` is often used to get the CPU core count in
Node.js. In reality that will return the thread count which may not be
what the user intended.

PR-URL: #16282
Fixes: #16279
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 21, 2017
Original text implied it returns physical cpu cores. This is misleading
as `os.cpus().length` is often used to get the CPU core count in
Node.js. In reality that will return the thread count which may not be
what the user intended.

PR-URL: #16282
Fixes: #16279
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 28, 2017
Original text implied it returns physical cpu cores. This is misleading
as `os.cpus().length` is often used to get the CPU core count in
Node.js. In reality that will return the thread count which may not be
what the user intended.

PR-URL: #16282
Fixes: #16279
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants