Bugzilla

Hey Oskar, 
>  Just pushed a WIP. I think the locks I removed were relatively straight forward, but I'm not sure about FlushQueue(), because there are lots of locks in there and also stuff happening in between that doesn't need locking (see https://searchfox.org/mozilla-central/source/netwerk/ipc/ChannelEventQueue.cpp#39,46,56,81,86,103,108,113,120,125).
So in my mind it didn't make sense to put that whole function under a single lock, because then we are stopping other threads from accessing member vars for a prolonged time. I'm not sure about the performance tradeoffs here (or how to test them). That's why I called mMutex.Unlock() before calling FlushQueue() in MaybeFlushQueue() , but I'm also not sure if that's the correct pattern for unlocking before scope ends.

The current code (w/o your patch) minimizes critical section duration, reducing the time that other threads are blocked. However, the large number of locks introduces a higher lock acquisition overhead. An optimal solution would be to balance the duration of critical sections and the number of locks used. We need to adjust the code so that critical sections avoid executing any long-running tasks particularly the [runnables](https://searchfox.org/mozilla-central/rev/387f3edbef37d31b2e91fb0812c74b54729e86ff/netwerk/ipc/ChannelEventQueue.cpp#94) in the event queue.  Hence, I think we can get rid of `mMutex.Unlock()` as well and further optimize FlushQueue.  Apart from this I dont see any other code paths that could block for considerable duration.  Please run the code with the thread sanitizers in the try server. Let me know if you need more info on that.

Hey Oskar, 
>  Just pushed a WIP. I think the locks I removed were relatively straight forward, but I'm not sure about FlushQueue(), because there are lots of locks in there and also stuff happening in between that doesn't need locking (see https://searchfox.org/mozilla-central/source/netwerk/ipc/ChannelEventQueue.cpp#39,46,56,81,86,103,108,113,120,125).
So in my mind it didn't make sense to put that whole function under a single lock, because then we are stopping other threads from accessing member vars for a prolonged time. I'm not sure about the performance tradeoffs here (or how to test them). That's why I called mMutex.Unlock() before calling FlushQueue() in MaybeFlushQueue() , but I'm also not sure if that's the correct pattern for unlocking before scope ends.

The current code (w/o your patch) minimizes critical section duration, reducing the time that other threads are blocked. However, the large number of locks introduces a higher lock acquisition overhead. An optimal solution would be to balance the duration of critical sections and the number of locks used. We need to adjust the code so that critical sections avoid executing any long-running tasks particularly the [runnables](https://searchfox.org/mozilla-central/rev/387f3edbef37d31b2e91fb0812c74b54729e86ff/netwerk/ipc/ChannelEventQueue.cpp#94) in the event queue.  Hence, I think we can get rid of `mMutex.Unlock()` as well and further optimize FlushQueue.  Apart from this I dont see any other code paths that could block for considerable duration.  Kindly share if you come across more such paths.

 Please run the code with the thread sanitizers in the try server. Let me know if you need more info on that.