On 1/19/22 08: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 jinoh.kang.kr@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.
This isn't a regression. My initial impression was that it was, but closer inspection revealed that the winsock->Afd transition only preserved the bug already present in the old code, not introduced a new one.
Now that the bug is in fact not a regression, I suppose I have much weaker argument to get this committed before release. I'm okay either way.
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.
Yes, this was my intention.