zhaih commented on PR #12257: URL: https://github.com/apache/lucene/pull/12257#issuecomment-1533425493
Hi @jimczi, thanks for the comment. Yeah I'm aware of that PR and also looking forward to that. However I decided to make this PR because: 1. This can resolve the thread-safety issue faster, as I would imagine we will iterate on that PR a bit longer. 2. I'm not sure whether the concurrent writable & readable HNSW graph will completely replace the current one, citing the author's own words: > With the optimizations it's still about 38% slower than the non-concurrent code (including the HashMap optimization for the non-concurrent that went in). I think the original impl still has it's own value in case where people don't necessarily need concurrent write, so have a thread-safe search solution may still be desirable? (Especially when it is quite easy to make one) As for > Adding multi-threading just for search would be misleading imo since this class exposes insertion too. Yes I agree, but I think it's more of a javadoc issue? If we documented it clearly that HNSW graph is not supposed to be written and read at the same time unless explicitly stated, maybe it is more acceptable? Just like HashMap, ArrayList and tons of other data structures that are not supposed to be safe when written & read together but expose an insertion API, if we don't advertise it as "totally safe" then I don't think people should have that impression? -- 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
