RFR: 8224974: Implement JEP 352
adinn at redhat.com
Mon Aug 19 11:29:32 UTC 2019
Thanks for looking at the patch again. I think I have addressed all your
concerns (comments inline). Webrev and retest results at the end.
On 16/08/2019 11:39, Alan Bateman wrote:
> I think the main changes since I looked at it previously have been in
> the tests.
That's mostly it. I did also fix a problem that was breaking build of
x86_32. I also ensured that MAP_SYNC maps fail as expected on that arch.
> On non-Linux platforms, FileChannel.map should throw UOE when invoked
> with a mode map of READ_ONLY_SYNC or READ_WRITE_SYNC so I think we need
> a test for that.
> MapFail seems fragile. If you add @modules java.base/jdk.internal.misc
> to the test description then it could use Unsafe::isWritebackEnabled and
> I think would simplify the test and checks. It would mean it could run
> on all platforms. Also "MapFail" is probably too general a name for a
> test in MBB because its specific to the extended map modes, not a
> general test of map failing.
I renamed the test to MapSyncFail and modified it to run without
restriction to a specific os or cpu.
I also generalized it to run twice with a boolean arg which selects mode
READ_ONLY_SYNC on the first run and READ_WRITE_SYNC on the second one.
The logic of the test is now to expect
1) IOException if Unsafe.isWriteBackEnabled -> true
2) UnsupportedOperationException if Unsafe.isWriteBackEnabled -> false
If the wrong exception or neither exception is thrown the test fails.
Case 1 currently only applies for x86_64.
Case 2 applies for all other architectures.
> In passing, MappedByteBuffer load/isLoaded check the fd value before
> isSync, can force() do the same? Also the @return on the private isSync
> method is very wordy and I don't think needs to duplicate the method
Sure, I have modified force() to do that check first.
Of course, that means that force(int, int) is going to repeat the same
test -- it has to because it may be called direct without going via force().
However, that's not really a problem since the compiler should elide the
I also shortened the text following the @return annotation as requested.
passes as expected on x86_64 (only arch for which DAX file system is
fails to pass as expected on aarch64 and x86_32 (however, this case is
covered by the next test)
passes with expected exceptions on Linux for x86_64 (IOException),
aarch64 and x86_32 (UnsupportedOperationException).
not tested on other arch/OS combinations (I have no access to the
Red Hat MW tests:
All still passing successfully
still in progress
Is it ok to push if the submit test comes back clean?
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