Count processors via affinity mask on linux

[kubo39]

This Fixes #12

[seanmonstar]

Hey, thanks for all this! This is impressive!

I just have some questions left in comments.

[seanmonstar]

I wonder whether using affinity by default is better, or if it'd be a sort of breaking change. Would one always always want the affinity, or could one conceivably want to just know the number or CPUs on the machine anyways?

Put another way, should this be a separate function, or just be the default and only implementation for linux? I'm not saying one or the other is correct, I'd be interested in rationale for either case.

[seanmonstar]

Is there large benefits to using this asm as opposed to not requiring unstable features and using the bit masking?

[sfackler]

This function exists in the standard library: https://doc.rust-lang.org/std/primitive.u64.html#method.count_ones

[kubo39]

It's just taken from my perfromance sensitive code written in C.
count_ones is just what I wanted, thanks!

[seanmonstar]

I've moved the question out of the line comment, so that edits don't make it disappear:

I wonder whether using affinity by default is better, or if it'd be a sort of breaking change. Would one always always want the affinity, or could one conceivably want to just know the number or CPUs on the machine anyways?

Put another way, should this be a separate function, or just be the default and only implementation for linux? I'm not saying one or the other is correct, I'd be interested in rationale for either case.

[kubo39]

** updated **

I researched some other language implementations on Linux.

Counts via affinity mask

golang

• runtime.NumCPU: the number of logical CPUs usable by the current process.
  https://golang.org/pkg/runtime/#NumCPU

nproc(1)

• counts cores via affinity mask
  http://man7.org/linux/man-pages/man1/nproc.1.html

Ruby

• Etc.nprocessors: number of processes to use all available processors
  https://ruby-doc.org/stdlib-2.2.0/libdoc/etc/rdoc/Etc.html#method-c-nprocessors

Counts by sysconf(_SC_NPROCESSORS_ONLN)

D

• std.parallelism.totalCPUs: total number of CPU cores available on the current machine
  https://dlang.org/phobos/std_parallelism.html#.totalCPUs

Both

python

• os.cpu_count: number of CPUs in the system
• len(os.sched_getaffinity(0)): number of usable CPUs can be obtained with
  https://docs.python.org/3.6/library/os.html#os.cpu_count

ref: https://github.com/kubo39/cpucount

[seanmonstar]

I'm slightly inclined to follow golang, but welcome other opinions. I also wonder if that's breaking change (or a "bug fix").

[seanmonstar]

My current feelings are:

• get() should check affinity. It's the "default" function to call, and I assume most people using it are just wanting to know how many threads or processes they should spawn, so it's actually bad to report more CPUs than the process can actually use.
• get_physical() is already there for those who need that specific datum.
• get_all() or something could be added later if there is really a need to know ALL cores, physical and virtual.

So, I'm considering merging this as-is, unless there's other thoughts.

[brson]

@seanmonstar I agree that the default behavior should give you a reasonable number of threads to spawn.

I'm not sure about whether it's a breaking change or not, but I'd say it should get a major bump.

[pedrocr]

@seanmonstar the reason for get_physical() is to get the same thing as get() but without hyperthreading. I agree that it makes sense that get() returns the threads you can use but then get_physical() should return the physical threads you can use. Otherwise it's inconsistent. So I think it makes more sense to do 4 functions having normal/physical versions of with/without affinity and have the default be the affinity ones. So:

• get() is all threads with affinity
• get_physical() is all physically sepparate threads with affinity
• get_all() is all threads without affinity
• get_all_physical() is the current get_physical()

To do this what we're missing is the get_physical() implementation where affinity is taken into account.

[seanmonstar]

@pedrocr that makes sense, thanks for the insight! I've opened #40 for tracking that.

[seanmonstar]

Thanks @kubo39! Excellent work!