-
-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Rename NVM_CPU_THREADS to NVM_CPU_CORES #1326
Conversation
if [ "$NVM_CPU_THREADS" -gt 2 ]; then | ||
NVM_MAKE_JOBS=$((NVM_CPU_THREADS - 1)) | ||
nvm_echo "Set the number of jobs to $NVM_CPU_THREADS - 1 = $NVM_MAKE_JOBS jobs to speed up the build" | ||
nvm_echo "Detected that you have $NVM_CPU_CORES CPU thread(s)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably alter this text to be more accurate too?
Updated, PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Is this really correct? I think we do use CPU threads not cores directly here, for example: Intel® Core™ i7-4790 Processor has only 4 cores but 8 threads: |
@PeterDaveHello then on that processor, the number of make jobs should be 3. |
It'll be 7, as you'll see 8 cores there, and it'll be faster than 3. |
Perhaps the terms "core" and "thread" are used a bit loosely? My intention is that there should always be at least one execution context, that is not from |
I don't think the terms "core" and "thread" are used a bit loosely, I'll call it threads but not cores, they are different, at least, call them CPUs, as other common programs, I didn't see any common case which really needs to take the physical cores rather than the threads on the cores, maybe I should just send a PR to modify it again to call it If you really think we should use 3 cores not 7 threads in the hyper-threading cases, you should use the
|
Do we really need to use the term "CORE" here? Maybe should open an issue for the discussion? |
Sure, an issue is great. |
@PeterDaveHello I'd say that Cores is still more appropriate than threads. I agree that if you have an Intel core with hyperthreading then you can run two threads at a time on each core, but most OSs spin up a bunch of threads and context switch between them, and I'd say that's the more common meaning of threads. From the OS, a processor with 4 physical cores and hyperthreading (or a single Power8 processor) would look like 8 "virtual cores" to the OS. Happy to discuss further on an issue! |
@gibfahn just reply to your activity monitor here, the threads term here is not about how many threads we have on CPU but how many threads the software created on the OS. |
@PeterDaveHello that's what "threads" most commonly means to programmers - software threads created by the OS. |
Following #1319 , rename NVM_CPU_THREADS to NVM_CPU_CORES
cc/ @GeorgeAdams95