Use double checked locking to remove sync block #361
Conversation
|
You do know that double-locking is nearly universally condemned as an anti-pattern? Beyond question of correctness, how much is expected improvement, and for what cases? Ideally there would be a benchmark to show performance improvement. |
|
Pre Java 1.5 you would be correct. The Java memory model was fixed in Java 1.5, so now double checked locking works fine, as long as the variable is volatile. Most of the info on double checked locking if you just google for it is outdated, and refers to the old memory model before JSR 133. There was also some FUD just after 1.5 about how synchronized was much cheaper now so it was still not necessary, however a volatile read is still much cheaper on most platforms. On server class systems with lots of processors all trying to acquire the lock it can become very expensive. I can provide some numbers if you really want, but in general the more processors you have the bigger the difference it will make. |
|
Ah. I did not notice that you had used I would be interested in just some validation; I assume that you saw lock contention and this is based on solving an actual problem. I just have occasionally received patches that have turned out to be more speculative, which is why I ask nowadays. I think it'd be fine to just describe observed improvement (problem there was, how its solved), although of course numbers would be great if you already had some. But it is hard to get solid and meaningful numbers so I am ok without. Does this make sense? Also: thank you for the patch! There have been couple of other synchronization hot spots, and this one is on critical path (esp. when dealing with small objects), so it is not entirely surprising. |
|
While profiling I observed an inordinate amount of time spent in the method, and YJP monitor profiling was showing contention. I also just know from my general experience profiling Wildfly that even short held locks can have a big impact on performance if they are on a critical path that lots of threads are taking. If you want numbers I should be able to get them early next week. |
|
Ok. The only concern I have is that sometimes profiled bottlenecks are more artifacts of the tool -- having said that, I don't think I have ever added harmful optimizations that way; at most things that may not have had same impact as what original snapshot would suggest. But as I said this particular code path is something that I could see being in critical path. If you can get the numbers, that'd be great. I will think about this a bit, and may just go ahead even before; depending on how fast I can process other contributions (there's been uptick on reports since 2.3.0, which is a good sign wrt adoption and getting new users). |
|
Actually I think this makes sense; and although I am still interested in numbers (if possible), will just integrate this in. |
Use double checked locking to remove sync block
No description provided.