zhaih commented on code in PR #12660:
URL: https://github.com/apache/lucene/pull/12660#discussion_r1369658959
##########
lucene/core/src/java/org/apache/lucene/util/hnsw/NeighborArray.java:
##########
@@ -35,6 +38,9 @@ public class NeighborArray {
float[] score;
int[] node;
private int sortedNodeSize;
+ public final ReadWriteLock rwlock = new ReentrantReadWriteLock(true);
Review Comment:
I think we can hide the lock inside the `NeighborArray` by adding some APIs,
but it's hard to make a totally thread safe class given how we use it right
now: for example, when we add a neighbor and need to purge the non-diverse one,
we're adding -> sorting -> calculating non-diverse -> removing. And it's not
enough to just ensure each operation to be atomic because someone can always
add a node after we sort and makes things more complex. So even I create this
`ConcurrentNeighborArray` and every operation is nicely protected the caller
still need to be extra careful...
But with the caller control the lock, it's much easier to control this
"transaction" behavior and only add lock when they need.
I'm not saying this is good, but given this is really a very hacky class
already (even before the lock) and internal to HNSW only maybe we can leave it
as a follow-up?
--
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
