Add experimental option to run unary-stream RPCs on a single Python thread. #20753
Conversation
gnossen
added
kind/enhancement
lang/Python
release notes: yes
There was a problem hiding this comment.
The change looks good! One major thing I think can improve is about the regression of initial_metadata.
The idea is that if the info that the application is asking is not ready yet, then donate the calling
You can change the condition.wait into trying to get an event, and handle it. If response event propagated earlier, the RPCState will cache it. In that way, the _SingleThreadedRendezvous can be truly singled threaded.
| def _done(): | ||
| return self._state.initial_metadata is not None | ||
|
|
||
| _common.wait(self._state.condition.wait, _done) |
There was a problem hiding this comment.
We might want to change the condition.wait to do other things that either move the RPC forward or notify application that the info is not ready.
Applies to all condition.wait in _SingleThreadedRendezvous.
There was a problem hiding this comment.
This changes some of the current assumptions in the code. At the very least, we'd have to buffer message events dequeued by the attribute methods here so that they can later be produced by the iterator. I don't view this as a blocker for this PR, considering the use cases of our target user. Adding as a TODO.
There was a problem hiding this comment.
Let's link the issue here
| def _done(): | ||
| return self._state.initial_metadata is not None | ||
|
|
||
| _common.wait(self._state.condition.wait, _done) |
There was a problem hiding this comment.
Let's link the issue here
|
The TODO was added just a few lines down. |
Benchmarking revealed that context switching and lock contention led to a considerable amount of overhead in executing RPCs taking the "integrated" code path, which used multiple threads.
Overall, this PR has been shown to consistently decrease end-to-end latency of unary-stream RPCs by around 6.5% on Intel x86-64 workstations. Additional testing involving pinned cores and disabling Intel Turbo revealed that, under non-ideal thermal conditions, this win may degrade to around 2.5%.
An experimental channel option consumed only by the Python layer and not passed to core has been added to activate the single-threaded code path, since it cannot fulfill the
grpc.Futureinterface.This change was tested both in OSS using bazel and in the Google codebase with Blaze, using various sanitizers, including ASAN and MSAN. I was originally concerned that the change to
channel.pyx.pxiwould result in a memory leak, but this was not born out by testing.At the moment, testing this code path is manual, since activating it across the entire test suite would require manual instrumentation and parametrization of every existing unit test. I have filed an issue to track this work.
This PR resolves #20741.