Skip to content

[Python] Remove GIL from ReceiveMessageOperation.un_c method#41812

Closed
googlyrahman wants to merge 1 commit into
grpc:masterfrom
googlyrahman:samplefix
Closed

[Python] Remove GIL from ReceiveMessageOperation.un_c method#41812
googlyrahman wants to merge 1 commit into
grpc:masterfrom
googlyrahman:samplefix

Conversation

@googlyrahman

Copy link
Copy Markdown
Contributor

Remove one of the GIL contention from the grpc python wrapper.

The python releases the GIL while doing b"".join. This was one of the major GIL contention in the python wrapper.

Fixing this significantly reduces the GIL contention.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Mar 6, 2026

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: googlyrahman / name: Margubur Rahman (df4796a)

@sergiitk sergiitk changed the title Remove GIL from ReceiveMessageOperation.un_c method [Python] Remove GIL from ReceiveMessageOperation.un_c method Mar 10, 2026
@sergiitk

Copy link
Copy Markdown
Member

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request aims to reduce GIL contention when receiving messages by changing how byte buffers are constructed. The approach of collecting byte chunks in a list and then using b"".join() is a good improvement over repeated concatenation to a bytearray.

I've provided one suggestion for a more significant optimization. By leveraging grpc_byte_buffer_reader_readall from the C-core library, you can avoid the overhead of the Python-level loop and object creation entirely, leading to better performance. This requires a small change to expose the function to Cython, which I've detailed in my comment.

Comment thread src/python/grpcio/grpc/_cython/_cygrpc/operation.pyx.pxi
@asheshvidyut asheshvidyut force-pushed the samplefix branch 2 times, most recently from 6ad31ae to 45ecf70 Compare March 17, 2026 10:16

@asheshvidyut asheshvidyut 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.

LGTM.

@sergiitk sergiitk 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.

LGTM

asheshvidyut pushed a commit to asheshvidyut/grpc that referenced this pull request Mar 26, 2026
)

Remove one of the GIL contention from the grpc python wrapper.

The python [releases the GIL](python/cpython#17757) while doing b"".join. This was one of the major GIL contention in the python wrapper.

Fixing this significantly reduces the GIL contention.

Closes grpc#41812

COPYBARA_INTEGRATE_REVIEW=grpc#41812 from googlyrahman:samplefix df4796a
PiperOrigin-RevId: 888241985
asheshvidyut pushed a commit to asheshvidyut/grpc that referenced this pull request Apr 8, 2026
)

Remove one of the GIL contention from the grpc python wrapper.

The python [releases the GIL](python/cpython#17757) while doing b"".join. This was one of the major GIL contention in the python wrapper.

Fixing this significantly reduces the GIL contention.

Closes grpc#41812

COPYBARA_INTEGRATE_REVIEW=grpc#41812 from googlyrahman:samplefix df4796a
PiperOrigin-RevId: 888241985
asheshvidyut pushed a commit to a-detiste/grpc that referenced this pull request Jun 10, 2026
)

Remove one of the GIL contention from the grpc python wrapper.

The python [releases the GIL](python/cpython#17757) while doing b"".join. This was one of the major GIL contention in the python wrapper.

Fixing this significantly reduces the GIL contention.

Closes grpc#41812

COPYBARA_INTEGRATE_REVIEW=grpc#41812 from googlyrahman:samplefix df4796a
PiperOrigin-RevId: 888241985
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang/Python release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants