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

GIL functions for genuine multi-threading #535

Merged
merged 41 commits into from
Aug 7, 2024
Merged

GIL functions for genuine multi-threading #535

merged 41 commits into from
Aug 7, 2024

Conversation

cjdoris
Copy link
Collaborator

@cjdoris cjdoris commented Aug 2, 2024

Adds PythonCall.GIL module with:

  • lock(f) to execute f() with the GIL held
  • @lock expr the macro equivalent
  • release(f) to execute f() with the GIL released
  • @release expr the macro equivalent

Here's an example from the tests. It calls Python's time.sleep(1) twice in two Julia threads. GIL.@release is required to release the GIL so that within the threads GIL.@lock can re-acquire it. Since time.sleep itself then releases the GIL again, the sleep actually happens in parallel and the whole block takes 1 second.

PythonCall.GIL.@release Threads.@threads for i = 1:2
    PythonCall.GIL.@lock pyimport("time").sleep(1)
end

Here's a similar example from the Python tests. It defines a Julia function jsleep which behaves pretty much like time.sleep. In particular it also releases the GIL, so when you call jsleep(1) twice in parallel, it only takes 1 second in total.

from concurrent.futures import ThreadPoolExecutor, wait
from juliacall import Main as jl
jsleep = jl.seval("t::Real -> PythonCall.GIL.@release Libc.systemsleep(t)")
pool = ThreadPoolExecutor(2)
fs = [pool.submit(jsleep, 1) for _ in range(2)]
wait(fs)

When using this from the Python side, if the Julia code can yield (e.g. sleep(1)), the threaded tasks won't finish unless you explicitly yield back from Python to let the yielding code finish. You'll probably need to yield in a loop. We could provide some functionality for this in JuliaCall but I'll leave that for another ticket.

  • release notes
  • julia tests
  • python tests
  • documentation - I think a lot of the advice in the FAQ could move into a new GIL/multithreading section in the main docs.
  • builds on More thread-safe GC #529 - merge that first

@cjdoris cjdoris changed the base branch from safer-gc to main August 2, 2024 20:42
@cjdoris
Copy link
Collaborator Author

cjdoris commented Aug 2, 2024

@MilesCranmer @ericphanson thought you'd like this PR - I think it's cool

@MilesCranmer
Copy link
Contributor

This is awesome!!

@MilesCranmer
Copy link
Contributor

MilesCranmer commented Aug 2, 2024

Yeah I think this would be very very useful for the PySR/SymbolicRegression.jl ecosystem. The most requested feature is being able to use Python functionality from within user-defined custom loss functions (which ought to be run in parallel to get the best speeds). I think this feature would help enable that.

Also this would make MilesCranmer/PySR#589 much less hackier

@cjdoris cjdoris mentioned this pull request Aug 2, 2024
@cjdoris cjdoris added the documentation Improvements or additions to documentation label Aug 3, 2024
@cjdoris
Copy link
Collaborator Author

cjdoris commented Aug 3, 2024

Ok I'm happy with this except the names lock and release which feel asymmetric. My suggestions in order of preference:

  • acquire and release (Python uses this terminology in the C API docs, though it also uses ensure/release and restore/save)
  • lock and unlock (more like the Julia stdlib)
  • pygil_acquire and pygil_release and export these.

@MilesCranmer
Copy link
Contributor

MilesCranmer commented Aug 3, 2024

I strongly prefer lock and unlock. Here's my reasoning:

Even though I use Python for 50% of my work, I have literally never seen acquire and release used before. I'm assuming this is because the Python multithreading/multiprocessing API 99% of the time just assumes immutability of shared storage (or copies mutable versions) and interacts with a mapped function on a thread pool. In other words, thread locks are fairly uncommon in Python (maybe because you can't really control the GIL from within python), and I think most people interacting with this type of pattern will be more used to the Julia versions.

In Julia, lock and unlock are fairly common functions. So much so that they are found in (and exported by!) Base, which is unlike Python where acquire is only found in a separate stdlib modules (like here). The lock() do ... syntax is very Julian, it would be weird to see mixed acquire() do ... syntax – with the acquire inspired by Python and the function-passing inspired by Julia.

Though these functions control the Python API, the exposed API is called via PythonCall.jl. Thus, I think they should respect the Julia standards. Perhaps a juliacall version could use acquire and release. Basically whenever there is an ambiguity in a syntax choice, I think PythonCall.jl should prefer normal Julia syntax, and juliacall should prefer normal Python syntax.

@MilesCranmer
Copy link
Contributor

MilesCranmer commented Aug 3, 2024

One other thought: would it ever be possible to control multiple Python processes from PythonCall.jl? In this case I wonder if there should be some kind of lock(f::Function, lock::PythonProcessGIL) implemented, so you can choose a specific Python process to lock the GIL of. In this case, you could simply extend Base.lock to PythonProcessLock, rather than having a separate lock function.

Maybe this is completely out of scope though, and the right way is to have multiple Julia processes, each with their own Python processes.

Edit: Yeah, thinking about this more, it's much more sensible to have a separate Python process for each Julia process, rather than needing all that increased complexity.

@MilesCranmer
Copy link
Contributor

MilesCranmer commented Aug 3, 2024

Still though, it might be nice to have a Julian version, so you could really use the standard patterns (including other tools that just overloads Base.lock, like:

using PythonCall: PythonGIL

lock(PythonGIL) do
    #= do stuff =#
end

Rather than needing a new PythonCall.GIL.lock function.

Then you get stuff like Base.@lock for free.

(Probably should also add a Base.trylock and Base.islocked)

@MilesCranmer
Copy link
Contributor

MilesCranmer commented Aug 3, 2024

Argh, but then again, I guess the GIL is kind of different than a normal Julia lock... So that might motivate having different names for these.

I really don't know.

@cjdoris
Copy link
Collaborator Author

cjdoris commented Aug 3, 2024

Yeah I think I agree that lock, unlock is right. This is a Julia package so should use Julia terminology - which has been my approach elsewhere in PythonCall and CondaPkg.

It's a nice idea to try overloading Base functions for locking, but it's not immediately obvious how, so I'll leave that for a separate PR.

@MilesCranmer
Copy link
Contributor

MilesCranmer commented Aug 3, 2024

It's a nice idea to try overloading Base functions for locking, but it's not immediately obvious how, so I'll leave that for a separate PR.

The tricky thing here is that it would require a slightly different syntax. The lock in Base locks a specific object, whereas the current one in this PR takes no arguments. This is kind of why I feel like lock(PythonCall.GILRef) might be nice. And GILRef can just be an abstract type to allow declaring a Base.lock(f::Function, ::Type{PythonCall.GILRef}) = PythonCall.GIL.lock(f)

(Not sure the right name here... GILRef doesn't sound great)

@cjdoris cjdoris merged commit 4a1ee78 into main Aug 7, 2024
14 checks passed
@cjdoris cjdoris deleted the gil branch August 7, 2024 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants