[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI
From: Alexander Kulyakhtin <alexander.kulyakhtin () oracle ! com>
Date: 2016-07-22 13:05:16
Message-ID: 17851fa4-dbbb-4a12-98ca-eea7cf8aa4a1 () default
[Download RAW message or body]
Hi Christian,
Thank you very much for the review.
Best regards,
Alexander
----- Original Message -----
From: christian.tornqvist@oracle.com
To: alexander.kulyakhtin@oracle.com
Cc: serguei.spitsyn@oracle.com, serviceability-dev@openjdk.java.net
Sent: Friday, July 22, 2016 4:02:11 PM GMT +03:00 Iraq
Subject: RE: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
Hi Alexander,
This looks good, thanks for adding this J
Thanks,
Christian
From: Alexander Kulyakhtin [mailto:alexander.kulyakhtin@oracle.com]
Sent: Friday, July 22, 2016 8:57 AM
To: christian.tornqvist@oracle.com
Cc: serguei.spitsyn@oracle.com; serviceability-dev@openjdk.java.net
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
Hi Christian,
Yes, my intention was to check the equality of the returned data.
I've changed line 68 to:
Asserts.assertEquals(Layer.boot().modules(), getModulesJVMTI());
and removed line 90 since it's not needed.
As to the line 76, that is how Netbeans has formatted the code. I've changed it to \
have {} on the same line now.
Please, find the updated review at: \
http://cr.openjdk.java.net/~akulyakh/8153978_7/test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java.html \
Best regards,
Alexander
----- Original Message -----
From: christian.tornqvist@oracle.com
To: alexander.kulyakhtin@oracle.com , serguei.spitsyn@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Friday, July 22, 2016 3:50:09 PM GMT +03:00 Iraq
Subject: RE: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
Hi Alexander,
As Serguei said, the lines 68 and 90 doesn't check the results so they should either \
do that or be removed. If you remove those lines, you can remove the filtering out of \
unnamed modules in getModulesJVMTI as well since that will no longer be necessary.
Minor style thing, move the } on 76 to be on the same line as the opening {.
Thanks,
Christian
From: Alexander Kulyakhtin [ mailto:alexander.kulyakhtin@oracle.com ]
Sent: Friday, July 22, 2016 7:40 AM
To: serguei.spitsyn@oracle.com
Cc: christian.tornqvist@oracle.com ; serviceability-dev@openjdk.java.net
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
Hi Sergey,
Thank you very much for the review. I'm going to wait for any other findings today \
and, if everything is fine, will push the fix then.
Best regards,
Alexander
----- Original Message -----
From: serguei.spitsyn@oracle.com
To: alexander.kulyakhtin@oracle.com , christian.tornqvist@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Friday, July 22, 2016 11:31:13 AM GMT +03:00 Iraq
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
Alexander,
A thumbs up on the push.
I leave it up to you and Christian to tweak and polish the test if you think it is \
necessary.
Thank you a lot for working on it!
Thanks,
Serguei
On 7/21/16 14:05, serguei.spitsyn@oracle.com wrote:
On 7/21/16 11:35, serguei.spitsyn@oracle.com wrote:
Hi Alexander,
JvmtiGetAllModulesTest.java
It looks pretty good but it would be nice if there is any chance to simplify even \
more. However, I can't suggest anything at the moment.
67 // Verify that JVMTI reports exactly the same info as Java regarding the named \
modules 68 Layer.boot().equals(getModulesJVMTI()); 69
. . .
89 // Verify the consistency of the whole JVMTI report again
90 Layer.boot().equals(getModulesJVMTI()); 91
The above lines can be removed.
They even do not check the result of comparison.
Thanks,
Serguei
libJvmtiGetAllModulesTest.c
Unneeded indent for all lines.
Otherwise, it is good.
Thanks,
Serguei
On 7/21/16 10:14, Alexander Kulyakhtin wrote:
Christian, Sergey,
I've modified the test per your findings. Now it is one java file and one C file \
only.
Please, find the updated review at:
Webrev: http://cr.openjdk.java.net/~akulyakh/8153978_6/
Thank you very much for your help.
Best regards,
Alexander
----- Original Message -----
From: serguei.spitsyn@oracle.com
To: christian.tornqvist@oracle.com , alexander.kulyakhtin@oracle.com
Cc: serviceability-dev@openjdk.java.net
Sent: Thursday, July 21, 2016 6:39:21 PM GMT +03:00 Iraq
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
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@oracle.com ]
Sent: Thursday, July 21, 2016 10:04 AM
To: Serguei Vladimirovich Spitsyn <serguei.spitsyn@oracle.com> ; \
christian.tornqvist@oracle.com
Cc: serviceability-dev@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@oracle.com
To: alexander.kulyakhtin@oracle.com , serviceability-dev@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@openjdk.java.net ] On \
Behalf Of serguei.spitsyn@oracle.com
Sent: Monday, May 2, 2016 6:06 PM
To: Alexander Kulyakhtin < alexander.kulyakhtin@oracle.com >; Serviceability-Dev < \
serviceability-dev@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/ \
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 issue \
https://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
[Attachment #3 (text/html)]
<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div \
style='font-family: Times New Roman; font-size: 12pt; color: #000000'>Hi \
Christian,<br><br>Thank you very much for the review. <br><br>Best \
regards,<br>Alexander<br><br>----- Original Message -----<br>From: \
christian.tornqvist@oracle.com<br>To: alexander.kulyakhtin@oracle.com<br>Cc: \
serguei.spitsyn@oracle.com, serviceability-dev@openjdk.java.net<br>Sent: Friday, July \
22, 2016 4:02:11 PM GMT +03:00 Iraq<br>Subject: RE: RFR:8153978:New test to verify \
the modules info as returned by the JVMTI<br><br><style><!-- /* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";
color:black;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;
color:black;}
span.EmailStyle21
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle22
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle23
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle24
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle25
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle26
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle27
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle28
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--><div lang="EN-US"><div class="WordSection1"><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Hi \
Alexander,</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">This \
looks good, thanks for adding this </span><span \
style="font-size:11.0pt;font-family:Wingdings;color:windowtext">J</span><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"></span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Thanks,</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Christian</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><div><div \
style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"><p \
class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">From:</span></b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> \
Alexander Kulyakhtin [mailto:alexander.kulyakhtin@oracle.com] <br><b>Sent:</b> \
Friday, July 22, 2016 8:57 AM<br><b>To:</b> \
christian.tornqvist@oracle.com<br><b>Cc:</b> serguei.spitsyn@oracle.com; \
serviceability-dev@openjdk.java.net<br><b>Subject:</b> Re: RFR:8153978:New test to \
verify the modules info as returned by the JVMTI</span></p></div></div><p \
class="MsoNormal"> </p><div><p class="MsoNormal">Hi Christian,<br><br>Yes, my \
intention was to check the equality of the returned data.<br><br>I've changed line 68 \
to:<br><br>Asserts.assertEquals(Layer.boot().modules(), \
getModulesJVMTI());<br><br>and removed line 90 since it's not needed.<br><br>As to \
the line 76, that is how Netbeans has formatted the code. I've changed it to \
have {} on the same line now.<br><br>Please, find the updated review at: <a \
href="http://cr.openjdk.java.net/~akulyakh/8153978_7/test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java.html" \
target="_blank">http://cr.openjdk.java.net/~akulyakh/8153978_7/test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java.html</a><br><br>Best \
regards,<br>Alexander<br><br><br><br>----- Original Message -----<br>From: <a \
href="mailto:christian.tornqvist@oracle.com" \
target="_blank">christian.tornqvist@oracle.com</a><br>To: <a \
href="mailto:alexander.kulyakhtin@oracle.com" \
target="_blank">alexander.kulyakhtin@oracle.com</a>, <a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a><br>Cc: <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br>Sent: Friday, July 22, \
2016 3:50:09 PM GMT +03:00 Iraq<br>Subject: RE: RFR:8153978:New test to verify the \
modules info as returned by the JVMTI<br><br><br></p><div><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Hi \
Alexander,</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">As \
Serguei said, the lines 68 and 90 doesn't check the results so they should either do \
that or be removed. If you remove those lines, you can remove the filtering out of \
unnamed modules in getModulesJVMTI as well since that will no longer be \
necessary.</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Minor \
style thing, move the } on 76 to be on the same line as the opening {. </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Thanks,</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Christian</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><div><div \
style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"><p \
class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">From:</span></b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> \
Alexander Kulyakhtin [<a href="mailto:alexander.kulyakhtin@oracle.com" \
target="_blank">mailto:alexander.kulyakhtin@oracle.com</a>] <br><b>Sent:</b> Friday, \
July 22, 2016 7:40 AM<br><b>To:</b> <a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a><br><b>Cc:</b> <a \
href="mailto:christian.tornqvist@oracle.com" \
target="_blank">christian.tornqvist@oracle.com</a>; <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br><b>Subject:</b> Re: \
RFR:8153978:New test to verify the modules info as returned by the \
JVMTI</span></p></div></div><p class="MsoNormal"> </p><div><p class="MsoNormal" \
style="margin-bottom:12.0pt">Hi Sergey,<br><br>Thank you very much for the \
review. I'm going to wait for any other findings today and, if everything is \
fine, will push the fix then.<br><br>Best regards,<br>Alexander<br><br>----- Original \
Message -----<br>From: <a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a><br>To: <a \
href="mailto:alexander.kulyakhtin@oracle.com" \
target="_blank">alexander.kulyakhtin@oracle.com</a>, <a \
href="mailto:christian.tornqvist@oracle.com" \
target="_blank">christian.tornqvist@oracle.com</a><br>Cc: <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br>Sent: Friday, July 22, \
2016 11:31:13 AM GMT +03:00 Iraq<br>Subject: Re: RFR:8153978:New test to verify the \
modules info as returned by the JVMTI</p><div><div><p \
class="MsoNormal">Alexander,<br><br>A thumbs up on the push.<br>I leave it up to you \
and Christian to tweak and polish the test if you think it is necessary.<br><br>Thank \
you a lot for working on it!<br><br>Thanks,<br>Serguei<br><br><br>On 7/21/16 14:05, \
<a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a> wrote:</p></div><blockquote \
style="margin-top:5.0pt;margin-bottom:5.0pt"><div><p class="MsoNormal">On 7/21/16 \
11:35, <a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a> wrote:</p></div><blockquote \
style="margin-top:5.0pt;margin-bottom:5.0pt"><div><p class="MsoNormal">Hi \
Alexander,<br><br>JvmtiGetAllModulesTest.java<br><br> It looks pretty good but \
it would be nice if there is any chance to simplify even more.<br> However, I \
can't suggest anything at the moment.</p></div></blockquote><p class="MsoNormal" \
style="margin-bottom:12.0pt"><br> 67 // Verify that JVMTI reports exactly the \
same info as Java regarding the named modules<br> 68 \
Layer.boot().equals(getModulesJVMTI()); 69 <br> . . .<br> 89 // Verify \
the consistency of the whole JVMTI report again<br> 90 \
Layer.boot().equals(getModulesJVMTI()); 91 <br><br> The above \
lines can be removed.<br> They even do not check the result of \
comparison.<br><br>Thanks,<br>Serguei<br><br><br></p><blockquote \
style="margin-top:5.0pt;margin-bottom:5.0pt"><div><pre><span \
style="font-size:12.0pt"> </span></pre><p \
class="MsoNormal"><br>libJvmtiGetAllModulesTest.c<br><br> Unneeded indent for \
all lines.<br> Otherwise, it is \
good.<br><br>Thanks,<br>Serguei<br><br><br><br>On 7/21/16 10:14, Alexander Kulyakhtin \
wrote:</p></div><blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"><div><p \
class="MsoNormal" style="margin-bottom:12.0pt">Christian, Sergey,<br><br>I've \
modified the test per your findings. Now it is one java file and one C file \
only.<br><br>Please, find the updated review at:<br><br>Webrev: <a \
href="http://cr.openjdk.java.net/%7Eakulyakh/8153978_6/" \
target="_blank">http://cr.openjdk.java.net/~akulyakh/8153978_6/</a><br><br>Thank you \
very much for your help.<br><br>Best regards,<br>Alexander<br><br><br>----- Original \
Message -----<br>From: <a href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a><br>To: <a \
href="mailto:christian.tornqvist@oracle.com" \
target="_blank">christian.tornqvist@oracle.com</a>, <a \
href="mailto:alexander.kulyakhtin@oracle.com" \
target="_blank">alexander.kulyakhtin@oracle.com</a><br>Cc: <a \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br>Sent: Thursday, July 21, \
2016 6:39:21 PM GMT +03:00 Iraq<br>Subject: Re: RFR:8153978:New test to verify the \
modules info as returned by the JVMTI</p><div><div><p class="MsoNormal">On 7/21/16 \
08:29, Christian Tornqvist wrote:</p></div><blockquote \
style="margin-top:5.0pt;margin-bottom:5.0pt"><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Hi \
Alexander,</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">>The \
JVMTI always reports 3 unnamed modules: the boot module, the system module and the \
application module. </span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">>The \
Java API does not report any unnamed modules.</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">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.</span></p></blockquote><p class="MsoNormal" \
style="margin-bottom:12.0pt"><br>The rule is that there is one unnamed module per a \
class loader.<br>The options are: to test this rule or remove the check.<br>For \
simplicity is better to remove this check as \
unclear.<br><br>Thanks,<br>Serguei<br><br></p><blockquote \
style="margin-top:5.0pt;margin-bottom:5.0pt"><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">>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</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">>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.</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">>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.</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">>As \
the JVMTI spec says: "GetAllModules: Return an array of all modules loaded in the \
virtual machine.", it does not mention defining modules.</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">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.</span></p><p class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Thanks,</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">Christian</span></p><p \
class="MsoNormal"><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> </span></p><div><div \
style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"><p \
class="MsoNormal"><b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">From:</span></b><span \
style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> \
Alexander Kulyakhtin [<a href="mailto:alexander.kulyakhtin@oracle.com" \
target="_blank">mailto:alexander.kulyakhtin@oracle.com</a>] <br><b>Sent:</b> \
Thursday, July 21, 2016 10:04 AM<br><b>To:</b> Serguei Vladimirovich Spitsyn <a \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank"><serguei.spitsyn@oracle.com></a>; <a \
href="mailto:christian.tornqvist@oracle.com" \
target="_blank">christian.tornqvist@oracle.com</a><br><b>Cc:</b> <a \
href="mailto:serviceability-dev@openjdk.java.net" \
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic