jpountz commented on code in PR #12199:
URL: https://github.com/apache/lucene/pull/12199#discussion_r1132647604
##########
lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThreadPool.java:
##########
@@ -113,15 +113,19 @@ private synchronized DocumentsWriterPerThread newWriter()
{
* operation (add/updateDocument).
*/
DocumentsWriterPerThread getAndLock() {
- synchronized (this) {
- ensureOpen();
- DocumentsWriterPerThread dwpt =
freeList.poll(DocumentsWriterPerThread::tryLock);
- if (dwpt == null) {
- dwpt = newWriter();
+ ensureOpen();
+ DocumentsWriterPerThread dwpt =
freeList.poll(DocumentsWriterPerThread::tryLock);
+ if (dwpt == null) {
Review Comment:
I don't think that this is a case of double-checked locking. In general
double-checked locking tries to reduce the overhead of acquiring a lock by
adding a quick check before the lock. Here the logic is different, it would be
legal to remove the call to `poll` under lock, I only added it because there is
a chance that a thread had to wait on the lock, so we could check if another
DWPT was added to the queue in the meantime in order to save creating a new
DWPT. I don't think it's actually important, I could remove the second call to
`poll` to make it look less like double-checked locking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
