RFR: 8224974: Implement JEP 352

Andrew Dinn adinn at redhat.com
Tue Aug 6 15:58:57 UTC 2019


Hello Dmitry,

On 06/08/2019 15:50, Dmitry Chuyko wrote:
> 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.

Sure, no problem. Thank you for looking at the patch.

> 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.

No this behaviour is not currently tested. However, the only client at
present will never exercise that path so it is not critical to test it
now. I'd be happy to address testing of this behaviour as part of a
follow-up JIRA issue if you would be so good as to raise it. I say that
because I would very much like to get this functionality into a release
to simplify more extensive testing by Red Hat's middleware teams.

> 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.

The documentation of the API under test makes it clear that both errors
can occur and under what circumstances. However, a note in the test will
certainly do no harm. I will insert one before checking in the patch.

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

I don't see any need for this now that MapFail has been provided. Wit
that alterative in place for checking map failures on non-DAX file
syetems PmemTest is now primarily intended to test behaviour with a DAX
file system which it expects to have been set up in advance as described
in the main comment. So, the scenario you describe is not really an
intended usage and I woudl argue that a failure is the right way to
signal 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


More information about the core-libs-dev mailing list