Staffan, OK!
-Dmitry On 2014-02-07 19:49, Staffan Larsen wrote: > > On 7 feb 2014, at 16:24, Dmitry Samersoff <dmitry.samers...@oracle.com> wrote: > >> Staffan, >> >> FileInputStream.java >> >> 55: It's better to initialize path with null. > > I agree with Chris here. The value should be explicitly initialized by all > constructors. > >> 134: It's better to assign name at one of first lines, in this case we >> will be able to retrieve file name ever if open fails for some reason. > > This is the constructor. If anything fails it will throw and exception, and > there won’t be an object to look at. > >> 171: It's not necessary > > All constructors must initialize the value. > > Thanks, > /Staffan > >> >> (the same is applicable to other files) >> >> I'm a bit scared changing signature of public methods of FileChannelImpl >> but if Alan says it's OK - lets go with it. >> >> -Dmitry >> >> >> On 2014-02-07 16: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 >>> >>> Thanks, /Staffan >>> >> >> >> -- >> Dmitry Samersoff >> Oracle Java development team, Saint Petersburg, Russia >> * I would love to change the world, but they won't give me the sources. > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
