RFR - Changes to address CCC 8014135: Support for statically linked agents

[serguei.spitsyn at oracle.com]

Hi Bill,

Sorry for the big delay.


http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.01/


  src/share/classes/com/sun/tools/attach/VirtualMachine.java:


I'm suggesting to use the reference 
*<code>Agent_OnAttach[_L]</code>**||* even more consistently.
(If, in some cases, you prefer the longer form to underline the 
difference between
  dynamically and statically linked libraries then feel free to leave it 
as it is.)

It would simplify the following fragments:

304      * It then causes the target VM to invoke the <code>Agent_OnAttach</code> function
305      * or, for a statically linked agent named 'L', the <code>Agent_OnAttach_L</code> function

   ==>

304      * It then causes the target VM to invoke the <code>Agent_OnAttach[_L]</code> function


409      * It then causes the target VM to invoke the <code>Agent_OnAttach</code>
410      * function or, for a statically linked agent named 'L', the
411      * <code>Agent_OnAttach_L</code> function as specified in the

   ==>

  409      * It then causes the target VM to invoke the <code>Agent_OnAttach[_L]</code>
  410      * function as specified in the



the following 4 identical fragments:

  341      *          If the <code>Agent_OnAttach</code> function returns an error
  342      *          or, for a statically linked agent named 'L', if the
  343      *          <code>Agent_OnAttach_L</code> function returns
  344      *          an error.

  375      *          If the <code>Agent_OnAttach</code> function returns an error
  376      *          or, for a statically linked agent named 'L', if the
  377      *          <code>Agent_OnAttach_L</code> function returns
  378      *          an error.

  442      *          If the <code>Agent_OnAttach</code> function returns an error
  443      *          or, for a statically linked agent named 'L', if the
  444      *          <code>Agent_OnAttach_L</code> function returns an error

  475      *          If the <code>Agent_OnAttach</code> function returns an error
  476      *          or, for a statically linked agent named 'L', if the
  477      *          <code>Agent_OnAttach_L</code> function returns
  478      *          an error.

   ==>

  336      *          If the <code>Agent_OnAttach[_L]</code> function 
returns an error.




http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/


http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/jvmti.html
   src/share/vm/prims/jvmti.xml

  Lines 442-462:  many extra <p/>'s. The fragment does not look well.
  I'd suggest to remove most of them.
  Also, these lines are too long. Could you make them shorter, please?
  The same is applied to other long new lines in this file.

  Lines 490-491, 502-503, 505-506:
      The same sentence is repeated 3 times: "the agent library may be 
statically linked ..."

  490     Note that the agent library may be statically linked into the 
executable
491     in which case no actual loading takes place.

  501 <code>-agentpath:c:\myLibs\foo.dll=opt1,opt2</code> is specified, 
the VM will attempt to
  502         load the shared library <code>c:\myLibs\foo.dll</code>. As 
mentioned above, the agent library may be statically linked into the 
executable
  503         in which case no actual loading takes place

  505     Note that the agent library may be statically linked into the 
executable
  506     in which case no actual loading takes place.

  Lines 677-678: The dot is missed at the end of line 677:

  677     and enabled the event


  src/os/posix/vm/os_posix.cpp

    - no comments

  src/os/windows/vm/os_windows.cpp

    - no comments

  src/share/vm/prims/jvmtiExport.cpp

    - no comments

  src/share/vm/runtime/arguments.hpp

    - no comments

  src/share/vm/runtime/os.cpp

  Space is missed after the 'if':
    471     if(entryName != NULL) {

  Extra space after the '*':
   483   void * saveHandle;

  src/share/vm/runtime/os.hpp

    - no comments

  src/share/vm/runtime/thread.cpp

   The line has been removed:
   3866         break;


I'm still in process of reading the code.
Another pass is needed to make sure that nothing is missed.
But in general, the code quality is pretty good.

Thanks,
Serguei



On 7/25/13 10:47 AM, bill.pittore at oracle.com wrote:
> Still need an official reviewer for the hotspot changes for statically 
> linked agents.
>
> thanks,
> bill
>
>> These changes address bug 8014135 which adds support for statically 
>> linked agents in the VM. This is a followup to the recent JNI spec 
>> changes that addressed statically linked JNI libraries( 8005716).
>> The JEP for this change is the same JEP as the JNI changes:
>> http://openjdk.java.net/jeps/178
>>
>> Webrevs are here:
>>
>> http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.00/
>> http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/
>>
>> The changes to jvmti.xml can also be seen in the output file that I 
>> placed here:
>> http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/jvmti.html 
>>
>>
>> Thanks,
>> bill
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20130726/756a4f87/attachment-0001.html