Conversation
adamreichold
force-pushed
the
drop-ensure-gil
branch
from
81b5224 to
c413bd4
Compare
kngwyu
left a comment
kngwyu
left a comment
There was a problem hiding this comment.
Cool. Looks good to me. At least, I don't see any benefit of having Option<GILPool> in the struct.
adamreichold
force-pushed
the
drop-ensure-gil
branch
2 times, most recently
from
b8ef9a3 to
3b3eead
Compare
|
@davidhewitt Since this is strictly refactoring only, I think this would be good to merge based on one review. But then again this code is somewhat involved/unsafe and you wrote most of the module in question. Would you like to give this a review yourself or do you think we can merge this without it? |
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Yep, I think this is good to merge. Nice tidy up! 👍
|
bors r=davidhewitt |
3166: RFC: Drop EnsureGIL to remove one layer of indirection from the implementation of Python::with_gil r=davidhewitt a=adamreichold I am not sure if other people would consider this a simplification as well, but it helped me to remove one layer of indirection used by the implementation of `Python::with_gil`, especially flattening the `Option<_>` layers in `EnsureGIL` and `GILGuard::pool`. This makes it obvious that we always create `GILPool` when we actually acquire the GIL. (Of course, there still might be extra `GILPool` instances created manually.) Co-authored-by: Adam Reichold <adam.reichold@t-online.de>
…tion of Python::with_gil
adamreichold
force-pushed
the
drop-ensure-gil
branch
from
3b3eead to
62f424b
Compare
|
bors retry |
|
Canceled. |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
3 participants
I am not sure if other people would consider this a simplification as well, but it helped me to remove one layer of indirection used by the implementation of
Python::with_gil, especially flattening theOption<_>layers inEnsureGILandGILGuard::pool. This makes it obvious that we always createGILPoolwhen we actually acquire the GIL. (Of course, there still might be extraGILPoolinstances created manually.)