Redundant condition inside `take_gil` function

Core Development
1

Hi, I want to ask it here since I am not sure I don’t miss anything before opening a PR. There is this piece of code inside take_gil function inside Python/ceval_gil.c:

What is the point of following condition?

if (!_Py_atomic_load_int_relaxed(&gil->locked)) {
    goto _ready;
}

Isn’t it redundant? I know in multithreaded environment there could be tricky things but still I can’t find reason why this condition has to be there in this scenario. I tried removing it and all tests pass. Also by doing some benchmarks I could see performance increase (tiny) under certain circumstances.

1 Like
2

Diving into the history of the code, I believe that came from the original “new” GIL work who’s logic looked a little bit different while attempting to make something work right on Windows.

There was a step between that condition and the while loop that it was skipping. That platform specific COND_RESET aka COND_PREPARE macro went away not long after in Issue #8411: new condition variable emulation under Windows for the n… · python/cpython@e1dd174 · GitHub by further refactoring.

I believe you are right, it could be considered redundant as the same code path should be taken based on the while condition failing as that if+goto.

1 Like
3

Thanks, I created issue+PR: gh-112978: Remove redundant condition in by WolframAlph · Pull Request #112979 · python/cpython · GitHub

1 Like