https://bugs.winehq.org/show_bug.cgi?id=46252
Bug ID: 46252 Summary: StarCitizen has bad performance since bugfix on bug #46099 Product: Wine Version: 3.21 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: -unknown Assignee: wine-bugs@winehq.org Reporter: rawfox@freenet.de Distribution: ---
Since bug 46099 is fixed, the game has bad performance, fps is 3-5. https://bugs.winehq.org/show_bug.cgi?id=46099
To make the game perform normal, we need these two other patches applied to wine:
https://bugs.winehq.org/attachment.cgi?id=62897 and https://bugs.winehq.org/attachment.cgi?id=62902
I dont know mutch about WaitOnAddress and the futex thing in wine, but the game works definitly like a charm with those patches applied.
https://bugs.winehq.org/show_bug.cgi?id=46252
rawfox rawfox@freenet.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|-unknown |ntdll Severity|normal |major
https://bugs.winehq.org/show_bug.cgi?id=46252
rawfox rawfox@freenet.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |rawfox@freenet.de
https://bugs.winehq.org/show_bug.cgi?id=46252
rawfox rawfox@freenet.de changed:
What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=46099
https://bugs.winehq.org/show_bug.cgi?id=46252
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|major |normal
https://bugs.winehq.org/show_bug.cgi?id=46252
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|StarCitizen has bad |StarCitizen has bad |performance since bugfix on |performance |bug #46099 | Keywords| |performance
--- Comment #1 from Austin English austinenglish@gmail.com --- Is this a regression caused by 6060d2703c2e1382f076ad40ea43980781eaca2c? If I'm reading the bug correctly, it didn't work before, so I don't see how it would be a regression.
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #2 from rawfox rawfox@freenet.de --- (In reply to Austin English from comment #1)
Is this a regression caused by 6060d2703c2e1382f076ad40ea43980781eaca2c? If I'm reading the bug correctly, it didn't work before, so I don't see how it would be a regression.
No, the WaitOnAddress() seems to work now. Nontheless it is very very slow with the game and it needs this futex implementation of WaitOnAddress() to work properly.
This is valid for wine-4.0rc1 as well. The game works but has low performance. Patching /dlls/ntdll/sync.c with the two patches above gives the needed game performance.
I dont know why, thats up to you coders to figure that out.
What ive read on this topic is, Windows uses WaitOnAddress() while Linux uses kernel futexes. So to my understanding, a fast thread organisation is done in the linux Kernel while it seems to be in the ntdll on Windows.
A possible solution would be, to add these 2 patches into staging. For now, they conflict there in the staging patches with another futex condition implementation.
https://bugs.winehq.org/show_bug.cgi?id=46252
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12@gmail.com
--- Comment #3 from Zebediah Figura z.figura12@gmail.com --- (In reply to Austin English from comment #1)
Is this a regression caused by 6060d2703c2e1382f076ad40ea43980781eaca2c? If I'm reading the bug correctly, it didn't work before, so I don't see how it would be a regression.
Kind of, though not really. It's more of a continuation of that bug's regression from 7c430f5b3—I suspect that all the overhead here is coming from wineserver interaction (though certainly the CS doesn't help).
(In reply to rawfox from comment #2)
What ive read on this topic is, Windows uses WaitOnAddress() while Linux uses kernel futexes. So to my understanding, a fast thread organisation is done in the linux Kernel while it seems to be in the ntdll on Windows.
These would have to go into the kernel on Windows too, otherwise the thread couldn't schedule. The overhead in Wine comes from going through the server. Futexes go to the kernel too (well, sometimes), but they only do it once and we don't block on waiting for the server.
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #4 from rawfox rawfox@freenet.de --- Can you please merge these patches into the futex condition patch that is already existing in wine-staging ?
Im getting a nutter, doing that :\
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #5 from rawfox rawfox@freenet.de --- Created attachment 63216 --> https://bugs.winehq.org/attachment.cgi?id=63216 merged futex conditions patch for wine vanilla
this is the merged patch from wine-staging/patches/ntdll-futex-condition-var/0001-ntdll-Add-a-futex-based-condition-variable-implement.patch merged together with the StarCitizen performance patches implementing fast_wait/wake_addr().
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #6 from rawfox rawfox@freenet.de --- Created attachment 63217 --> https://bugs.winehq.org/attachment.cgi?id=63217 merged futex conditions patch for wine staging
this is the unmerged performance patch, adding required conditions to a wine-staging patched source.
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #7 from rawfox rawfox@freenet.de --- Hey people, please be so kind and check this patch. https://bugs.winehq.org/attachment.cgi?id=63216
It is the merged patch from wine-staging together with the fast_wait/wake_addr() from your recent futex tweaks for StarCitizen.
The former functionality should be ok, enhanced to make StarCitizen perform better.
This patch should replace the existing patch of the futex condition variable implementation (0001-ntdll-Add-a-futex-based-condition-variable-implement.patch) in wine-staging.
It is tested and works nice with the StarCitizen game while former functionality (bug 46208 and bug 45524) is not tested yet, but should be given, as you gladly named fast_wait() and fast_wait_addr() different. That was superb :)
It would be easiest if you could check this out and replace it in the wine-staging patches :)
cheers, raw^^
https://bugs.winehq.org/show_bug.cgi?id=46252
Greg Smith codedonewell@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |codedonewell@gmail.com
--- Comment #8 from Greg Smith codedonewell@gmail.com --- I can confirm that this ticket correctly reports the current behaviour. The two patches mentioned do indeed improve things very much. They were submitted to wine-devel before the code freeze, but have not been included/accepted.
https://bugs.winehq.org/show_bug.cgi?id=46252
rawfox rawfox@freenet.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|3.21 |4.0-rc5
https://bugs.winehq.org/show_bug.cgi?id=46252
Józef Kucia joseph.kucia@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|4.0-rc5 |3.21 CC| |joseph.kucia@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #9 from rawfox rawfox@freenet.de --- FYI, the game is full Silver rated. Recent updates from the developers of StarCitizen made the game fully installable and playable under wine !!!
With this patch going into wine-staging after you have approved it, StarCitizen can be rated Gold in Wine AppDB.
It can only be Gold, if the enduser does not need futher patches. Installing wine-staging must be enuff.
When i can do anything, to push the process, please let me know.
https://bugs.winehq.org/show_bug.cgi?id=46252
George Gibbs vash63@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |vash63@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #10 from rawfox rawfox@freenet.de --- Comment on attachment 63216 --> https://bugs.winehq.org/attachment.cgi?id=63216 merged futex conditions patch for wine vanilla
--- a/dlls/ntdll/sync.c 2019-01-07 22:06:29.876808084 +0100 +++ b/dlls/ntdll/sync.c 2019-01-07 23:51:22.897154211 +0100 @@ -26,6 +26,7 @@
#include <assert.h> #include <errno.h> +#include <limits.h> #include <signal.h> #ifdef HAVE_SYS_TIME_H # include <sys/time.h> @@ -36,6 +37,9 @@ #ifdef HAVE_SYS_POLL_H # include <sys/poll.h> #endif +#ifdef HAVE_SYS_SYSCALL_H +# include <sys/syscall.h> +#endif #ifdef HAVE_UNISTD_H # include <unistd.h> #endif @@ -61,7 +65,140 @@
HANDLE keyed_event = NULL;
static const LARGE_INTEGER zero_timeout; +#define TICKSPERSEC 10000000
+#ifdef __linux__
+static int wait_op = 128; /*FUTEX_WAIT|FUTEX_PRIVATE_FLAG*/ +static int wake_op = 129; /*FUTEX_WAKE|FUTEX_PRIVATE_FLAG*/
+static inline int futex_wait( int *addr, int val, struct timespec *timeout ) +{
- return syscall( __NR_futex, addr, wait_op, val, timeout, 0, 0 );
+}
+static inline int futex_wake( int *addr, int val ) +{
- return syscall( __NR_futex, addr, wake_op, val, NULL, 0, 0 );
+}
+static inline int use_futexes(void) +{
- static int supported = -1;
- if (supported == -1)
- {
futex_wait( &supported, 10, NULL );
if (errno == ENOSYS)
{
wait_op = 0; /*FUTEX_WAIT*/
wake_op = 1; /*FUTEX_WAKE*/
futex_wait( &supported, 10, NULL );
}
supported = (errno != ENOSYS);
- }
- return supported;
+}
+static inline NTSTATUS fast_wait( RTL_CONDITION_VARIABLE *variable, int val,
- const LARGE_INTEGER *timeout)
+{
- struct timespec timespec;
- LONGLONG timeleft;
- LARGE_INTEGER now;
- int ret;
- if (!use_futexes()) return STATUS_NOT_IMPLEMENTED;
- if (timeout && timeout->QuadPart != TIMEOUT_INFINITE)
- {
if (timeout->QuadPart >= 0)
{
NtQuerySystemTime( &now );
timeleft = timeout->QuadPart - now.QuadPart;
if (timeleft < 0) timeleft = 0;
}
else
timeleft = -timeout->QuadPart;
timespec.tv_sec = timeleft / TICKSPERSEC;
timespec.tv_nsec = (timeleft % TICKSPERSEC) * 100;
ret = futex_wait( (int *)&variable->Ptr, val, ×pec );
- }
- else
ret = futex_wait( (int *)&variable->Ptr, val, NULL );
- if (ret == -1 && errno == ETIMEDOUT) return STATUS_TIMEOUT;
- return STATUS_WAIT_0;
+}
+static inline NTSTATUS fast_sleep_cs( RTL_CONDITION_VARIABLE *variable,
- RTL_CRITICAL_SECTION *crit, const LARGE_INTEGER *timeout )
+{
- int val = *(int *)&variable->Ptr;
- NTSTATUS ret;
- RtlLeaveCriticalSection( crit );
- ret = fast_wait( variable, val, timeout );
- RtlEnterCriticalSection( crit );
- return ret;
+}
+static inline NTSTATUS fast_sleep_srw( RTL_CONDITION_VARIABLE *variable,
- RTL_SRWLOCK *lock, const LARGE_INTEGER *timeout, ULONG flags )
+{
- int val = *(int *)&variable->Ptr;
- NTSTATUS ret;
- if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED)
RtlReleaseSRWLockShared( lock );
- else
RtlReleaseSRWLockExclusive( lock );
- ret = fast_wait( variable, val, timeout );
- if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED)
RtlAcquireSRWLockShared( lock );
- else
RtlAcquireSRWLockExclusive( lock );
- return ret;
+}
+static inline NTSTATUS fast_wake( RTL_CONDITION_VARIABLE *variable, int val ) +{
- if (!use_futexes()) return STATUS_NOT_IMPLEMENTED;
- interlocked_xchg_add( (int *)&variable->Ptr, 1 );
- futex_wake( (int *)&variable->Ptr, val );
- return STATUS_SUCCESS;
+}
+#else +static inline NTSTATUS fast_sleep_cs( RTL_CONDITION_VARIABLE *variable,
- RTL_CRITICAL_SECTION *crit, const LARGE_INTEGER *timeout )
+{
- return STATUS_NOT_IMPLEMENTED;
+}
+static inline NTSTATUS fast_sleep_srw( RTL_CONDITION_VARIABLE *variable,
- RTL_SRWLOCK *lock, const LARGE_INTEGER *timeout, ULONG flags )
+{
- return STATUS_NOT_IMPLEMENTED;
+}
+static inline NTSTATUS fast_wake( RTL_CONDITION_VARIABLE *variable, int val ) +{
- return STATUS_NOT_IMPLEMENTED;
+} +#endif
static inline int interlocked_dec_if_nonzero( int *dest ) { @@ -1868,8 +2005,11 @@ */ void WINAPI RtlWakeConditionVariable( RTL_CONDITION_VARIABLE *variable ) {
- if (interlocked_dec_if_nonzero( (int *)&variable->Ptr ))
NtReleaseKeyedEvent( 0, &variable->Ptr, FALSE, NULL );
- if (fast_wake( variable, 1 ) == STATUS_NOT_IMPLEMENTED)
- {
if (interlocked_dec_if_nonzero( (int *)&variable->Ptr ))
NtReleaseKeyedEvent( keyed_event, &variable->Ptr, FALSE, NULL );
- }
}
/*********************************************************************** @@ -1879,9 +2019,12 @@ */ void WINAPI RtlWakeAllConditionVariable( RTL_CONDITION_VARIABLE *variable ) {
- int val = interlocked_xchg( (int *)&variable->Ptr, 0 );
- while (val-- > 0)
NtReleaseKeyedEvent( 0, &variable->Ptr, FALSE, NULL );
- if (fast_wake( variable, INT_MAX ) == STATUS_NOT_IMPLEMENTED)
- {
int val = interlocked_xchg( (int *)&variable->Ptr, 0 );
while (val-- > 0)
NtReleaseKeyedEvent( keyed_event, &variable->Ptr, FALSE, NULL );
- }
}
/*********************************************************************** @@ -1903,17 +2046,24 @@ const LARGE_INTEGER *timeout ) { NTSTATUS status;
interlocked_xchg_add( (int *)&variable->Ptr, 1 );
RtlLeaveCriticalSection( crit );
status = NtWaitForKeyedEvent( 0, &variable->Ptr, FALSE, timeout );
if (status != STATUS_SUCCESS)
- if ((status = fast_sleep_cs( variable, crit, timeout )) == STATUS_NOT_IMPLEMENTED) {
if (!interlocked_dec_if_nonzero( (int *)&variable->Ptr ))
status = NtWaitForKeyedEvent( 0, &variable->Ptr, FALSE, NULL );
- }
interlocked_xchg_add( (int *)&variable->Ptr, 1 );
RtlLeaveCriticalSection( crit );
- RtlEnterCriticalSection( crit );
status = NtWaitForKeyedEvent( keyed_event, &variable->Ptr, FALSE, timeout );
if (status != STATUS_SUCCESS)
{
if (!interlocked_dec_if_nonzero( (int *)&variable->Ptr ))
status = NtWaitForKeyedEvent( keyed_event, &variable->Ptr, FALSE, NULL );
}
else if (status != STATUS_SUCCESS)
interlocked_dec_if_nonzero( (int *)&variable->Ptr );
RtlEnterCriticalSection( crit );
- } return status;
}
@@ -1940,24 +2090,31 @@ const LARGE_INTEGER *timeout, ULONG flags ) { NTSTATUS status;
interlocked_xchg_add( (int *)&variable->Ptr, 1 );
if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED)
RtlReleaseSRWLockShared( lock );
else
RtlReleaseSRWLockExclusive( lock );
status = NtWaitForKeyedEvent( 0, &variable->Ptr, FALSE, timeout );
if (status != STATUS_SUCCESS)
- if ((status = fast_sleep_srw( variable, lock, timeout, flags )) == STATUS_NOT_IMPLEMENTED) {
if (!interlocked_dec_if_nonzero( (int *)&variable->Ptr ))
status = NtWaitForKeyedEvent( 0, &variable->Ptr, FALSE, NULL );
- }
interlocked_xchg_add( (int *)&variable->Ptr, 1 );
- if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED)
RtlAcquireSRWLockShared( lock );
- else
RtlAcquireSRWLockExclusive( lock );
if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED)
RtlReleaseSRWLockShared( lock );
else
RtlReleaseSRWLockExclusive( lock );
status = NtWaitForKeyedEvent( keyed_event, &variable->Ptr, FALSE, timeout );
if (status != STATUS_SUCCESS)
{
if (!interlocked_dec_if_nonzero( (int *)&variable->Ptr ))
status = NtWaitForKeyedEvent( keyed_event, &variable->Ptr, FALSE, NULL );
}
else if (status != STATUS_SUCCESS)
interlocked_dec_if_nonzero( (int *)&variable->Ptr );
if (flags & RTL_CONDITION_VARIABLE_LOCKMODE_SHARED)
RtlAcquireSRWLockShared( lock );
else
RtlAcquireSRWLockExclusive( lock );
- } return status;
}
@@ -1987,6 +2144,89 @@ return FALSE; }
+#ifdef __linux__
+static int addr_futex_table[256];
+static inline int *hash_addr( const void *addr ) +{
- ULONG_PTR val = (ULONG_PTR)addr;
- return &addr_futex_table[(val >> 2) & 255];
+}
+static inline NTSTATUS fast_wait_addr( const void *addr, const void *cmp, SIZE_T size,
const LARGE_INTEGER *timeout )
+{
- int *futex;
- int val;
- LARGE_INTEGER now;
- timeout_t diff;
- struct timespec timespec;
- int ret;
- if (!use_futexes())
return STATUS_NOT_IMPLEMENTED;
- futex = hash_addr( addr );
- /* We must read the previous value of the futex before checking the value
* of the address being waited on. That way, if we receive a wake between
* now and waiting on the futex, we know that val will have changed. */
- val = interlocked_cmpxchg( futex, 0, 0 );
- if (!compare_addr( addr, cmp, size ))
return STATUS_SUCCESS;
- if (timeout)
- {
if (timeout->QuadPart > 0)
{
NtQuerySystemTime( &now );
diff = timeout->QuadPart - now.QuadPart;
}
else
diff = -timeout->QuadPart;
timespec.tv_sec = diff / 1000000;
timespec.tv_nsec = diff % 1000000;
ret = futex_wait( futex, val, ×pec );
- }
- else
ret = futex_wait( futex, val, NULL );
- if (ret == -1 && errno == ETIMEDOUT)
return STATUS_TIMEOUT;
- return STATUS_SUCCESS;
+}
+static inline NTSTATUS fast_wake_addr( const void *addr ) +{
- int *futex;
- if (!use_futexes())
return STATUS_NOT_IMPLEMENTED;
- futex = hash_addr( addr );
- interlocked_xchg_add( futex, 1 );
- futex_wake( futex, INT_MAX );
- return STATUS_SUCCESS;
+} +#else +static inline NTSTATUS fast_wait_addr( const void *addr, const void *cmp, SIZE_T size,
const LARGE_INTEGER *timeout )
+{
- return STATUS_NOT_IMPLEMENTED;
+}
+static inline NTSTATUS fast_wake_addr( const void *addr ) +{
- return STATUS_NOT_IMPLEMENTED;
+} +#endif
/***********************************************************************
RtlWaitOnAddress (NTDLL.@)
*/ @@ -2005,6 +2245,9 @@ if (size != 1 && size != 2 && size != 4 && size != 8) return STATUS_INVALID_PARAMETER;
- if ((ret = fast_wait_addr( addr, cmp, size, timeout )) != STATUS_NOT_IMPLEMENTED)
return ret;
- select_op.keyed_event.op = SELECT_KEYED_EVENT_WAIT; select_op.keyed_event.handle = wine_server_obj_handle( keyed_event ); select_op.keyed_event.key = wine_server_client_ptr( addr );
@@ -2059,6 +2302,9 @@ */ void WINAPI RtlWakeAddressAll( const void *addr ) {
- if (fast_wake_addr( addr ) != STATUS_NOT_IMPLEMENTED)
return;
- RtlEnterCriticalSection( &addr_section ); while (NtReleaseKeyedEvent( 0, addr, 0, &zero_timeout ) == STATUS_SUCCESS) {} RtlLeaveCriticalSection( &addr_section );
@@ -2069,6 +2315,9 @@ */ void WINAPI RtlWakeAddressSingle( const void *addr ) {
- if (fast_wake_addr( addr ) != STATUS_NOT_IMPLEMENTED)
return;
- RtlEnterCriticalSection( &addr_section ); NtReleaseKeyedEvent( 0, addr, 0, &zero_timeout ); RtlLeaveCriticalSection( &addr_section );
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #11 from rawfox rawfox@freenet.de --- The patch for wine vanilla had an unused argument at invoke_apc(), preventing a successfull build and dropped the error, too many arguments.
https://bugs.winehq.org/show_bug.cgi?id=46252
rawfox rawfox@freenet.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Distribution|--- |Fedora Version|3.21 |4.0
https://bugs.winehq.org/show_bug.cgi?id=46252
Józef Kucia joseph.kucia@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|4.0 |3.21
https://bugs.winehq.org/show_bug.cgi?id=46252
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |cc8f9b64194f115de1161526e2f | |0ebbcb0bdcc4b Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED
--- Comment #12 from Zebediah Figura z.figura12@gmail.com --- This is implemented in Wine now, https://source.winehq.org/git/wine.git/commitdiff/cc8f9b64194f115de1161526e2f0ebbcb0bdcc4b.
https://bugs.winehq.org/show_bug.cgi?id=46252
--- Comment #13 from rawfox rawfox@freenet.de --- wine-4.1-62-gc3ac646a8d
Works like a charm !!! Incredible. Can not thank you enuff for your work, but ... thank you !!
https://bugs.winehq.org/show_bug.cgi?id=46252
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #14 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 4.2.