Preliminary review: Adding tracing of I/O calls (original) (raw)

Staffan Larsen staffan.larsen at oracle.com
Sun Nov 4 12:50:25 UTC 2012

Thanks Alan. Some comments inline.

On 2 nov 2012, at 23:21, Alan Bateman <Alan.Bateman at oracle.com> wrote:

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.

Yes, it worries me a bit, too. I'll see what the microbenchmarks say.

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.

Wil fix.

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.

Will fix.

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.

Yes.

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.

This is an error. The socketReadEnd() call should be guarded the same way socketReadBegin() is.

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.

I need some help to understand which call path you are referring to here. I see SocketChannelImpl.socket() returning a SocketAdapter, but I don't see how/if this is Socket is configured to be non-blocking.

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.

Yes, I would be surprised if this was observable. I could rework the code so it's only called once.

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

Hard to tell if it will be interesting or not. If there is i/o related to the file, it is probably of interest when diagnosing problems. I also don't know how to exclude this information in a simple way.

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

Will fix.

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.

Sounds like the name of the stream would also be of interest to anyone tracing/diagnosing.

/Staffan

That's all I have.

-Alan.

More information about the core-libs-dev mailing list