On Sun, 5 Feb 2023 19:04:24 GMT, Alan Bateman <al...@openjdk.org> wrote:
> Are you planning to include benchmark results to go with this change? > > It should be okay to change readFullyAt to use pread, and ReadFile with an OV > with the position. The latter has a side effect that it changes the file > pointer but it should be okay in the zip code. One thing that the changes > highlight is that this native file is begging to be refactoring into platform > specific code (this isn't the PR to do that). > > In passing, the JNI code use 4 space indent, not 2. > > The O_CLOEXEC part is probably okay but if you look at the > Runtime.exec/Process implementation you'll see that it isn't really because > that code closes all file descriptors on exec. I can add benchmarks later this week. I'll also fix the indentation. On `O_CLOEXEC`, closing after exec isn't really foolproof. Additionally that requires always using the Java method of subprocess spawning, which isn't always the case. `O_CLOEXEC` covers all use cases and is generally the correct thing to do unless a file descriptor is explicitly intended to be shared. ------------- PR: https://git.openjdk.org/jdk/pull/12417
