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

[Keith McGuigan]

On Dec 16, 2010, at 5:12 PM, Mandy Chung wrote:

> On 12/15/10 20:37, Keith McGuigan wrote:
>
> Checking the JVMTI version would help when you bump the minor number  
> in the JVMTI version from 1.1.x to 1.2.  If a new JVMTI function is  
> added in JDK 7 in the future, it will bump the micro number but not  
> the minor number.   I believe the isVersionGte12x() method will not  
> work for that case.  The jdwp agent would get very complicated if it  
> has to handle the micro edition to the JVMTI during development.

Right, but if we update the micro number to add some new function,  
than the user of that function will just have to check to make sure  
the micro-number is greater-than-or-equal-to the expected value.  It's  
not isVersionGte12x(), but something similar would suffice  
(isVersionGte1_2_75() or whatever).  .  It wouldn't really complicate  
things much, and if it becomes widely used we could add some generic  
version checking infrastructure (like JDK_Version in Hotspot).

> Or you still have to live with the synchronization putback schdedule  
> - the jdk side has to wait until the hotspot change makes it into a  
> promoted build; or another alternative is to check in a temporary  
> check of the micro number  and remove that check in a later promoted  
> build.

Yuck and double yuck!  I especially don't want create new CRs for  
follow-up work since each changeset needs a separate CR.

> I understand the benefit of allowing an older VM to run on JDK 7 so  
> that we can easily verify a hotspot fix and identify hotspot  
> regression.  But I'm not sure if the jdwp agent should support that  
> and wonder if it's useful in practice.  The use case requiring the  
> jdwp agent to support an older JVMTI version is when a developer  
> wants to debug a Java application on JDK 7 with a JDK 6 VM.   I  
> would imagine that this is rarely needed.  Is there other use case  
> that suggests this jvmti version check worth doing?

Not a use case that exists right now, but I was imagining someone  
wanting to run the JDK with a different underlying JVM altogether (not  
Hotspot).

> The current version check isn't too bad but I'm concerned down the  
> road when more enhancements come in, it will complicate the JDWP  
> implementation and various check of different versions are added in  
> the implementation.

I understand the concern but the checks will occur only where the new  
functions are used.  And if there's some version checking  
infrastructure those checks won't be that bad.

> Would you consider keeping the jdwp agent in JDK 7 to require JVMTI  
> version >= 1.2 (i.e. works with JDK 7 or newer VM)?

As you can probably tell, I'd rather not :)  But if everyone agrees  
that dynamically checking the version is abhorrent I will defer to  
you.  You guys are the experts, I'm new at this code.
>
> When we do our engineering build of both jdk and hotspot  
> repositories (or nightly testing), does the hotspot VM have an  
> internal version string rather than 20.0-b03?  If that's the case,  
> does that mean the test will just pass even if the hotspot supports  
> JVMTI 1.2?  In that case, you would want the test to actually do the  
> work.

I don't think that the format of that part of the string changes, but  
if it does, then when the parsing fails the default is that the test  
is "enabled".  It will only be neutered when the version parsing  
succeeds and the JVM version is less than what is expected.

> FYI.  sun.misc.Version provides several internal methods to return  
> the version number and build number of the VM that you can use in  
> these tests to replace the regex.
>

Ok I'll take a look at at those.  Can I access internal classes like  
that from a jtreg test?  I wasn't sure I could.

--
- Keith
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20101216/5c670d92/attachment-0001.html