Alexandre Julliard wrote:
Gavriel State gav@transgaming.com writes:
After some more thinking, Ove and I have come up with a mechanism that should eliminate most of the wineserver overhead for mutexes and semaphores, without the need to resort to a kernel module. We're probably going to give this a try over the next few days, so any feedback will be very much appreciated.
I don't see how you are going to make this work reliably. A basic design principle of the server is that no matter what a client process does, it cannot break either the server or other clients; given the number of bugs Windows apps contain, I feel this is very important.
Definitely a drawback, I agree. But the shared memory area could be relatively high in the address space, so that a stray pointer is much more likely to run into unmapped memory and segfault before overwriting crucial data.
The only way we're going to be able to get both speed and safety though is to have some kernel support. While that's beyond the scope of what I'd like to accomplish in the short term, I think that it's something worth thinking about more. While the solution proposed by David Howells may be overkill, I'm not sure that simply changing the communication mechanism as you suggested (http://www.winehq.com/News/2000-37.html#0) will speed things up enough - we'll still have the context switch overhead.
As soon as you introduce a shared memory area, you need the collaboration of all clients to ensure the stability of the whole system, since any client can corrupt system data structures. This is very bad. Also since the server is single-threaded its data structures don't need to be protected; but as soon as you manipulate them from multiple threads you need locking mechanisms, which will probably cost a lot in performance too.
At least in the case of mutexes, I believe that all we need to do is make sure that the count field is always modified through interlocked increment/decrement/etc including the wineserver.
Here's some slightly modified pseudocode:
Client:
ReleaseMutex: Do everything that the server do_release used to do, except wake_up InterlockedDecrement(&count) Check if there were other threads waiting, and call the server to wake them.
WaitForSingleObject: last = InterlockedCompareExchange(&count,bignum,0) if count == bignum we have the lock count = last+1 do everything the server mutex_satisfied does return WAIT_OBJECT_0 else if we're the mutex owner count++ return WAIT_OBJECT_0 else call wineserver's WaitForSingleObject
Server:
mutex_signalled: last = InterlockedCompareExchange(&count,bignum,0) if count == bignum || we're the mutex owner we have the lock count = last+1
mutex_satisfied: Same as before, but don't change the count
check_wait: Same as before, but if we're checking multiple objects and one fails, we need to reset the count for any mutex objects that we signalled successfully.
Definitely more complicated than before, but if we're careful, the count value can itself serve as the locking mechanism. The only case I can see where there's a potential issue is where a process releases the mutex lock, calls the server to wake up any waiting threads, and before the server signals the waiting threads, a third thread grabs the mutex. While this isn't ideal, it gets resolved when the third thread releases and tells the server to wake up other processes.
This solution is probably only worthwhile as a short-term hack that we'll try in our tree. While I think it ought to work for the specific things that we're trying to accomplish immediately, it's probably too dangerous to put into the WineHQ tree.
If there's interest in moving forward on better solutions to the server overhead problem, we're certainly willing to help with those.
-Gav