On Thu, 10 Aug 2023 15:45:12 GMT, Aleksey Shipilev <sh...@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
