Thank you Roger!
Yes, you're right.
Please find the updated webrev here:
http://cr.openjdk.java.net/~igerasim/8023173/2/webrev/
Sincerely yours,
Ivan
On 20.10.2014 17:20, roger riggs wrote:
Hi Ivan,
This webrev appears removes the ability to interpose on various method
using byte-code injection. For example, FileOutputStream.write(byte).
Do *not *replace delete the Java method calls that call native.
It looks like an optimization but disables some functions that allow
monitoring
of I/O activities such as JFR.
Thanks, Roger
On 10/20/2014 8:34 AM, Ivan Gerasimov wrote:
Thank you Alan for the review!
On 17.10.2014 13:00, Alan Bateman wrote:
On 06/10/2014 11:41, Ivan Gerasimov wrote:
Hello everybody!
The append mode is emulated on Windows, therefore we have to keep a
flag indicating that.
With the current implementation, the FileDescriptor does not know
if the file were opened with O_APPEND, and the flag is maintained
at the upper level.
This can cause inconsistency, when the FileDescriptor is reused to
construct a new FileOutputStream, as there is no information
available about the append/non-append mode.
Even though the solution is quite straight-forward: moving the flag
from FileOutputStream, FileDispatcherImpl and FileChannelImpl to
the lower level of FileDescriptor, it touches 20 files across the
source-code base.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8023173
WEBREV: http://cr.openjdk.java.net/~igerasim/8023173/0/webrev/
With the fix, all the io/nio tests, including the new one, pass on
all available platforms.
Would you please help review this fix?
Martin prodded me a few times but I was busy with other things and
only getting to this now.
At a high level then moving the append down to FileDescriptor make
sense but I have a few concerns.
On Unix systems then it looks like getAppend is calling into the
ioctl to get the file descriptor flags.
If I read it correctly then it creates several problems for the
usage in FileChannelImpl.position.
I thought it might look like a data duplication, if we store the
append flag both in FileChannelImpl and FileDescriptor.
Though, we surely can retrieve the append flag only once and cache it.
I updated the webrev with this change.
I just wondering if you consider just leaving the append flag there
and just retrieve the value once in the open method.
I'm also wondering about the handleWrite implementation on Windows
which changes to use additional JNI to retrieve the value of the
append flag each time. We should try to avoid this (we want the
native methods to be as simple as possible so that we can replace
them in the future) so there may be an argument for passing that
down as per the current implementation.
We would still need to retrieve the flag from native code.
For example, the platform specific handleWrite() is called from the
native FileOutputStream.writeBytes(), which is called from
FileOutputStream.writt(), which is common for all the platforms.
Thus, we should either retrieve the append flag from the Java code
for any platform, and ignore it later on Unix, or read the flag in
the native Windows-only code.
Alternatively, we could separate implementations of
FileOutputStream.java for different platforms, but that would
complicate everything.
Hopefully, in the future we could find a way to use FILE_APPEND_DATA,
so FileDescriptor.append can be removed altogether with that
corresponding JNI code.
Here's the updated webrev:
http://cr.openjdk.java.net/~igerasim/8023173/1/webrev/
I've also included a few typo fixes left after JDK-8059840.
Sincerely yours,
Ivan
