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

Tasks spawned with tspawnat can migrate on 1.7 #32

Closed
Octogonapus opened this issue Jan 26, 2022 · 3 comments · Fixed by #33
Closed

Tasks spawned with tspawnat can migrate on 1.7 #32

Octogonapus opened this issue Jan 26, 2022 · 3 comments · Fixed by #33

Comments

@Octogonapus
Copy link

Using ThreadPools v2.1.0 and Julia v1.7.1, tasks spawned with @tspawnat can migrate between threads. Tasks spawned this way always spawn on the correct thread as given to the macro, but can migrate to another thread during execution.

There is some existing work to add support for sticky tasks in 1.7+ in this PR JuliaLang/julia#42302 but it will not land for some time.

Are you interested in supporting sticky tasks in 1.7+ in this package? From my testing, it would be a simple change. Just allow the user to set sticky to true here: https://github.com/tro3/ThreadPools.jl/blob/master/src/macros.jl#L270

cc @IanButterworth

@IanButterworth
Copy link
Collaborator

IanButterworth commented Jan 26, 2022

Making sticky=true optional does seem like the right approach. There are strong opinions about pinning tasks to threads being bad practice, but given what that macro promises to do, it should at least be properly pinnable.

So @tspawnat true .... might be the right way to go, with the default assuming false, so allowing migration

@carstenbauer
Copy link

There are strong opinions about pinning tasks to threads being bad practice

IMO, these opinions are particularly strong in the Julia community and, to some extent, understandable. But not every code is library code which needs to compose well with other library code / user code. It might just be a very particular application.

I'm working in HPC and often you just need to know that a certain piece of code will run on a certain hardware thread (which you might have pinned to a specific core). And the only built-in way (that I'm aware of) is @threads which currently [has sticky=true].(https://github.com/JuliaLang/julia/blob/05e0cb94e82d5fc11206bb22a404d5381001bedc/base/threadingconstructs.jl#L31).

[...] but given what that macro promises to do, it should at least be properly pinnable.

I'd say sticky = true should be the default. What do you think would be the purpose / use case of @tspawnat to begin with if the task can migrate away?

@IanButterworth
Copy link
Collaborator

That's a fair point. Will sticky=true break 1.6 behavior? Or perhaps it should always have been true..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants