Node: Change os.cpus() output to report whether a core is physical or logical

[httpdigest]

• Version:v7.0.0-pre (custom VS2015 build from commit a58b48b)
• Platform:Windows 7 x64
• Subsystem:?

os.cpus() returns an array of the logical CPUs. It would be nice to have a function returning the actual number of physical cores.

Implementation on Win32 (>= Windows XP) could use GetLogicalProcessorInformation. That name is pretty misleading as it can also return the number of _physical_ cores as seen in the provided example code.

[httpdigest]

Did a quick test on Win32 (Windows 7 x64) based on the MSDN example. The following works for me:

#include <windows.h>
#include <malloc.h>
#include <stdio.h>
typedef BOOL (WINAPI *LPFN_GLPI)(PSYSTEM_LOGICAL_PROCESSOR_INFORMATION, PDWORD);
static int numPhysicalCpus() {
    LPFN_GLPI glpi;
    BOOL done = FALSE;
    PSYSTEM_LOGICAL_PROCESSOR_INFORMATION buffer = NULL;
    PSYSTEM_LOGICAL_PROCESSOR_INFORMATION ptr = NULL;
    DWORD returnLength = 0;
    DWORD processorCoreCount = 0;
    DWORD byteOffset = 0;
    glpi = (LPFN_GLPI) GetProcAddress(GetModuleHandle(TEXT("kernel32")), "GetLogicalProcessorInformation");
    if (NULL == glpi)
        return -1; // <- Error: GetLogicalProcessorInformation is not supported!
    while (!done) {
        DWORD rc = glpi(buffer, &returnLength);
        if (FALSE == rc) {
            if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
                if (buffer) free(buffer);
                buffer = (PSYSTEM_LOGICAL_PROCESSOR_INFORMATION)malloc(returnLength);
                if (NULL == buffer) return -1; // <- Error: Allocation failure!
            } else
                return -1; // <- Error: See GetLastError();
        } else done = TRUE;
    }
    ptr = buffer;
    while (byteOffset + sizeof(SYSTEM_LOGICAL_PROCESSOR_INFORMATION) <= returnLength) {
        switch (ptr->Relationship) {
        case RelationProcessorCore:
            processorCoreCount++;
            break;
        default:
            break;
        }
        byteOffset += sizeof(SYSTEM_LOGICAL_PROCESSOR_INFORMATION);
        ptr++;
    }
    free(buffer);
    return processorCoreCount;
}
int main(int argc, char** argv) {
  printf("Physical cores: %d\n", numPhysicalCpus());
  return 0;
}

It prints:

Physical cores: 4

whereas os.cpu().length is 8

[ChALkeR]

Perhaps it would make more sense to just add more keys to the current output of os.cpus()?

[httpdigest]

Good idea! Something like a "isPhysical" or the like, to easily filter() over it.

[rvagg]

Is it possible to do an isPhysical though? Can you make a distinction between them in that list? Or would it just be "pick the first X and add isPhysical=true"?

[targos]

FWIW /proc/cpuinfo outputs a "physical id" property that holds more accurate information

[httpdigest]

I just had a look at node's sources, how this is accomplished for the various platforms. Internally, node has GetCPUInfo in src/node_os.cc. This in turn calls uv_cpu_info of the uv library in deps/uv. For Win32 the implementation of this function is in deps/uv/src/win/util.c.
This in turn calls the Win32 API function GetSystemInfo in line 598, which is insufficient to return the needed information. I guess this has to be changed to use GetLogicalProcessorInformation.
The issue is probably identical on other platforms.

[ChALkeR]

Change os.cpus() output to report whether a core is physical or logical

It wouldn't work that way. What would likely be possible is just adding the physical_id and core_id params.

[httpdigest]

Fine by me. :) I just would like to have a way to figure out how many physical cores are on the system, for clustering. If I need to do a distinct(core_id), so to speak, I'm fine with that.
But anyway, we need to find a solution that would work on all platforms.

[rvagg]

I think this has to move to https://github.com/libuv/libuv where the work needs to take place before coming back here. I'm +1 on doing this if we can get reliable information from libuv.

[ChALkeR]

@httpdigest Btw, to clear possible misunderstanding in the terminology here (in regards to physical/logical cores).

For example, a setup with 1 cpu (aka 1 socket) that has 4 real (physical) cores and hyper-threading, would have this set of virtual (logical) cores:

1. physical_id: 0, core_id: 0
2. physical_id: 0, core_id: 0
3. physical_id: 0, core_id: 1
4. physical_id: 0, core_id: 1
5. physical_id: 0, core_id: 2
6. physical_id: 0, core_id: 2
7. physical_id: 0, core_id: 3
8. physical_id: 0, core_id: 3

Btw, just distinct(core_id) won't give the correct result on a multi-cpu system, you would need distinct(physical_id, core_id).

[saghul]

This would need extending uv_cpu_info for adding the extra fields. FYI: This would break the ABI so it can't happen before v2.x.

[httpdigest]

For example, a setup with 1 cpu (aka 1 socket) that has 4 cores and hyper-threading, would have this set of virtual (logical) cores:

There is no misunderstanding, but thanks for the example. :) I then still mean distinct(core_id), so that I know it has 4 cores.

EDIT: Ah right, I just saw your writing about multi-core systems. You're right, a single distinct(core_id) is not enough in that case.
This all sounds quite like http://www.open-mpi.de/projects/hwloc/ now.

[Fishrock123]

Would it make more sense to have a property that maps to a numbered physical core?

e.g.physical: 2 from threads 3,4 on an i7

[httpdigest]

Correct me if I'm wrong, but isn't that what @ChALkeR is proposing? A property which tells which physical core it is on. Just in his case it is named core_id and starts counting from 0.

[Trott]

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

[gibfahn]

Sounds like this should be moved to libuv/libuv