Skip to content

OpenCL: shared context/queue/device/profiling - #4757

Open
kif wants to merge 3 commits into
silx-kit:mainfrom
kif:4755_shared_queue
Open

kif wants to merge 3 commits into
silx-kit:mainfrom
kif:4755_shared_queue

Conversation

@kif

@kif kif commented Oct 23, 2026

Copy link
Copy Markdown
Member

PR summary

The idea behind this PR is to share a single instance of OpenclQueue which carries the context, the queue, the device and the profiling configuration state.

Code in the different stages of silx has been addressed and remains backwards compatible.
Close #4755

AI Disclosure

  • No AI used
  • AI tool(s) Claude/Opus used for implementing the test and performing the refactoring of different classes. Code of the tomo part should be double checked by @pierrepaleo

@kif
kif requested review from pierrepaleo and t20100 October 23, 2026 13:50

@t20100 t20100 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick review:

This sounds a lot of logic to support hot change of the profiling.

Also it looks one has to know the internal logic to know which parameters will be used or not. It would be good to either simplify this or warn when an argument is set and not used.

I leave the second look to @pierrepaleo

normalize=normalize,
)
self.ctx = ctx
self.queue = queue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raise ValueError or at least log a warning if both ctx and queue are provided

deviceid=None,
memory=None,
profile=False,
queue=None,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why passing queue here?

np.int32(src_offset),
np.int32(transfer_shape[::-1]),
)
ev.wait()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

behavior change: this is not longer guaranteed to be done.
maybe it's the same at other places, to be checked.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow to share queue between OpenclProcessing

2 participants