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

silx.opencl: Avoid executing OpenCL kernels at import time #4088

Closed
t20100 opened this issue Mar 21, 2024 · 4 comments · Fixed by #4093
Closed

silx.opencl: Avoid executing OpenCL kernels at import time #4088

t20100 opened this issue Mar 21, 2024 · 4 comments · Fixed by #4093
Assignees
Milestone

Comments

@t20100
Copy link
Member

t20100 commented Mar 21, 2024

First raised in #4077 (comment) :

then there is most likely a design issue with the silx.opencl.common.py module and more specifically about the OpenCL class.
During import plenty of code will be executed. And this is what create a seg fault during import. Even if no-opencl is used when importing silx.opencl or other module linked.

Also, for advanced usage, 1it would be nice to be able to set the OpenCL platforms+devices to use to spare the probing of all platforms

@pierrepaleo
Copy link
Contributor

+1

I really don't like this probing at import which silently creates contexts all over the place. I've had numerous issues in the past with it. The only advantage is that the "best" device is automatically picked (though "best" can be incorrect in certain cases).

Perhaps this probing could be made optional with an environment variable, eg. SILX_OPENCL="auto".

@t20100
Copy link
Member Author

t20100 commented Mar 21, 2024

I would be in favor of a Python API rather than an env. var. to control this.

I'd keep the automatic selection as the default behavior (since it is a good default and it would otherwise break the compatibility).

@kif
Copy link
Member

kif commented Mar 22, 2024

I am on it ... several path have been envisaged to mitigate this effect:

  • do not instanciate the OpenCL in the module (lasy loading)
  • make the OpenCL class a borg (easy)
  • lasy test the devices only when any of the atomic feature is actually needed (not difficult).

Only the first one is really tricky...

@t20100
Copy link
Member Author

t20100 commented Mar 22, 2024

BTW, there is silx.config which is used to custom some behaviors (mostly for plots), but it may be used here. Best to stick to built-in types there though.

kif added a commit to kif/silx that referenced this issue Mar 22, 2024
- make the OpenCL class a borg (i.e. instatiating it again is for free)
- make atomic32/64 cached properties, evaluated at runtime rather than
at import time
- make the ocl object creation at runtime, not at import-time

Overall the import time decreased from 2000ms to 200ms.

close silx-kit#4088

PS: I hate the black formating.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants