Add `setcpuaffinity(cmd, cpus)` for setting CPU affinity of subprocesses #42469

tkf · 2021-10-02T03:53:40Z

While working on implementing tests for #42340, I thought it'd make sense to add a Julia API to spawn processes with an explicit CPU affinity since libuv already can do this. It may also be useful for process-based parallelism libraries like Distributed and Dagger.

So, this patch adds a new API:

setcpuaffinity(original_command::Cmd, cpus) -> command::Cmd
Set the CPU affinity of the command by a list of CPU IDs (1-based) cpus. Passing cpus = nothing means to unset the CPU affinity if the original_command has any.

This is supported on Unix and Windows but not in macOS.

Example:

julia> run(setcpuaffinity(`sh -c 'taskset -p $$'`, [1, 2, 5]));
pid 2273's current affinity mask: 13

julia> 0b010011
0x13

We can now remove taskset from the test dependency.

Side notes: The behavior of cpus = nothing with JULIA_EXCLUSIVE=1 is currently badly defined/implemented #42338. So, we'd need to be careful when documenting its behavior.

What do you think? Is it an appropriate API for Julia Base?

base/cmd.jl

test/print_process_affinity.jl

test/threads.jl

Co-authored-by: Jameson Nash <[email protected]>

KristofferC · 2021-10-02T21:34:40Z

base/cmd.jl

+julia> run(setcpuaffinity(`sh -c 'taskset -p \$\$'`, [1, 2, 5]));
+pid 2273's current affinity mask: 13
+
+julia> 0b010011


The logic required to go from the first command to understand the point of the second feels a bit large here.

Thanks, good point. I added a bit more explanations in the latest commit.

tkf · 2021-10-02T22:21:38Z

test/show.jl

+@testset "Cmd" begin
+    @test sprint(show, `true`) == "`true`"
+    @test sprint(show, setenv(`true`, "A" => "B")) == """setenv(`true`,["A=B"])"""
+    @test sprint(show, setcpuaffinity(`true`, [1, 2])) == "setcpuaffinity(`true`,[1, 2])"
+    @test sprint(show, setenv(setcpuaffinity(`true`, [1, 2]), "A" => "B")) ==
+          """setenv(setcpuaffinity(`true`,[1, 2]),["A=B"])"""
+end


I couldn't find where show(::IO, ::Cmd) was tested. It looks like the original commit dee7f6c didn't add the test for show with setenv. So, I just threw this in for completeness.

tkf · 2021-10-14T00:25:10Z

@vtjnash Can you revisit the above coment? Is it good to go?

vtjnash

Some suggestions

base/cmd.jl

vtjnash · 2021-10-14T14:31:57Z

base/cmd.jl

+setcpuaffinity(cmd::Cmd, ::Nothing) = Cmd(cmd; cpus = nothing)
+setcpuaffinity(cmd::Cmd, cpus) = Cmd(cmd; cpus = collect(Int, cpus))


Suggested change

setcpuaffinity(cmd::Cmd, ::Nothing) = Cmd(cmd; cpus = nothing)

setcpuaffinity(cmd::Cmd, cpus) = Cmd(cmd; cpus = collect(Int, cpus))

setcpuaffinity(cmd::Cmd, ::Nothing) = Cmd(cmd; cpus = nothing)

setcpuaffinity(cmd::Cmd, cpus::AbstractVector{Bool}) = Cmd(cmd; cpus = findall(cpus))

setcpuaffinity(cmd::Cmd, cpus::AbstractVector) = Cmd(cmd; cpus = convert(Vector{UInt16}, cpus))

setcpuaffinity(cmd::Cmd, cpus) = setcpuaffinity(cmd, collect(cpus)::AbstractVector)

I'm against dispatching on Bool element type to change the meaning (in general). It will change the behavior between cpus = Bool[true] and cpus = Integer[true] (or Any[true]). I prefer to add a distinct function like setcpuaffinitymask or maybe a Mask type for supporting this.

test/print_process_affinity.jl

test/show.jl

test/print_process_affinity.jl

Co-authored-by: Jameson Nash <[email protected]>

test/threads.jl

tkf · 2021-10-15T23:31:41Z

