Uwe Schindler created LUCENE-8780:
-------------------------------------
Summary: Improve ByteBufferGuard in Java 11
Key: LUCENE-8780
URL: https://issues.apache.org/jira/browse/LUCENE-8780
Project: Lucene - Core
Issue Type: Improvement
Components: core/store
Affects Versions: master (9.0)
Reporter: Uwe Schindler
Assignee: Uwe Schindler
In LUCENE-7409 we added {{ByteBufferGuard}} to protect MMapDirectory from
crushing the JVM with SIGSEGV when you close and unmap the mmapped buffers of
an IndexInput, while another thread is accessing it.
The idea was to do a volatile write access to flush the caches (to trigger a
full fence) and set a non-volatile boolean to true. All accesses would check
the boolean and stop the caller from accessing the underlying ByteBuffer. This
worked most of the time, until the JVM optimized away the plain read access to
the boolean (you can easily see this after some runtime of our by-default
ignored testcase).
With master on Java 11, we can improve the whole thing. Using VarHandles you
can use any access type when reading or writing the boolean. After reading Doug
Lea's expanation and some testing, I was no longer able to crush my JDK (even
after running for minutes unmapping bytebuffers).
The apraoch is the same, we do a full-fenced write (standard volatile write)
when we unmap, then we yield the thread (to finish in-flight reads in other
threads) and then unmap all byte buffers.
On the test side (read access), instead of using a plain read, we use the new
"opaque read". Opaque reads are the same as plain reads, there are only
different order requirements. Actually the main difference is explained by Doug
like this: "For example in constructions in which the only modification of some
variable x is for one thread to write in Opaque (or stronger) mode,
X.setOpaque(this, 1), any other thread spinning in
while(X.getOpaque(this)!=1){} will eventually terminate. Note that this
guarantee does NOT hold in Plain mode, in which spin loops may (and usually do)
infinitely loop -- they are not required to notice that a write ever occurred
in another thread if it was not seen on first encounter." - And that's waht we
want to have: We don't want to do volatile reads, but we want to prevent the
compiler from optimizing away our read to the boolean. So we want it to
"eventually" see the change. By the much stronger volatile write, the cache
effects should be visible even faster (like in our Java 8 approach, jsut now we
improved our read side).
The new code is much slimmer (theoretically we could also use a AtomicBoolean
for that and use the new method {{getOpaque()}}, but I wanted to prevent extra
metod calls, so I used a VarHandle directly.
It's setup like this:
- The underlying boolean is a private member (with unused suppress wanrings, as
its unused by the java compiler), marked as volatile (that's the
recommendation, but in reality it does not matter at all).
- We create a VarHandle to access this boolean, we never do this directly (this
is why the volatile marking does not affect us).
- We use VarHandle.setVolatile() to change out "invalidated" boolean to "true",
so enforcing a full fence
- On the read side we use VarHandle.getOpaque() instead of VarHandle.get() (as
it would be the effect of our old code in Java 8).
I had to tune our test a bit, as the VarHandles make it take longer until it
actually crushes (as optimizations jump in later). I also used a random for the
reads to prevent the optimizer from removing all the bytebuffer reads. When we
commit this, we can disable the test again (it takes approx 50 secs on my
machine).
I'd still like to see the differences between the plain read and the opaque
read in production, so maybe [~mikemccand] or [~rcmuir] can do a comparison
with nightly benchmarker?
Have fun, maybe [~dweiss] has some ideas, too.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
