From: "Zebediah Figura (she/her)" Subject: Re: [PATCH 4/4] dlls/ntdll: Call try_recv inside sock_recv only if it is safe to do so. Message-Id: <6cc2faff-cb91-7422-987e-6ca32cfba378@codeweavers.com> Date: Tue, 18 Jan 2022 17:48:51 -0600 In-Reply-To: <2745e856-7ed9-e815-099f-9d2a75c399d2@codeweavers.com> References: <2745e856-7ed9-e815-099f-9d2a75c399d2@codeweavers.com> On 1/18/22 17:33, Paul Gofman wrote: > 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 >>> --- >>>   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? No, I think it's likely to 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. > What concerns me is the prospect that once we do one server call we've already lost. It leads me to wonder if three server calls is perceptibly worse than two—and if it is, perhaps we should find a way to eliminate those two server calls instead. It's quite likely that I'm biased from working with frame-time performance, though. It is probably very hard to not make any server calls at all when doing socket I/O. So depending on how ugly it gets to cut out the third server call it's probably not worth arguing against it.