On 23/05/2019 11:55, Andrew Dinn wrote:
Hi,
Could I please have reviews of the following change set which implements
JEP 352:
JEP: https://openjdk.java.net/jeps/352
JIRA: https://bugs.openjdk.java.net/browse/JDK-8207851
webrev: http://cr.openjdk.java.net/~adinn/8207851/webrev.00/
I would also very much like to target this implementation for JDK13.
You may want to take a pass over the JEP to make sure that everything is
accurate. I notice, for example, the section on BufferPoolMXBean has the
old name READ_ONLY_PERSISTENT. We went through a couple of iterations in
the discussion here so there might be a few others.
I think the API changes are okay. I don't see a CSR yet but I assume
you'll get to that soon.
I've read through the changes to java.base and jdk.unsupported.
Just a few minor points:
- I assume the update FileChannel.java should be dropped as it's just a
left over from when we agreed to split out the updates to the Java SE API.
- com.sun.nio.file.ExtendedMapMode.*_SYNC are missing javadoc, or rather
the descriptions are truncated with "...". I think this dates from when
were working out the right place to expose these constants. The source
file (and the internal ExtendedMapModes are missing copyright headers too).
- We didn't discuss the name of the buffer pool that is exposed through
the JMX/management interface. We could take inspiration from the names
of the CodeHeap spaces that are exposed with MemoryPoolMXBeans as there
is an established convention for naming there, e.g. "mapped -
'non-volatile memory'".
- Minor nit in Unmapper is that the methods to increment/decrement the
usage should use Java conventions so probably should be incrementUsage
and decrementUsage.
- PmemTest. This is awkward and I wonder if it should be @run
main/manual rather than @ignore. Also `@modules jdk.unsupported` would
be useful to ensure it will be skipped if run with a test JDK that
doesn't have this module.
-Alan
