Closed ioquatix closed 1 year ago
@tarcieri I'm personally against merging this at this point in time, but I wonder about how you feel about this. For single threaded event reactor, there seems to be no point having mutex. All specs also pass, so clearly we are not checking this behaviour. Do we make any official guarantee about thread safety?
The reason the mutex is there to begin with was to provide a pessimistic baseline behavior between MRI and JRuby. Java NIO selectors are absolutely multithreaded and intended to be used from multiple threads simultaneously, and have rather complex semantics to that effect. So I threw a mutex around all implementations as the shortest path to ensuring they behave in a roughly equivalent manner.
To me it's a question of if the mutex is removed, do the semantics still match? That answer, even with the GIL, is almost certainly no, and I'm wondering if the mutex were removed from the C version if it causes memory safety issues arising from potential concurrent modification.
If it were possible to lean on the GIL for some of that thread safety, while still using a mutex on JRuby, and getting roughly equivalent behaviors that way, that sounds ok.
A safer option to mitigate the mutex overhead would be to add a #synchronize
method that acquires the mutex and calls a block, then skips subsequent mutex acquisitions
It took me a while to grok what you said but it’s not a silly idea - locking it to one thread.
I don't feel strongly about pushing this across the finish line.
I wondered how much overhead the mutex was introducing.
It has about ~7% improved performance.
In real world test, it was inconclusive, which was expected.