uschindler commented on code in PR #12290:
URL: https://github.com/apache/lucene/pull/12290#discussion_r1199073297
##########
lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java:
##########
@@ -65,14 +62,8 @@ public ByteBufferGuard(String resourceDescription,
BufferCleaner cleaner) {
public void invalidateAndUnmap(ByteBuffer... bufs) throws IOException {
if (cleaner != null) {
invalidated = true;
- // This call should hopefully flush any CPU caches and as a result make
- // the "invalidated" field update visible to other threads. We
specifically
- // don't make "invalidated" field volatile for performance reasons,
hoping the
- // JVM won't optimize away reads of that field and hardware should ensure
- // caches are in sync after this call. This isn't entirely "fool-proof"
- // (see LUCENE-7409 discussion), but it has been shown to work in
practice
- // and we count on this behavior.
- barrier.lazySet(0);
+ // Makes "invalidated" field visible to other threads.
+ VarHandle.fullFence();
Review Comment:
I'd like to merge this, but could we restore most of the previous comment.
The comment added is not correct, because the invalidated field is not
definitely visible to other threads (only in code not yet optimized).
So please refer to the "fool-proof" discussion too.
--
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
