On Mon, 15 Dec 2025 11:49:19 GMT, Jonas Norlinder <[email protected]> 
wrote:

> # Background
> 
> When Java applications uses APIs like java.io.FileOutputStream it will hook 
> into native implementations in e.g. io_util_md.c for Unix/Linux. Java does 
> not allow reading a directory and the implementation reflect this fact. For 
> Unix there are three access modes O_RDONLY, O_WRONLY, O_RDWR. Moreover, on 
> Unix it is possible to read a directory and an extra check has been added in 
> the code to ensure that the user is trying to read a file (with O_RDONLY) and 
> not a directory. This extra check results in an additional syscall.
> 
> This check is actually redundant in case user are using access mode O_WRONLY 
> or O_RDWR. If one is trying to call open on a directory with these modes the 
> specification in Unix and Linux specifies that EISDIR shall be returned. For 
> the case of Unix standard it has been part of the standard at least since 
> 1997 (https://pubs.opengroup.org/onlinepubs/007908799/xsh/open.html) and 
> Linux since at least 2004 (see v 2.0 
> https://www.kernel.org/pub/linux/docs/man-pages/Archive/ ) to return error if 
> user is trying to write to an directory. In OpenJDK we also include AIX and 
> they are certified to follow the Unix standard 
> (https://www.opengroup.org/openbrand/register/ibm.htm). I believe that it is 
> therefore safe to assume that this is a well implemented aspect of the Unix 
> standard by now and that this technical debt can be eliminated (assuming that 
> this check was indeed needed at some point).
> 
> # Performance Improvements
> 
> A stress-test that opens a huge amount of files to trigger a syscall storm 
> reveals that a removal of this redundant syscall may also improve performance 
> during high load:
> 
> 
> JDK 27 baseline
> Benchmark            Mode   Cnt     Score    Error   Units
> FileWriteStress.test sample 8438452 3722.451 ± 2.402 ns/op
> 
> JDK 27 patched
> Benchmark            Mode   Cnt     Score    Error   Units
> FileWriteStress.test sample 4952304 3191.912 ± 4.011 ns/op
> 
> 
> ~17% performance boost.

The new APIs doesn't have this restriction but we are stuck with it for 
FileInputStream or when opening a file for read with RandomAccessFile. At some 
point we will replace the implementation of all 3 as they can be implemented on 
top of the new APIs.

src/java.base/unix/native/libjava/io_util_md.c line 82:

> 80:     // not a read access mode then the Unix standard
> 81:     // guarantees to have failed with EISDIR
> 82:     if (fd == -1 || ((oflag & O_ACCMODE) != O_RDONLY) != 0) {

Can you drop the "Fast-path" comment as it is confusing here. It just needs to 
say that there is no need to check if the file is a directory when opened for 
write.

src/java.base/unix/native/libjava/io_util_md.c line 86:

> 84:     }
> 85: 
> 86:     // Slow-path, while Unix allow for directories

We can replace this with a clearer comment to say that FileInputStream is 
specified to throw if the file is a directory?

test/micro/org/openjdk/bench/java/io/FileWriteStress.java line 26:

> 24: 
> 25: import java.io.File;
> 26: import java.io.FileNotFoundException;

Used?

test/micro/org/openjdk/bench/java/io/FileWriteStress.java line 44:

> 42: 
> 43: @Fork(value = 10)
> 44: public class FileWriteStress {

You might want to think about a better name for this as it's not really a write 
stress tests. It could test both FOS and RAF as the changes in the PR change 
both.

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

PR Comment: https://git.openjdk.org/jdk/pull/28823#issuecomment-3710409761
PR Review Comment: https://git.openjdk.org/jdk/pull/28823#discussion_r2661474563
PR Review Comment: https://git.openjdk.org/jdk/pull/28823#discussion_r2661479745
PR Review Comment: https://git.openjdk.org/jdk/pull/28823#discussion_r2634936021
PR Review Comment: https://git.openjdk.org/jdk/pull/28823#discussion_r2634935307

Reply via email to