RFR: 8224974: Implement JEP 352

[Andrew Dinn]

Hi Alan,

Thanks for reviewing the JEP one more time.

On 16/06/2019 09:47, Alan Bateman wrote:
> I re-read the JEP, trying to put myself in the position of someone
> reading it for the first time in 2020.
> 
> Summary section:
> 
> What would you think about replacing this with a sentence that makes it
> clear that the JEP adds new JDK-specific file mapping modes to allow the
> FileChannel API create MappedByteBuffers over non-volatile memory?

I would be happy with that way of describing the behaviour change except
that what you suggest doesn't mention actually making it work -- which
/is/ part of the JEP. How about:

"Add new JDK-specific file mapping modes, allowing the FileChannel API
to be used to create MappedByteBuffers over non-volatile memory, and
extend the underlying implementation to support this new use case"

> Goals section:
> 
> I think the first paragraph could be re-worded to make it clear that the
> goal is to use the existing MappedByteBuffer API to access NVM.

Yes indeed:

"This JEP proposes that class MappedByteBuffer be reworked to support
access to non-volatile memory (NVM). The only API change required is a
new enumeration employed by FileChannel clients to request mapping of a
file located on an NVM-backed file system rather than a conventional,
file storage system. Recent changes to the MappedByteBufer API mean that
it supports all the behaviours needed to allow direct memory updates and
provide the durability guarantees needed for higher level, Java client
libraries to implement persistent data types (e.g. block file systems,
journaled logs, persistent objects, etc.). The implementations of
FileChannel and MappedByteBuffer need revising to be aware of this new
backing type for the mapped file."

> Paragraphs 2-5 split this into two sub-goals. The first suggests that it
> extends the MBB API but this is no longer the case. The second also
> hints that it adds a new API. I think these two need to be re-worded.

Agreed. How about this:

"The primary goal of this JEP is to ensure that clients can access and
update NVM from a Java program efficiently and coherently. A key element
of this goal is to ensure that individual writes (or small groups of
contiguous writes) to a buffer region can be committed with minimal
overhead i.e. to ensure that any changes which might still be in cache
are written back to memory."

n.b. I snuck in the word coherence because I thought it clarified the
notion of committing to memory.

> Goal 3 and 4 are okay, although I think the 4th could be summarized as
> allowing mapped buffers on NVM to be tracked by the existing monitoring
> and management APIs.

Agreed. So, renumbering accordingly, how about this:

"A second, subordinate goal is to implement this commit behaviour using
a restricted, JDK-internal API defined in class Unsafe, allowing it to
be re-used by classes other than MappedByteBuffer that may need to
commit NVM.

A final, related goal is to allow buffers mapped over NVM to be tracked
by the existing monitoring and management APIs."

> Description section
> 
> It might be clearer of "Proposed Java API Changes" were renamed to
> "Proposed JDK-specific API changes".

Agreed.

> One other thing to mention is that I re-read the javadoc for the MBB
> force methods and I think we need to adjust one of the sentences in the
> existing (and new) methods to take account of implementation specific
> map modes. I've created JDK-8226203 [1] to track it. As support for
> implementation specific map modes is in new in Java SE 13 then it might
> be worth trying to get that fixed now while it is fresh in our minds.
Sure, I'll post an RFR with the javadoc patch as a separate step. Can it
go into jdk13? or is it too late for that?

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander