RFR: JDK-8301621: libzip should use pread instead of lseek+read
Justin King
jcking at openjdk.org
Mon Feb 6 15:24:49 UTC 2023
On Sun, 5 Feb 2023 19:04:24 GMT, Alan Bateman <alanb at 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
More information about the core-libs-dev
mailing list