@vtjnash Can you check my comments above?

vtjnash

Do we know what happened on FreeBSD? The CI logs make it seem like it hung in the precompile tests, before getting to testing the new code.

test/print_process_affinity.jl

test/threads.jl

Co-authored-by: Jameson Nash <[email protected]>

tkf · 2021-11-10T06:37:38Z

I have no idea what's happening in FreeBSD but I think the CI was older than the recent fixes for FreeBSD. I merged the master branch to see if it shows something new.

tkf · 2021-11-10T23:35:48Z

In win64 we are getting

ERROR: LoadError: IOError: getaffinity: bad file descriptor (EBADF)
Stacktrace:
 [1] uv_error
   @ .\libuv.jl:97 [inlined]
 [2] uv_thread_getaffinity()
   @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\print_process_affinity.jl:18
 [3] print_process_affinity()
   @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\print_process_affinity.jl:25
 [4] top-level scope
   @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\print_process_affinity.jl:30
in expression starting at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\print_process_affinity.jl:29

(which is the same as #42469 (comment) but with a better error message).

I guess this requires libuv/libuv#3357 too.

tkf · 2021-12-16T02:32:26Z

Now that #43219 pulls in the fix libuv/libuv#3357, the test works in Windows CI.

@vtjnash Good to go?

base/cmd.jl

test/threads.jl

…ses (JuliaLang#42469) Support running commands with cpumask.

tkf added needs docs Documentation for this change is required needs news A NEWS entry is required for this change labels Oct 2, 2021

Support running commands with cpumask

d501456

tkf force-pushed the cpumask branch from 28b2df8 to d501456 Compare October 2, 2021 04:05

vtjnash reviewed Oct 2, 2021

View reviewed changes

base/cmd.jl Outdated Show resolved Hide resolved

vtjnash reviewed Oct 2, 2021

View reviewed changes

base/cmd.jl Outdated Show resolved Hide resolved

vtjnash reviewed Oct 2, 2021

View reviewed changes

test/print_process_affinity.jl Outdated Show resolved Hide resolved

vtjnash reviewed Oct 2, 2021

View reviewed changes

test/threads.jl Show resolved Hide resolved

tkf and others added 3 commits October 2, 2021 13:57

Apply suggestions from code review

2a72e24

Co-authored-by: Jameson Nash <[email protected]>

Rename: setcpus -> setcpuaffinity

17eadca

Include it in docs and news

9af2985

tkf changed the title ~~RFC: add setcpus(cmd, cpus) for setting CPU affinity of subprocesses~~ RFC: add setcpuaffinity(cmd, cpus) for setting CPU affinity of subprocesses Oct 2, 2021

tkf added Buildkite - retry failed jobs and removed needs docs Documentation for this change is required needs news A NEWS entry is required for this change Buildkite - retry failed jobs labels Oct 2, 2021

Include examples in the docstring

a59228e

KristofferC reviewed Oct 2, 2021

View reviewed changes

tkf added 2 commits October 2, 2021 18:17

Delay converting to cpumask; define show

d65cacd

Fill between the lines in the docstring

ba5559f

tkf commented Oct 2, 2021

View reviewed changes

tkf removed the Buildkite - retry failed jobs label Oct 3, 2021

tkf mentioned this pull request Oct 3, 2021

Use uv_thread_getaffinity when --threads=auto #42340

Merged

2 tasks

Merge branch 'master' into cpumask

4d37558

tkf changed the title ~~RFC: add setcpuaffinity(cmd, cpus) for setting CPU affinity of subprocesses~~ Add setcpuaffinity(cmd, cpus) for setting CPU affinity of subprocesses Oct 14, 2021

vtjnash reviewed Oct 14, 2021

View reviewed changes

tkf and others added 2 commits October 14, 2021 12:55

Apply suggestions from code review

7edc261

Co-authored-by: Jameson Nash <[email protected]>

Fix cpus type

038c116

tkf added 4 commits October 14, 2021 19:17

Add compat annotation

7739a25

Fix show

994e081

Fix a typo

20618b9

Don't run test with uv_thread_getaffinity on Windows

fa1b115

tkf commented Oct 15, 2021

View reviewed changes

test/threads.jl Outdated Show resolved Hide resolved

tkf commented Oct 15, 2021

View reviewed changes

test/threads.jl Outdated Show resolved Hide resolved

tkf added 2 commits October 16, 2021 18:31

Mention that it's Linux-only for now

e446956

Merge branch 'master' into cpumask

b9feb8d

vtjnash reviewed Nov 9, 2021

View reviewed changes

test/print_process_affinity.jl Outdated Show resolved Hide resolved

test/print_process_affinity.jl Outdated Show resolved Hide resolved

test/threads.jl Outdated Show resolved Hide resolved

tkf and others added 3 commits November 9, 2021 22:23

Through a proper error in print_process_affinity.jl

304330e

Co-authored-by: Jameson Nash <[email protected]>

Fix print_process_affinity.jl for Windows

a83b105

Co-authored-by: Jameson Nash <[email protected]>

Merge branch 'master' into cpumask

1beffff

Merge branch 'master' into cpumask

21ff94d

tkf added 2 commits December 15, 2021 17:11

Merge branch 'master' into cpumask

123a072

Merge branch 'master' into cpumask

70138f4

vchuravy requested a review from vtjnash January 6, 2022 17:57

vchuravy reviewed Jan 6, 2022

View reviewed changes

base/cmd.jl Outdated Show resolved Hide resolved

test/threads.jl Show resolved Hide resolved

vchuravy added this to the 1.8 milestone Jan 6, 2022

Clarify supported platform

24763b0

vtjnash merged commit 7b62896 into JuliaLang:master Jan 10, 2022

tkf deleted the cpumask branch January 17, 2022 04:22

LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022

Add setcpuaffinity(cmd, cpus) for setting CPU affinity of subproces…

9f19050

…ses (JuliaLang#42469) Support running commands with cpumask.

LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022

Add setcpuaffinity(cmd, cpus) for setting CPU affinity of subproces…

66494af

…ses (JuliaLang#42469) Support running commands with cpumask.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `setcpuaffinity(cmd, cpus)` for setting CPU affinity of subprocesses #42469

Add `setcpuaffinity(cmd, cpus)` for setting CPU affinity of subprocesses #42469

tkf commented Oct 2, 2021 •

edited

Loading

KristofferC Oct 2, 2021 •

edited

Loading

tkf Oct 2, 2021

tkf Oct 2, 2021

tkf commented Oct 14, 2021

vtjnash left a comment

vtjnash Oct 14, 2021

tkf Oct 14, 2021

tkf commented Oct 15, 2021

vtjnash left a comment

tkf commented Nov 10, 2021

tkf commented Nov 10, 2021

tkf commented Dec 16, 2021

		setcpuaffinity(cmd::Cmd, ::Nothing) = Cmd(cmd; cpus = nothing)
		setcpuaffinity(cmd::Cmd, cpus) = Cmd(cmd; cpus = collect(Int, cpus))

Add setcpuaffinity(cmd, cpus) for setting CPU affinity of subprocesses #42469

Add setcpuaffinity(cmd, cpus) for setting CPU affinity of subprocesses #42469

Conversation

tkf commented Oct 2, 2021 • edited Loading

KristofferC Oct 2, 2021 • edited Loading

Choose a reason for hiding this comment

tkf Oct 2, 2021

Choose a reason for hiding this comment

tkf Oct 2, 2021

Choose a reason for hiding this comment

tkf commented Oct 14, 2021

vtjnash left a comment

Choose a reason for hiding this comment

vtjnash Oct 14, 2021

Choose a reason for hiding this comment

tkf Oct 14, 2021

Choose a reason for hiding this comment

tkf commented Oct 15, 2021

vtjnash left a comment

Choose a reason for hiding this comment

tkf commented Nov 10, 2021

tkf commented Nov 10, 2021

tkf commented Dec 16, 2021

Add `setcpuaffinity(cmd, cpus)` for setting CPU affinity of subprocesses #42469

Add `setcpuaffinity(cmd, cpus)` for setting CPU affinity of subprocesses #42469

tkf commented Oct 2, 2021 •

edited

Loading

KristofferC Oct 2, 2021 •

edited

Loading