Repository navigation
Allow threads - #15
Conversation
| fn put(&self, py: Python, location: PyPath, bytes: Vec<u8>) -> PyResult<()> { | ||
| py.allow_threads(|| { | ||
| self.rt | ||
| .block_on(self.inner.put(&location.into(), bytes.into())) | ||
| .map_err(ObjectStoreError::from)?; | ||
| Ok(()) | ||
| }) |
There was a problem hiding this comment.
I always thought that if you didn't include the py: Python argument, that the GIL would be released automatically. Is this not true? To release the GIL you have to include the py argument and then also call allow_threads?
There was a problem hiding this comment.
I thought the same thing but that's not what my test showed. If you look at the docs: https://pyo3.rs/v0.21.1/parallelism#parallelism
It goes from
fn search_sequential(contents: &str, needle: &str) -> usize {
contents.lines().map(|line| count_line(line, needle)).sum()
}To
fn search_sequential_allow_threads(py: Python<'_>, contents: &str, needle: &str) -> usize {
py.allow_threads(|| search_sequential(contents, needle))
}So I think we are wrong.
I can confirm with @davidhewitt tomorrow.
There was a problem hiding this comment.
Indeed the GIL is not released automatically, as many arguments like Bound<'py, PyAny> can also implicitly carry the GIL even if py: Python<'py> is not included.
|
@roeap quick ping on this |
|
@adriangb - sorry for being MIA for so long. Do you mind rebasing, and I am happy to review / merge then. |
|
@roeap will you push a 0.2.0 release after this one? |
This should release the GIL and allow use in multiple threads.
I tested with this script:
Locally there isn't much difference, I'm IO bound. But on GCP compute that goes from ~15s to ~3s for me.