Avoid notifying when no one is waiting on proceedLock #1657
Conversation
|
|
|
I don't understand why "they cannot occur at the same time"? wakeUp() can be expected to be invoked in other threads rather than the one waiting. |
|
@maosongfu They are guarded by the same lock. If wait is executing then execution cannot reach notify and vice versa. |
|
@maosongfu It was my rusty understanding of how wait works. Wait actually gives up the monitor and waits for another thread to enter the same monitor and call notify. So you're right and that's why the test is failing. Let me change the implementation. |
pankajroark
changed the title
|
Proposed a different way of optimizing, changed the title to reflect that. |
|
@pankajroark Hi Can you add more comments on it, and describe the case this pull request covers in this pr's description? |
|
@maosongfu I changed the description and added more information. Let me know if any more info is required. Thanks |
|
looks good to me. 👍 |
Avoid notifying when no one is waiting on proceedLock
In one of our Heron jobs we found that the spout is spending 20-30% time in just calling notify in SlaveLooper. Notify happens to be an expensive call and in this case it seems that the invocation of notify is almost always redundant since SpoutInstance calls SlaveLooper from a single thread. Optimizing that here. The cost of optimization is two extra volatile writes while waiting and one extra volatile read in wakeUp. volatile reads are pretty fast so wouldn't add much overhead to wakeUp. volatile writes are slower but waits are less common and cost of volatile writes is much lower compared to wait.