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

Make threadpoolsize(), threadpooltids(), and ngcthreads() public #55701

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

JamesWrigley
Copy link
Contributor

threadpoolsize() and ngcthreads() were already in the docs anyway, and threadpooltids() is useful for checking which threads belong to a pool.

@JamesWrigley
Copy link
Contributor Author

(bump)

@oscardssmith oscardssmith added needs decision A decision on this change is needed triage This should be discussed on a triage call labels Sep 27, 2024
@gbaraldi
Copy link
Member

gbaraldi commented Dec 5, 2024

threadpoolsize anad ngcthreads are fine but threadpooltids is an internal that we don't want the user to see or care about ;)

@JamesWrigley
Copy link
Contributor Author

But I need to care about it 😅 For context, this is the usecase that motivated the PR: https://github.com/Gnimuc/CImGui.jl/blob/7c315112271b61d59c35873a2f1691021fb40198/ext/GlfwOpenGLBackend.jl#L177

That function allows pinning a task on a specific thread or in a given threadpool, which is sometimes necessary for libuv/GLFW reasons, but to do that I do need a valid thread ID. I could try recomputing the valid thread IDs but that's not completely trivial going from the definition so I feel that it would better if this was public.

@vchuravy
Copy link
Member

vchuravy commented Dec 6, 2024

Making something public is a commitment to an API, which we are not ready to make. It's often fine to access internals, but you need to be aware of the fact that you are doing so (and you don't get to complain when a future Julia version changes things on you).

So threadpooltids not being public is very much intended.

These are already mentioned in the docs.
@JamesWrigley
Copy link
Contributor Author

Ok, fair enough 👍 I rebased the branch and removed threadpooltids() from the commit.

@gbaraldi gbaraldi removed the triage This should be discussed on a triage call label Dec 19, 2024
@JamesWrigley
Copy link
Contributor Author

Bump, AFAICS the test failures are unrelated.

@inkydragon inkydragon added the multithreading Base.Threads and related functionality label Dec 31, 2024
@IanButterworth IanButterworth removed the needs decision A decision on this change is needed label Dec 31, 2024
Copy link
Member

@IanButterworth IanButterworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that they are already in the manual, so this is just a bugfix really.

@IanButterworth IanButterworth merged commit d604057 into JuliaLang:master Dec 31, 2024
5 of 10 checks passed
@JamesWrigley JamesWrigley deleted the threading-functions branch December 31, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants