[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8075586: add @modules as needed to the open hotspot tests
From:       Mandy Chung <mandy.chung () oracle ! com>
Date:       2015-03-26 20:18:05
Message-ID: 5514697D.3020308 () oracle ! com
[Download RAW message or body]

On 3/26/15 5:38 AM, Yekaterina Kantserova wrote:
> Mandy,
>
> Thank you very much for the catch! The updated webrev can be found here:
>
> http://cr.openjdk.java.net/~ykantser/8075586/webrev.00/
>

Thanks for fixing this.  Lois and other have reviewed it.  I only tried 
to spot anything stick out.  This looks okay and good to push.

+ * @modules java.base/sun.misc
+ *          java.management

I don't have the cycle to find out where the dependencies for this patch.
looks like they are mostly from the testlibrary.

Per our offline discussion, it'd be desirable to update the testlibrary 
and tests to assist the dependency analysis process or eliminate the 
dependency.

One more thing to mention is that the testlibrary depending 
java.management to get the process ID can soon be removed once the new 
Process API getting pid is integrated into JDK 9.

thanks
Mandy

> Best regards,
> Katja
>
>
> On 03/26/2015 12:11 AM, Mandy Chung wrote:
>> Alexandar, Shura,
>>
>> The dependency analysis is not up-to-date that sun.tools.jar
>> has been moved to jdk.jartool module in jdk9 b55.  It has been
>> in jdk9/dev since 3/6.
>>
>> I have pointed out multiple times previously that jdk.dev/sun.tools.jar
>> is wrong in the jdk side of change.
>>
>> Below includes an example.
>> Mandy
>>
>> --- old/test/runtime/RedefineTests/RedefineAnnotations.java 
>> 2015-03-25 16:24:41.462038538 +0300
>> +++ new/test/runtime/RedefineTests/RedefineAnnotations.java 
>> 2015-03-25 16:24:41.386038539 +0300
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2014, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -25,6 +25,9 @@
>>   * @test
>>   * @library /testlibrary
>>   * @summary Test that type annotations are retained after a retransform
>> + * @modules java.base/jdk.internal.org.objectweb.asm
>> + *          java.instrument
>> + *          jdk.dev/sun.tools.jar
>>   * @run main RedefineAnnotations buildagent
>>   * @run main/othervm -javaagent:redefineagent.jar RedefineAnnotations
>>   */
>>
>>
>> On 3/25/15 7:38 AM, Alexander Kulyakhtin wrote:
>>> Hi
>>>
>>> Please, find the updated review for the bulk @modules change at the 
>>> link below.
>>>
>>> We have fixed the copyrights and the files mentioned in the mail 
>>> from Lois.
>>>
>>> http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8075586/index.html
>>>
>>> Best regards,
>>> Alex
>>>
>>>
>>>
>>> ----- Original Message -----
>>> From: lois.foltan@oracle.com
>>> To: yekaterina.kantserova@oracle.com
>>> Cc: serviceability-dev@openjdk.java.net, staffan.larsen@oracle.com, 
>>> hotspot-dev@openjdk.java.net, alexander.kulyakhtin@oracle.com, 
>>> alexandre.iline@oracle.com
>>> Sent: Tuesday, March 24, 2015 3:57:54 PM GMT +04:00 Abu Dhabi / Muscat
>>> Subject: Re: RFR: JDK-8075586: add @modules as needed to the open 
>>> hotspot tests
>>>
>>>
>>> This looks good, thank you for making these changes!  A couple of
>>> comments that I don't feel need another webrev but should be fixed
>>> before pushing.
>>>
>>>       - copyrights on all the tests need to be updated
>>>       - the following tests have a blank comment line before the new
>>> "@modules" line that could be removed
>>>         test/gc/metaspace/TestMetaspacePerfCounters.java
>>>         test/runtime/contended/Basic.java
>>> test/compiler/jsr292/CreatesInterfaceDotEqualsCallInfo.java
>>>         test/compiler/cpuflags/RestoreMXCSR.java
>>>         test/compiler/debug/VerifyAdapterSharing.java
>>>
>>> Thanks,
>>> Lois
>>>
>>> On 3/24/2015 8:09 AM, Yekaterina Kantserova wrote:
>>>> Notifying hotspot-dev as well.
>>>>
>>>> // Katja
>>>>
>>>>
>>>>
>>>> On 03/24/2015 11:48 AM, Alexander Kulyakhtin wrote:
>>>>> Could the reviewers, please, have a look at the proposed changes 
>>>>> below?
>>>>>
>>>>> In addition, we are going to make a change to the TEST.ROOT file as
>>>>> indicated by Staffan in the mail below.
>>>>>
>>>>> Do you think the changes (plus the one-line change to the TEST.ROOT)
>>>>> can be pushed into the jdk?
>>>>>
>>>>> Best regards,
>>>>> Alex
>>>>>
>>>>> ----- Original Message -----
>>>>> From: staffan.larsen@oracle.com
>>>>> To: alexander.kulyakhtin@oracle.com
>>>>> Cc: serviceability-dev@openjdk.java.net, alexandre.iline@oracle.com
>>>>> Sent: Friday, March 20, 2015 7:39:10 PM GMT +04:00 Abu Dhabi / Muscat
>>>>> Subject: Re: RFR: JDK-8075586: add @modules as needed to the open
>>>>> hotspot tests
>>>>>
>>>>> I haven't looked at the changes in detail, but please change the
>>>>> requiredVersion in TEST.ROOT to 4.1 b11 as part of this change.
>>>>>
>>>>> Thanks,
>>>>> /Staffan
>>>>>
>>>>>> On 20 mar 2015, at 13:16, Alexander Kulyakhtin
>>>>>> <alexander.kulyakhtin@oracle.com> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Could you, please, review the fix below.
>>>>>>
>>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8075586
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~tpivovarova/akulyakh/8075586/webrev.00/
>>>>>>
>>>>>> The fix adds @modules keyword to the existing hotspot tests, as
>>>>>> needed, so that the tests can access the required API when the new
>>>>>> modular architecture is in place.
>>>>>>
>>>>>> Best regards,
>>>>>> Alex
>>
>


[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 3/26/15 5:38 AM, Yekaterina
      Kantserova wrote:<br>
    </div>
    <blockquote cite="mid:5513FDB6.6070606@oracle.com" type="cite">Mandy,
      <br>
      <br>
      Thank you very much for the catch! The updated webrev can be found
      here:
      <br>
      <br>
      <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~ykantser/8075586/webrev.00/">http://cr.openjdk.java.net/~ykantser/8075586/webrev.00/</a>
  <br>
      <br>
    </blockquote>
    <br>
    Thanks for fixing this.   Lois and other have reviewed it.   I only
    tried to spot anything stick out.   This looks okay and good to push.
    <br>
    <pre><meta http-equiv="content-type" content="text/html; charset=UTF-8">+ * \
@modules java.base/sun.misc + *          java.management<pre></pre>I don't have the \
cycle to find out where the dependencies for this patch. looks like they are mostly \
from the testlibrary. </pre>
    Per our offline discussion, it'd be desirable to update the
    testlibrary and tests to assist the dependency analysis process or
    eliminate the dependency.<br>
    <br>
    One more thing to mention is that the testlibrary depending
    java.management to get the process ID can soon be removed once the
    new Process API getting pid is integrated into JDK 9.     <br>
    <br>
    thanks<br>
    Mandy<br>
    <br>
    <blockquote cite="mid:5513FDB6.6070606@oracle.com" type="cite">Best
      regards,
      <br>
      Katja
      <br>
      <br>
      <br>
      On 03/26/2015 12:11 AM, Mandy Chung wrote:
      <br>
      <blockquote type="cite">Alexandar, Shura,
        <br>
        <br>
        The dependency analysis is not up-to-date that sun.tools.jar
        <br>
        has been moved to jdk.jartool module in jdk9 b55.   It has been
        <br>
        in jdk9/dev since 3/6.
        <br>
        <br>
        I have pointed out multiple times previously that
        jdk.dev/sun.tools.jar
        <br>
        is wrong in the jdk side of change.
        <br>
        <br>
        Below includes an example.
        <br>
        Mandy
        <br>
        <br>
        --- old/test/runtime/RedefineTests/RedefineAnnotations.java
        2015-03-25 16:24:41.462038538 +0300
        <br>
        +++ new/test/runtime/RedefineTests/RedefineAnnotations.java
        2015-03-25 16:24:41.386038539 +0300
        <br>
        @@ -1,5 +1,5 @@
        <br>
          /*
        <br>
        - * Copyright (c) 2014, Oracle and/or its affiliates. All rights
        reserved.
        <br>
        + * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All
        rights reserved.
        <br>
           * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE
        HEADER.
        <br>
           *
        <br>
           * This code is free software; you can redistribute it and/or
        modify it
        <br>
        @@ -25,6 +25,9 @@
        <br>
           * @test
        <br>
           * @library /testlibrary
        <br>
           * @summary Test that type annotations are retained after a
        retransform
        <br>
        + * @modules java.base/jdk.internal.org.objectweb.asm
        <br>
        + *                   java.instrument
        <br>
        + *                   jdk.dev/sun.tools.jar
        <br>
           * @run main RedefineAnnotations buildagent
        <br>
           * @run main/othervm -javaagent:redefineagent.jar
        RedefineAnnotations
        <br>
           */
        <br>
        <br>
        <br>
        On 3/25/15 7:38 AM, Alexander Kulyakhtin wrote:
        <br>
        <blockquote type="cite">Hi
          <br>
          <br>
          Please, find the updated review for the bulk @modules change
          at the link below.
          <br>
          <br>
          We have fixed the copyrights and the files mentioned in the
          mail from Lois.
          <br>
          <br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8075586/index.html">http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8075586/index.html</a>
  <br>
          <br>
          Best regards,
          <br>
          Alex
          <br>
          <br>
          <br>
          <br>
          ----- Original Message -----
          <br>
          From: <a class="moz-txt-link-abbreviated" \
href="mailto:lois.foltan@oracle.com">lois.foltan@oracle.com</a>  <br>
          To: <a class="moz-txt-link-abbreviated" \
href="mailto:yekaterina.kantserova@oracle.com">yekaterina.kantserova@oracle.com</a>  \
                <br>
          Cc: <a class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a>,
  <a class="moz-txt-link-abbreviated" \
href="mailto:staffan.larsen@oracle.com">staffan.larsen@oracle.com</a>, <a \
class="moz-txt-link-abbreviated" \
href="mailto:hotspot-dev@openjdk.java.net">hotspot-dev@openjdk.java.net</a>,  <a \
class="moz-txt-link-abbreviated" \
href="mailto:alexander.kulyakhtin@oracle.com">alexander.kulyakhtin@oracle.com</a>, <a \
class="moz-txt-link-abbreviated" \
href="mailto:alexandre.iline@oracle.com">alexandre.iline@oracle.com</a>  <br>
          Sent: Tuesday, March 24, 2015 3:57:54 PM GMT +04:00 Abu Dhabi
          / Muscat
          <br>
          Subject: Re: RFR: JDK-8075586: add @modules as needed to the
          open hotspot tests
          <br>
          <br>
          <br>
          This looks good, thank you for making these changes!   A couple
          of
          <br>
          comments that I don't feel need another webrev but should be
          fixed
          <br>
          before pushing.
          <br>
          <br>
                     - copyrights on all the tests need to be updated
          <br>
                     - the following tests have a blank comment line before
          the new
          <br>
          "@modules" line that could be removed
          <br>
                         test/gc/metaspace/TestMetaspacePerfCounters.java
          <br>
                         test/runtime/contended/Basic.java
          <br>
          test/compiler/jsr292/CreatesInterfaceDotEqualsCallInfo.java
          <br>
                         test/compiler/cpuflags/RestoreMXCSR.java
          <br>
                         test/compiler/debug/VerifyAdapterSharing.java
          <br>
          <br>
          Thanks,
          <br>
          Lois
          <br>
          <br>
          On 3/24/2015 8:09 AM, Yekaterina Kantserova wrote:
          <br>
          <blockquote type="cite">Notifying hotspot-dev as well.
            <br>
            <br>
            // Katja
            <br>
            <br>
            <br>
            <br>
            On 03/24/2015 11:48 AM, Alexander Kulyakhtin wrote:
            <br>
            <blockquote type="cite">Could the reviewers, please, have a
              look at the proposed changes below?
              <br>
              <br>
              In addition, we are going to make a change to the
              TEST.ROOT file as
              <br>
              indicated by Staffan in the mail below.
              <br>
              <br>
              Do you think the changes (plus the one-line change to the
              TEST.ROOT)
              <br>
              can be pushed into the jdk?
              <br>
              <br>
              Best regards,
              <br>
              Alex
              <br>
              <br>
              ----- Original Message -----
              <br>
              From: <a class="moz-txt-link-abbreviated" \
href="mailto:staffan.larsen@oracle.com">staffan.larsen@oracle.com</a>  <br>
              To: <a class="moz-txt-link-abbreviated" \
href="mailto:alexander.kulyakhtin@oracle.com">alexander.kulyakhtin@oracle.com</a>  \
                <br>
              Cc: <a class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a>,
                
              <a class="moz-txt-link-abbreviated" \
href="mailto:alexandre.iline@oracle.com">alexandre.iline@oracle.com</a>  <br>
              Sent: Friday, March 20, 2015 7:39:10 PM GMT +04:00 Abu
              Dhabi / Muscat
              <br>
              Subject: Re: RFR: JDK-8075586: add @modules as needed to
              the open
              <br>
              hotspot tests
              <br>
              <br>
              I haven't looked at the changes in detail, but please
              change the
              <br>
              requiredVersion in TEST.ROOT to 4.1 b11 as part of this
              change.
              <br>
              <br>
              Thanks,
              <br>
              /Staffan
              <br>
              <br>
              <blockquote type="cite">On 20 mar 2015, at 13:16,
                Alexander Kulyakhtin
                <br>
                <a class="moz-txt-link-rfc2396E" \
href="mailto:alexander.kulyakhtin@oracle.com">&lt;alexander.kulyakhtin@oracle.com&gt;</a> \
wrote:  <br>
                <br>
                Hi,
                <br>
                <br>
                Could you, please, review the fix below.
                <br>
                <br>
                CR: <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8075586">https://bugs.openjdk.java.net/browse/JDK-8075586</a>
  <br>
                webrev:
                <br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~tpivovarova/akulyakh/8075586/webrev.00/">http://cr.openjdk.java.net/~tpivovarova/akulyakh/8075586/webrev.00/</a>
  <br>
                <br>
                The fix adds @modules keyword to the existing hotspot
                tests, as
                <br>
                needed, so that the tests can access the required API
                when the new
                <br>
                modular architecture is in place.
                <br>
                <br>
                Best regards,
                <br>
                Alex
                <br>
              </blockquote>
            </blockquote>
          </blockquote>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic