-
Notifications
You must be signed in to change notification settings - Fork 188
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
Add PyFuncWrap #483
Add PyFuncWrap #483
Conversation
Couldn't we just cache these for all Python functions? i.e. have a const pycall_args = Vector{PyObject}[] # argtuples[i] is a tuple of length i-1
const pycall_ret = PyNULL() where |
Yeah in an earlier iteration I had a (equiv of) |
Not threadsafe (in my possibly flawed mental model) because e.g. say thread 1 is setting some args, then say thread 2 gets scheduled and sets some args, then thread 1 is scheduled again - thread 1 could make the call with thread 2's args. |
This method was also faster, I think, can't remember by how much. Let me know if you want me to check. |
I don't think the CPython API is thread-safe anyway, but if we were worried about that we could always use a per-thread array similar to JuliaLang/julia#26562. But the lack of thread-safety of the CPython API makes me disinclined to bother. |
In particular, if we want to make |
Yeah, all good points. I will benchmark the difference between this and a |
You can check out a version of The benchmark results (from that branch - shown below) suggest that the wrap in this PR is faster. Having said that, I'm not really sure why the A cleaned up version of
|
I'd rather have the time spent on making |
So, you've managed to sucker me in to cleaning up my |
Closing in favour of #492 |
Some performance issues were reported here: JuliaML/OpenAIGym.jl#9
This adds a wrapper for a python function (similar to the FuncWrapper in callback.jl for julia functions) so that when calling it in tight loops you reduce the number of PyObjects created to store the arguments and the return value. The included benchmarks show significant increase in calling speed.