[libc-commits] [libc] [libc] Scan the ports more fairly in the RPC server (PR #66680)

Johannes Doerfert via libc-commits libc-commits at lists.llvm.org
Sun Sep 24 11:04:07 PDT 2023


================
@@ -560,9 +562,9 @@ template <uint16_t opcode> LIBC_INLINE Client::Port Client::open() {
 template <uint32_t lane_size>
 [[clang::convergent]] LIBC_INLINE
     cpp::optional<typename Server<lane_size>::Port>
-    Server<lane_size>::try_open() {
+    Server<lane_size>::try_open(uint32_t start) {
   // Perform a naive linear scan for a port that has a pending request.
-  for (uint32_t index = 0; index < process.port_count; ++index) {
+  for (uint32_t index = start; index < process.port_count; ++index) {
----------------
jdoerfert wrote:

Ok. I think I see the disconnect. Your scheme doesn't actually provide the guarantee I thought I was, which is why we talk past each other. In your patch, one call to handle_server will always start to process the ports from 0 till the end. With this patch it will ensure termination as it checks each port once. That is clearly better than the code before, not necessarily because it could get stuck, but because it could starve ports. I don't think stuck performing work is too bad for the host thread. For now I don't see what the benefit would be to return to the caller of handle_server, just to let them call it again.

That all said, what I thought you were doing is to remember the index of the last port that was handled across calls. Such that you'd scan from there the next time. Now, you still always prefer port 0 over port 5 even if you just handled port 0 last time. That said, this patch at least ensures both are handled during a handle_server call. If index is persistent though, which is what I thought, port 5 would be prioritized over port 0 if port 0 was handled last time. In that scheme, you need to iterate from 0 to index at the end, since index did not start at 0. Hence my request for the extra iterations.

I still think the persistent index has benefits when it comes to fairness, but I agree this patch in itself does what you describe in a sensible way.

https://github.com/llvm/llvm-project/pull/66680


More information about the libc-commits mailing list