On 2 nov 2012, at 21:12, Mandy Chung <mandy.ch...@oracle.com> wrote:
> Hi Staffan, > > On 11/2/2012 11:36 AM, 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/ > > This looks okay to me. > > Minor comments: > sun/misc/IoTrace.java L36: should it be volatile in case another thread is > setting to another listener? Yes, that could be a good idea. > The *Begin() methods return a "handle" that will be passed to the *End() > methods. Have you considered to define a type for it rather than Object? Something like an empty interface, just to signal the intent? > Do you have any performance measurement result that you can share? I don't yet have any specific numbers - I'll try to get some. The testing I have done indicates that the overhead is negligible. But it would be good to confirm this. > As for the unit tests, I know you have tests written for the feature that > implements the listeners. I wonder if it's worth adding some sanity tests > along with this change? Yes, that is probably a good addition, too. Thanks! /Staffan > > Mandy > >> Feedback is most welcome, >> /Staffan >>