Preliminary review: Adding tracing of I/O calls

[Alan Bateman]

On 02/11/2012 18:36, Staffan Larsen wrote:
> This is a preliminary review request for adding an API for tracing I/O calls. For now, this is an empty infrastructure intended to enable diagnosing/tracing of i/o calls. A user of the API can register a listener and get callbacks for read and write operations on sockets and files. It does not (yet) cover asynchronous i/o calls. When not used, the implementation should add a minimum of overhead. To provide useful information to the user, FileInputStream, FileOutputStream and RandomAccessFile have been modified to keep track of the path they operate on (when available).
>
> Webrev: http://cr.openjdk.java.net/~sla/iotrace/webrev.00/
>
> Feedback is most welcome,
> /Staffan
>
Part of me is a somewhat disappointed to see hooks going into the I/O 
paths, but I understand why it needs to be done. I see the mails that 
getting some performance figures on the overhead and that would be good 
to have.

I think IoTrace/IoTraceListener needs to have some javadoc. I suggest 
this because it's impossible to implement IoTraceListener (even in the 
JDK) without some documentation as to how it is used. I see there is a 
block comment in IoTraceListener but there are other things that an 
implementer needs to know, particularly as to possible behavior when 
there is an I/O error. Looking at the usages then it looks like in *End 
might not be called, in other cases it can be called with 0 when there 
is an error.

I also see mails about IoTrace.listener needing to be volatile, that 
would be good to do.

I also think it would be useful to have a basic sanity test of the 
hooks. I realize there will be product usage elsewhere but we should 
have something simple in the jdk repository too.

In FileInputStream, FileOutputStream and FileChannelImpl then the 
comment on path is that it is null "if there is no file" but I think 
this should say that the stream (or parent stream in the case of a file 
channel) is created with a file descriptor.

In SocketChannelImpl then if the channel is configured non-blocking 
socketReadEnd will be called without a socketReadBegin. If this is 
intended then it will be something for the IoTrace/IoTraceListener javadoc.

I realize the focus is blocking I/O for now but one thing to know is 
that timed read operations on socket adapters (the socket obtained by 
calling SocketChannel's socket method) configures the socket channel to 
be non-blocking so this means that this I/O will not be observed by the 
IoTraceListener.

In both FileChannelImpl and SocketChannelImpl then normalize will now be 
called twice on each return status, I don't expect this will be 
observable from a performance perspective.

SolarisUserDefinedFileAttributeView - this is I/O on a file's extended 
attributes rather than its contents, it might not interesting to the 
IoTraceListener.

UnixChannelFactory L137, this line is getting long, might be better to 
go into a second line.

WindowsChannelFactory - one thing to be aware of is newFileChannel will 
also be called when open named streams so pathForWindows won't be the 
name of a file that you see on the file system. I don't know if this is 
interesting here or not, it should be rare.

That's all I have.

-Alan.