RFR: 8224974: Implement JEP 352

[Dmitry Chuyko]

Hi Andrew,

Thanks for previous detailed explanations, and great, there are now 
checks for exceptions in webrev.11. Let me add just a few comments about 
that.

On 8/1/19 12:49 PM, Andrew Dinn wrote:
> Hello Dmitry,
>
> On 01/08/2019 05:09, Dmitry Chuyko wrote:
>> Just a small comment about the tests. As you can see, some of tests in
>> jdk/java/nio/channels/FileChannel check various modes, arguments, and
>> expected exceptions. E.g. see MapTest, Mode and Args.
>>
>> I noticed that in your changes for the new map mode there are new
>> exceptions thrown in different situations. For example, when the
>> required mode is not supported, or it does not fit. In particular, this
>> should happen correctly on systems that do not support nvram. Have you
>> considered the possibility of extending or adding tests for this behavior?
> I agree that these failure cases need better test coverage and
> automation. However, that is not to say they have not been tested. All
> current success and failure cases can be exercised by the current mmap
> test (PmemTest) if run in the correct circumstance. Unfortunately,
> automatic testing is not straightforward.

New Unsafe.writebackMemory()/checkWritebackMemory()/checkWritebackEnabled() @throws RuntimeException "if memory writeback is not supported...". There is no test that check this behavior of Unsafe class while it seems to be relatively simple as inner check actually operates with a constant value.

New MapFail test succeeds if proper IOException or any UnsupportedOperationException was caught but it aren't those situations actually 2 different ones that require distinct checks? If you say that is the situation when results depend on Linux version, it makes sense at least to put a comment in the test because it's really not trivial.

Can PmemTest check IOException with message "map with mode MAP_SYNC unsupported" as a part of expected behavior, not just showing a test failure?

-Dmitry

>
> 1) On x86_64 where MAP_SYNC is supported test PMemTest includes
> instructions to exercise a successful map from a real or emulated DAX
> file system. It can also be used (and has been used) to exercise the
> IOException failure path in the case where the file it tries to open
> belongs to a non-DAX file system (see line 99).
>
> Note that testing for success or failure cannot be done automatically
> using the test as it currently stands. Testing for a successful map
> requires mounting a real or emulated DAX file system at a known mount
> point and creating a writable dir to hold the mapped file. Testing for
> the IOException failure requires either setting up an equivalent non-DAX
> file system mount or editing the test to open the file in a non-DAX file
> system dir like, say, /tmp.
>
> A new, separate test for the IOException failure could be automated on
> x86_64 by trying to map a file located in /tmp (on the assumption that
> /tmp is not mounted as a DAX file system). Of course, at present this
> will only give the IOException result when MAP_SYNC is supported. Given
> a suitably old Linux/x86_64, UnsupportedOperationException could also be
> thrown.
>
> 2) On AArch64 where MAP_SYNC is not currently supported PmemTest clearly
> cannot be used to exercise the success path. However, it can be used
> (and has been used) to exercise the UnsupportedOperationException
> failure path.
>
> A check for the UnsupportedOperationException failure could be automated
> on AArch64 by a new test as described above. Of course, once MAP_SYNC
> support arrives in a new Linux/AArch64 it would also become for
> IOException to be thrown.
>
> So, to sum up, it would be possible to add a new, automatic test that
> checks for one or other failure occurring. I am happy to add such a test
> to the next webrev.
>
> 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
>