[PATCH 4/4] dlls/ntdll: Call try_recv inside sock_recv only if it is safe to do so.

[Paul Gofman]

On 1/19/22 02:08, Zebediah Figura (she/her) wrote:
> On 1/18/22 13:30, Jinoh Kang wrote:
>> Otherwise, try_recv() call from sock_recv() may race against try_recv()
>> call from async_recv_proc(), shuffling the packet order.
>>
>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52401
>> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
>> ---
>>   dlls/ntdll/unix/socket.c | 38 ++++++++++++++++++-------------------
>>   dlls/ws2_32/tests/sock.c |  8 ++++----
>>   server/protocol.def      |  4 ++--
>>   server/sock.c            | 41 ++++++++++++++++++++++++++++------------
>>   4 files changed, 54 insertions(+), 37 deletions(-)
>
> Despite the attempts to split 2/4 and 3/4 out of this patch, it's 
> still doing two things at once. Namely, the optimization whereby 
> recv_socket returns STATUS_ALERTED should not be part of this patch.
>
> As I discussed in [1] it's not really clear to me that three server 
> calls is significantly worse than two. It'd be nice to know that just 
> removing the initial try_recv() call is really going to make programs 
> worse if we're going to pursue it.

GeForce NOW comes to mind with network speed times slower in Wine vs 
Windows, although I didn't measure anything myself with it nor have any 
proof that the major part of slowdown server roundtrip.

I was under impression that the major socket performance compromise in 
network heavy applications happens due to server calls during socket 
I/O. Are there reasons to think it might not be the case? If that is 
considered to be the likely case, the question is: if there is a planned 
change which is going to increase amount of server calls per every sync 
socket operation from 2 to 3 should it be presumed ok and subject to be 
proven otherwise? Or should instead some metrics and reasoning 
suggesting that the added overhead is not sufficient should be provided 
instead before making such a change? I am not sure if the issue 
addressed by these patches is a regression, maybe if that is a 
regression it can give a motivation to fix that first and think about 
optimization next. Otherwise I was under impression that (as usual for 
Wine patches?) it is the subject for patch justification to make some 
argument that it doesn't regress performance considerably.