-
Notifications
You must be signed in to change notification settings - Fork 214
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
PERF: drop GIL during long-running proj database calls #1354
Conversation
These calls can take hundreds of milliseconds to access the PROJ databases, during which time no other Python code in any other thread can run. Use `nogil` to avoid this effect. This should be safe for these calls as they all take a separate PROJ threading context.
Codecov Report
@@ Coverage Diff @@
## main #1354 +/- ##
=======================================
Coverage 96.41% 96.41%
=======================================
Files 20 20
Lines 1812 1812
=======================================
Hits 1747 1747
Misses 65 65 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know enough about the thread-safety of PROJ, but I like the idea of this optimization. @tpwrules do you have any profiling results (pretty plots?) that show the execution time improvements you've seen by doing this?
Of course, something this low-level should really get approval from @snowman2 before merging.
It doesn't improve execution time, just makes other threads in the process more stable. It's a really hard thing to benchmark and measure unfortunately. I just was able to observe the effect on my program, but I can't really share the program here. |
PROJ does not deal with thread safety internally. It should be managed by the user via the "context". The same context must not be used simultaneously by two different threads. |
context, | ||
c_auth_name, | ||
c_category, | ||
bool(allow_deprecated), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised cython didn't complain about using bool
with nogil
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming it's valid if it did not complain, but I'm not 100% sure.
Thanks @tpwrules 👍 |
I would be interested to see if doing this with |
(Tests fail with environment issues, but one can load pyproj outside of the test environment. Filed upstream.) Enhancements: * PERF: drop GIL during long-running proj database calls by @tpwrules in pyproj4/pyproj#1354 * PERF: thread local context by @snowman2 in pyproj4/pyproj#1419 * ENH: Add is_deprecated and get_non_deprecated() to CRS by @jjimenezshaw in pyproj4/pyproj#1383 * ENH: Add runtime & compiled PROJ versions by @snowman2 in pyproj4/pyproj#1427
These calls can take hundreds of milliseconds to access the PROJ databases, during which time no other Python code in any other thread can run. Use
nogil
to avoid this effect.This should be safe for these calls as they all take a separate PROJ threading context.
It's quite probable there are other calls which could use this, but these negatively affected my application and I was able to identify them. I would welcome additions to the set of calls to wrap.
history.rst
for all changes andapi/*.rst
for new API