RFR: 8314120: Add tests for FileDescriptor.sync

Alan Bateman alanb at openjdk.org
Fri Aug 11 14:55:59 UTC 2023


On Thu, 10 Aug 2023 15:45:12 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> When backporting [JDK-8312127](https://bugs.openjdk.org/browse/JDK-8312127), I realized there are no targeted tests for `FileDescriptor.sync` that can be used to qualify the changes in that area. 
> 
> Additionally, we use `FD.sync` for durability in Java databases, and we want to make sure at least some smoke tests are available in OpenJDK. Asserting durability would be hard, but let's at least test the Java code does not throw unexpected exceptions and native code does not crash the VM.
> 
> The benchmark will show, among other things, that the recent change to `FileDescriptor.sync` does not affect the performance much, compared to the cost of the `fsync` itself. It deliberately targets tmpfs to provide the lowest actual FS overhead.
> 
> 
> Benchmark                Mode  Cnt    Score   Error  Units
> 
> # Before JDK-8312127
> FileDescriptorSync.sync  avgt   15  351,688 ? 2,477  ns/op
> 
> # After JDK-8312127
> FileDescriptorSync.sync  avgt   15  353,331 ? 2,116  ns/op
> 
> 
> The new regression test completes in <0.5s on my Mac.

It's a historical issue that the tests for this are elsewhere so it's good to add some sanity test for this legacy method.

test/jdk/java/io/FileDescriptor/Sync.java line 36:

> 34:  * @requires vm.continuations
> 35:  * @library /test/lib
> 36:  * @run main/othervm -XX:+UnlockExperimentalVMOptions -XX:-VMContinuations Sync

Running it with -XX:-VMContinuations is probably overkill here as it will be the same as the default case.

test/jdk/java/io/FileDescriptor/Sync.java line 83:

> 81: 
> 82:         File tmpFile = File.createTempFile("FileDescriptorSync2", "tmp");
> 83:         testWith(tmpFile);

This will leave the file on the tmp file system. A try-finally to delete it might be best as we've had issues with temp file accumulating in long test runs.

-------------

PR Review: https://git.openjdk.org/jdk/pull/15231#pullrequestreview-1573793329
PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291415629
PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291417472


More information about the core-libs-dev mailing list