RFR: 8003322: Add instrumentation points for tracing of I/O calls (original) (raw)

Alan Bateman Alan.Bateman at oracle.com
Wed Nov 14 12:50:16 UTC 2012

On 13/11/2012 10:16, Staffan Larsen wrote:

This is a request for review for adding tracing to 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 callback 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/8003322/webrev.00/ Thanks for the update. Do you have any updated performance data to share (just to confirm that the updated implementation doesn't have any real impact)?

Anyway, I took a pass over the new webrev.

I'm not sure that passing a value of 0 for errors to xxEnd is the best approach, particularly if this is ever extended to non-blocking I/O. Also I think there are a few inconsistencies with respect to EOF -- eg: in FileInputStream then read() will call the hook with 0 at EOF whereas the other read methods will call the hook with -1 at EOF. In FileChannelImpl then some places use normalize, some not.

I guess the main question is whether the agent needs to distinguish I/O errors from EOF and 0 bytes (the latter is assuming this may be extended to non-blocking I/O). It may be that you need to use -2 or anything < -1 to distinguish all cases.

Minor nit but there is a bit of inconsistency with the variables names, usage of "v" in RandomAccessFile for example whereas FIS/FOS have bytesRead and bytesWritten.

Thanks for adding javadoc to IoTrace. One suggestion is to include a big warning that the hooks may be called while holding low-level locks in the implementation and so great care must be taken, any synchronization or interaction with other threads could easily deadlock the VM.

I skimmed over the tests (not a detailed review) and they look reasonable. You might need to check the copyright headers as it looks like at least one of the tests has the GPL+Classpath exception whereas we normally use just the GPL header on tests. Also good to ensure that there is @bug tag on the tests to link it to 8003322. In ioTraceTest.sh I see "cd ${PWD}" that I didn't quite get.

Do you think these tests will be reliable when running without an images build (meaning raw classes files on the system)? Just wondering if expectFileRead might fail due to I/O caused by class loading.

That's all I have on the detailed review. As you mentioned there is still have the substantive issue as to whether it's open season on sun.misc.IoTrace*. Ignoring Unsafe (we know this needs to be standardized or a standard alternative introduced), then nothing outside of the JDK should be using sun.* classes directly.

-Alan

More information about the core-libs-dev mailing list