On 12/15/10 20:37, Keith McGuigan wrote:

On Dec 15, 2010, at 4:44 AM, Alan Bateman wrote:

Keith McGuigan wrote:

:
Yeah I missed that check too. I'm very much in favor of being able to use an older (or newer) JVM in the JDK7 image. Triage is one important reason but another is the non-synchronous putback schedules of the jdk and hotspot workspaces. There will be a period of QA time when the JDK uses an older VM (or vice-versa) just because of the way our process works.

I think the jdk code should probe the JVMTI version and use whatever is available. I'm in favor of modifying the code in debugInit.c to verify only 1.1 and then dynamically probe before using any 1.2 features (and so on in the future).

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. 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.

In the past I don't think we've done this. Instead we've usually just kept back the jdk changes until the hotspot changes made it into a promoted build. One other thing is the regression test. The debugger tests are in jdk/test/com/sun/jdi and it is important to add a new test, or add to an existing test. If you are pushing the jdk changes before the VM changes have made it into a promoted build then I guess you'll need to hold off pushing the test (or add it with the @ignore tag, removing it later via a different bugID).

-Alan

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.

This check looks okay but it doesn't help when new APIs are added in the same minor version. It's not a problem for this fix to address. However, I wonder if the jdwp agent has to support older VM (older JVMTI version).

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?

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.

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)?

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.


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.

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.

Mandy

Mandy

Reply via email to