search

letmaik / rawpy

📷 RAW image processing for Python, a wrapper for libraw
https://pypi.python.org/pypi/rawpy
MIT License
587 stars 67 forks source link

Remove gil from C implementations to enable reading raw files in parallel threads #189

Closed kmaddock closed 3 months ago

kmaddock commented 1 year ago

Removing the gil allows loading raw files from multiple python threads in parallel to improve performance

letmaik commented 1 year ago

Cython in CI is failing, did you test this locally? Did you use a different cython version perhaps?

kmaddock commented 1 year ago

I have been running it on Windows, I didn't do anything special with the Cython version!

It looks like I just need to move path.encode("UTF-8") out of the nogil block

letmaik commented 1 year ago

This also needs a test that tries to open/process multiple files in parallel.

letmaik commented 1 year ago

@kmaddock Are you still interested in working on this PR?

kmaddock commented 1 year ago

Hi, I'm still interested in working on this PR, but I haven't had a chance to get back to it!

letmaik commented 11 months ago

@kmaddock There are still some things that need to be adapted for this to work, see the CI logs. Also, I noticed that the Windows and macOS builds use the thread-safe variant of libraw whereas on Linux the non-thread-safe variant is picked up via pkg-config, so this has to be changed as well.

jontyrudman commented 8 months ago

@kmaddock thanks for your hard work, this is really useful for making my image viewer more usable. Would you mind giving me access to your branch for this PR so I can add my changes please? I've finished it off at https://github.com/jontyrudman/rawpy/tree/nogil .

@letmaik my changes involve removing with nogil for open_file and altering setup.py to find libraw_r so it should use the threadsafe version on all platforms.

I ran your CI workflow on my fork and the only issue left is minor, I get a ModuleNotFoundError for distutils on the Python 3.12 macOS and Windows tests only.

EDIT: I've also started using it in my image viewer and nogil helps me a whole lot for my use case (still rendering Qt widgets while running postprocess() in a QThread without it freezing).

jontyrudman commented 8 months ago

I'll still need to write the test for it, of course.

letmaik commented 8 months ago

@jontyrudman Feel free to open a new PR for this. We can give credit to @kmaddock in the PR description / commit message.

letmaik commented 7 months ago

@jontyrudman How are you getting on? It's quite a useful feature!

jontyrudman commented 7 months ago

@letmaik I've been quite busy this week because I'm starting a new job next week, but I've got plans to work on this over the weekend.