[Bug 45524] New: Add a futex-based implementation of condition variables
https://bugs.winehq.org/show_bug.cgi?id=45524 Bug ID: 45524 Summary: Add a futex-based implementation of condition variables Product: Wine Version: 3.13 Hardware: x86 OS: Linux Status: NEW Severity: normal Priority: P2 Component: ntdll Assignee: wine-bugs(a)winehq.org Reporter: z.figura12(a)gmail.com Distribution: --- Created attachment 61910 --> https://bugs.winehq.org/attachment.cgi?id=61910 [PATCH] ntdll: Add a futex-based condition variable implementation. Titanfall 2 has reportedly high wineserver overhead, and this seems to help with that. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 --- Comment #1 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- Wouldn't it be more natural to move the futexes support to the underlying ntdll implementations of NtWaitForKeyedEvent, etc? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 --- Comment #2 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Dmitry Timoshkov from comment #1)
Wouldn't it be more natural to move the futexes support to the underlying ntdll implementations of NtWaitForKeyedEvent, etc?
I'm not sure that would work; I had assumed that keyed events didn't have to use pointers to valid memory as a key. Is this not the case? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 --- Comment #3 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Zebediah Figura from comment #2)
Wouldn't it be more natural to move the futexes support to the underlying ntdll implementations of NtWaitForKeyedEvent, etc?
I'm not sure that would work; I had assumed that keyed events didn't have to use pointers to valid memory as a key. Is this not the case?
I don't see how is this related, I'm just suggesting instead of introducing new layer of code with futexes and still having separate ntdll fallback implementation have everything in one place - ntdll. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 --- Comment #4 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Dmitry Timoshkov from comment #3)
(In reply to Zebediah Figura from comment #2)
Wouldn't it be more natural to move the futexes support to the underlying ntdll implementations of NtWaitForKeyedEvent, etc?
I'm not sure that would work; I had assumed that keyed events didn't have to use pointers to valid memory as a key. Is this not the case?
I don't see how is this related, I'm just suggesting instead of introducing new layer of code with futexes and still having separate ntdll fallback implementation have everything in one place - ntdll.
Sorry, should have read as: have everything in one place - ntdll conditional variables APIs. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 --- Comment #5 from Zebediah Figura <z.figura12(a)gmail.com> --- I'm sorry; I'm not sure I understand. If you mean implementing keyed events on top of futexes; I'm not sure that'll work since I don't think the key has to be a valid pointer (whereas for futexes it absolutely must be). If your concern is with the architecture of the patch, I just followed the model used in critical sections. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 --- Comment #6 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Zebediah Figura from comment #5)
I'm sorry; I'm not sure I understand. If you mean implementing keyed events on top of futexes; I'm not sure that'll work since I don't think the key has to be a valid pointer (whereas for futexes it absolutely must be). If your concern is with the architecture of the patch, I just followed the model used in critical sections.
I don't understand your concern. For instance look at your new implementation void WINAPI RtlWakeConditionVariable( RTL_CONDITION_VARIABLE *variable ) { if (interlocked_dec_if_nonzero( (int *)&variable->Ptr )) - NtReleaseKeyedEvent( keyed_event, &variable->Ptr, FALSE, NULL ); + { + NTSTATUS ret; + if ((ret = fast_wake( variable, 1 )) == STATUS_NOT_IMPLEMENTED) + NtReleaseKeyedEvent( keyed_event, &variable->Ptr, FALSE, NULL ); + } } Why not move fast_wake() call into the NtReleaseKeyedEvent() implementation? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |STAGED CC| |leslie_alistair(a)hotmail.com Staged patchset| |https://github.com/wine-sta | |ging/wine-staging/tree/mast | |er/patches/ntdll-futex-cond | |ition-var -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 --- Comment #7 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Dmitry Timoshkov from comment #6)
Why not move fast_wake() call into the NtReleaseKeyedEvent() implementation?
If you're envisioning using the key as the pointer to the futex word, that won't work. The key doesn't have to be a valid pointer; there are tests for this in ntdll:om. If you're envisioning using some internal field as the futex word (and then copying most of the logic from the server to ntdll) I think that wouldn't work since keyed events need to be cross-process. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |be541f1b0cf2bec93b59d3a2fde | |2b6c0593c011b Status|STAGED |RESOLVED Resolution|--- |FIXED --- Comment #8 from Zebediah Figura <z.figura12(a)gmail.com> --- This is committed now, <https://source.winehq.org/git/wine.git/commit/be541f1b0cf2bec93b59d3a2fde2b6c0593c011b>. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45524 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #9 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 4.2. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org