On 7 feb 2014, at 13:27, Alan Bateman <alan.bate...@oracle.com> wrote:
> On 07/02/2014 12:07, Staffan Larsen wrote: >> Instrumentation agents that want to instrument >> FileInputStream/FileOutputStream to see which files are being accessed do >> not currently have access to the file system path of the stream. This is >> because the path is never stored in the stream class, only the file >> descriptor is. (This is also true for RandomAccessFile and FileChannel). >> >> An agent could instrument the respective constructors to store the path. The >> problem is where to store it. To add a field, the instrumentation agent >> needs to change the schema of the class. This is not possible at runtime but >> can be done at class-loading time. However for a j.l.instrument agent these >> classes are already defined when the agent is first called. For a native >> JVMTI agent the problem becomes parsing and modifying byte codes in a native >> agent which is error prone and requires a lot of code to maintain. >> >> If instead the stream classes were modified to store a reference to the >> path, it would be readily available for agents at a minimum of cost to the >> libraries. This is what this patch does. FileInputStream, FileOutputStream, >> RandomAccessFile and FileChannelImpl are modified to record the path they >> operate on in a private field. There are no accessors added to retrieve the >> path - it is purely stored for instrumentation purposes. The path is >> intentionally not resolved to be an absolute path since that would >> potentially add unwanted overhead. If a stream is created from a file >> descriptor, no path will be stored. >> >> The overhead for this path will be keeping the path String alive for a >> longer period of time. I hope this will not cause any problems. >> >> A consumer of this feature will be Java Flight Recorder, but the >> implementation is usable by other agents as well. >> >> webrev: http://cr.openjdk.java.net/~sla/8033917/webrev.00/ >> bug: https://bugs.openjdk.java.net/browse/JDK-8033917 >> > I have reservations about doing this as hints of code making use of private > fields which isn't good. > > For the comments in FileInputStream and other then it might be best to keep > the line lengths consistent with the existing code if you can (it makes > future side-by-side reviews a bit easier too). I’ve updated the comments to have shorter lines and javadoc style comments. > In WindowsChannelFactory then you've re-order and expand imports. The > ordering of the import groups in this area has been Java SE, JDK-specific and > then finally the JDK-internal. It's not a big deal of course. Personally I > prefer the original static imports but I know some people don’t. I’ve reverted to the original order, and only expanded the non-static imports. (The danger of IDEs). Updated webrev here: http://cr.openjdk.java.net/~sla/8033917/webrev.01/ Thanks, /Staffan
