Skip to content

Release GIL during rank filters - #10041

Open
danking wants to merge 3 commits into
python-pillow:mainfrom
danking:release-gil-rank-filter
Open

danking wants to merge 3 commits into
python-pillow:mainfrom
danking:release-gil-rank-filter

Conversation

@danking

@danking danking commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

This doesn’t completely fix but is related to:
#6410

I noticed that MedianFilter doesn’t release the GIL which, for those of us not yet on free threaded Python, prevents us from fully utilizing all the cores.

I did use an LLM to do some research on pillow and to author these three lines but I would have written the exact same three. I simply don’t have a full sized keyboard at hand right now.

Thank you kindly for your time reviewing.

@danking
danking marked this pull request as ready for review September 22, 2026 14:35

@akx akx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM (from the sidelines, as it were). (I should maybe take a look at other places where we could release the GIL more than we do now...)

#9744 already sped RankFilter up a little (unreleased as of yet).

My little eye also spies an opportunity to hoist those ysize and xsize (see #9788 etc.) for an additional speed boost, but that sounds out of scope for this PR.

@danking

danking commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Hmm... That feels a bit like I might have uncovered some concurrency issue. Can't look carefully now but I can take a look in ~8 hours.

XFAIL Tests/test_imagedraw.py::test_line_h_s1_w2 - failing test
XFAIL Tests/test_image_resample.py::TestCoreResampleAlphaCorrect::test_levels_rgba - Current implementation isn't precise enough
XFAIL Tests/test_image_resample.py::TestCoreResampleAlphaCorrect::test_levels_la - Current implementation isn't precise enough
FAILED Tests/test_file_msp.py::test_open_windows_v2 - OSError: Corrupted MSP file in row 233

@akx

akx commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Out of curiosity, @danking: what size are the MedianFilters you generally run (in, I assume, production code)?

@danking

danking commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

You are too generous! This is just for a silly project of mine. The files are all very tiny, on the order of tens to hundreds of KB. The filters are 3x3 or 5x5. You shouldn't take anything I'm doing here as grounded in any serious production work.

@akx

akx commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Silly projects are the best projects! But okay, I was just thinking if it'd maybe be worth it to special-case the 3x3 median case. I'd imagine it's a fairly common one.

@danking

danking commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Haha, agreed. I think you can accelerate that case. I have some ideas. I'll toss a PR up if one of them turns out to be good.

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.

2 participants