request for review: 6436034: Instance filter doesn't filter event if it occurs in native method

[Daniel D. Daugherty]

On 12/15/2010 9:37 PM, Keith McGuigan wrote:
> Ok, here's a new webrev: 
> http://cr.openjdk.java.net/~kamg/6436034/webrev.01/
>
> I added a regression test and modified the code in debugInit.cpp to 
> explicitly allow running with JVMTI 1.1 if that's what the JVM 
> supports.  The regression test is setup to pass when run with a JVM 
> less than version 20 build 05 (where the new JVMTI function is added), 
> so it should run and pass in either situation and won't require a 
> later update.
General comments:

- To reiterate, I like the idea of being able to drop an older VM
  into JDK7 in order to triage failures. This is particularly
  important as we try to spin up the Serviceability Team again
  and are working through the enormous bug backlog.

- Since the JDK7 project plan has been morphed into the JDK7 and
  JDK8 plans, I don't think we have to worry about any more JVM/TI
  API addition in the JDK7 timeframe.

- This JDK code will also be likely back ported into OpenJDK6 so
  that release train will also have the same "drop in a VM"
  capability.

- don't forget to update the copyright years in the various files

- block comment style is different than existing code. Should be:
  /*
   * ...
   */


src/share/back/debugInit.c
    Yes, keep the JVM/TI version 0.x compatibility check. It doesn't
    really cost much and you never know what crazy things we'll try
    to do when triaging.

    No content comments.

src/share/back/eventFilter.c
    Don't really need the 'else' on line 292.

src/share/javavm/export/jvmti.h
    Any idea why there is a new blank line at 2534? Will jcheck
    complain about that?

test/com/sun/jdi/NativeInstanceFilter.java
    Didn't review closely since I think you'll be tweaking the test.

test/com/sun/jdi/NativeInstanceFilterTarg.java
    Didn't review closely since I think you'll be tweaking the test.