RFR:8153978:New test to verify the modules info as returned by the JVMTI
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Thu Jul 21 15:39:19 UTC 2016
On 7/21/16 08:29, Christian Tornqvist wrote:
>
> Hi Alexander,
>
> >The JVMTI always reports 3 unnamed modules: the boot module, the system
> module and the application module.
>
> >The Java API does not report any unnamed modules.
>
> I’ll leave this up to you if this is something that we need to verify
> or not, the code for doing this is also overcomplicated and can be
> reduced to a simple assertGTE.
>
The rule is that there is one unnamed module per a class loader.
The options are: to test this rule or remove the check.
For simplicity is better to remove this check as unclear.
Thanks,
Serguei
> >This should be doable without using JAR's and custom loaders by using
> Layer.defineModules(), see the examples in
> jdk/test/java/lang/reflect/Layer/BasicLayerTest.java
>
> >The test has been written from the user perspective. The user loads a new
> module in the form of jar using the ModuleLoader.loadModule() API.
> Then the test checks that JVMTI does return the info about that loaded
> module.
>
> >Probably, defining the module using Layer.defineModules would not be the same
> as loading the module using ModuleLoader.loadModule(), since the JVMTI
> GetAllModules() returns the info about all the currently loaded modules.
>
> >As the JVMTI spec says: "GetAllModules: Return an array of all modules
> loaded in the virtual machine.", it does not mention defining modules.
>
> There are several ways to get modules loaded/defined, the
> Layer.defineModules is part of the official Java API and is one of
> them. It doesn’t matter to JVMTI if they come from JAR files on disk
> or if they’re defined using a Java API, so I suggest you go with
> Layer.defineModules.
>
> Thanks,
>
> Christian
>
> *From:*Alexander Kulyakhtin [mailto:alexander.kulyakhtin at oracle.com]
> *Sent:* Thursday, July 21, 2016 10:04 AM
> *To:* Serguei Vladimirovich Spitsyn <serguei.spitsyn at oracle.com>;
> christian.tornqvist at oracle.com
> *Cc:* serviceability-dev at openjdk.java.net
> *Subject:* Re: RFR:8153978:New test to verify the modules info as
> returned by the JVMTI
>
> Christian,
>
> Thank you very much for your comments. I have some concerns about the
> proposed changes:
>
> @45 & @67
>
> Why is this check needed? Why are there least 3 unnamed modules?
> The JVMTI always reports 3 unnamed modules: the boot module, the
> system module and the application module.
> The Java API does not report any unnamed modules.
>
> @54
>
> This should be doable without using JAR's and custom loaders by using
> Layer.defineModules(), see the examples in
> jdk/test/java/lang/reflect/Layer/BasicLayerTest.java
> The test has been written from the user perspective. The user loads a
> new module in the form of jar using the ModuleLoader.loadModule() API.
> Then the test checks that JVMTI does return the info about that loaded
> module.
> Probably, defining the module using Layer.defineModules would not be
> the same as loading the module using ModuleLoader.loadModule(), since
> the JVMTI GetAllModules() returns the info about all the currently
> loaded modules.
> As the JVMTI spec says: "GetAllModules: Return an array of all modules
> loaded in the virtual machine.", it does not mention defining modules.
>
> Could you, please, clarify these points for me so I fix the test
> appropriately?
>
> Best regards,
> Alexander
>
>
>
>
>
> ----- Original Message -----
> From: christian.tornqvist at oracle.com
> <mailto:christian.tornqvist at oracle.com>
> To: alexander.kulyakhtin at oracle.com
> <mailto:alexander.kulyakhtin at oracle.com>,
> serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>
> Sent: Wednesday, July 20, 2016 8:11:14 PM GMT +03:00 Iraq
> Subject: RE: RFR:8153978:New test to verify the modules info as
> returned by the JVMTI
>
>
> Hi Alexander,
>
> This test is unnecessarily complicated, it could be simplified a lot.
>
> JvmtiGetAllModulesTest.java
>
> Move getModulesNative() into JvmtiGetAllModulesTest.java and have it
> return a Set<Module> to be able to use equals later
>
> @27 * @compile JvmtiGetAllModulesTest.java
>
> No need for this, jtreg will compile it for you
>
> @45 & @67
>
> Why is this check needed? Why are there least 3 unnamed modules?
>
> @50
>
> Change this to: assertTrue(Layer.boot().equals(getModulesNative()));
>
> @54
>
> This should be doable without using JAR's and custom loaders by using
> Layer.defineModules(), see the examples in
> jdk/test/java/lang/reflect/Layer/BasicLayerTest.java
>
> @65
>
> Change this to an assertTrue using the layer containing the new
> module, similar to the change @50
>
> @73
>
> No need for this method
>
> @81
>
> Change this method to use the Layer.defineModules() method to define a
> module instead, this eliminates the need for external JAR's
>
> @98
>
> No need for this method
>
> If you use Layer.defineModules(), the following files can be removed:
>
> JarBuilder.java
>
> JavaModulesInfo.java
>
> JvmtiModulesInfo.java
>
> ModuleLoader.java
>
> ModulesInfo.java
>
> module-info.java
>
> Thanks,
>
> Christian
>
> *From:*serviceability-dev
> [mailto:serviceability-dev-bounces at openjdk.java.net] *On Behalf Of
> *serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>
> *Sent:* Monday, May 2, 2016 6:06 PM
> *To:* Alexander Kulyakhtin <alexander.kulyakhtin at oracle.com
> <mailto:alexander.kulyakhtin at oracle.com>>; Serviceability-Dev
> <serviceability-dev at openjdk.java.net
> <mailto:serviceability-dev at openjdk.java.net>>
> *Subject:* Re: RFR:8153978:New test to verify the modules info as
> returned by the JVMTI
>
> Hi Alexander,
>
>
> Could you, fix a couple of minor issues?
>
> test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java
>
> 58 for(Module mod : my.modules()) {
> 59 if(!jvmtiModules.contains(mod)) {
>
> A space is missed after the 'for' and 'if' keywords.
>
>
> test/serviceability/jvmti/GetModulesInfo/ModulesInfo.java.
>
> 31 boolean compareExcludingUnnamed(ModulesInfo other) {
>
> I'd suggest to call it compareNamed.
>
>
> Otherwise, the new test looks great.
> Thanks a lot for taking care about it!
>
> Thanks,
> Serguei
>
>
>
> On 4/29/16 06:12, Alexander Kulyakhtin wrote:
>
> Hi,
>
>
>
> Could you, please, review these test-only changes (adding a new test).
>
>
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8153978 "New test to verify the modules info as returned by the JVMTI"
>
> Webrev:http://cr.openjdk.java.net/~akulyakh/8153978_01/
> <http://cr.openjdk.java.net/%7Eakulyakh/8153978_01/>
>
>
>
> The new test verifies that JVMTI returns the correct info about the modules loaded at the application startup.
>
> It also verifies that the returned info is consistent with the same info returned by the Java API.
>
> It then loads a new named module and checks the correctness of the JVMTI info again.
>
>
>
> Due to a tools issuehttps://bugs.openjdk.java.net/browse/CODETOOLS-7901662 the test can only be pushed in when the updated jtreg is released.
>
> The test passes fine with the nightly jtreg build, containing the CODETOOLS-7901662 fix.
>
>
>
> Best regards,
>
> Alexander
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160721/3ac20e93/attachment-0001.html>
More information about the serviceability-dev
mailing list