[8u] RFR for backport of JDK-8170395: Metaspace initialization queries the wrong chunk freelist

Muthusamy Chinnathambi muthusamy.chinnathambi at oracle.com
Thu Dec 7 09:05:24 UTC 2017


Thanks a lot Stefan and Thomas.

Regards,
Muthusamy C

-----Original Message-----
From: Stefan Karlsson 
Sent: Wednesday, December 6, 2017 5:47 PM
To: Muthusamy Chinnathambi <muthusamy.chinnathambi at oracle.com>; Thomas Stüfe <thomas.stuefe at gmail.com>; Ioi Lam <ioi.lam at oracle.com>
Cc: HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>; Per Liden <per.liden at oracle.com>; Stephen Fitch <stephen.fitch at oracle.com>
Subject: Re: [8u] RFR for backport of JDK-8170395: Metaspace initialization queries the wrong chunk freelist



On 2017-12-06 12:54, Muthusamy Chinnathambi wrote:
> Hi Stefan,
> 
> Thanks for the review
> 
>> 1) Can you make sure that the unit test gets hooked into
>> execute_internal_vm_test in:
>> http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/b0f7174de2c5/src/share/vm/prims/jni.cpp
> 
>> And test it with:
>> -XX:+ExecuteInternalVMTests
> Yes. Please find the updated webrev at http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.02/  .
> Tested with  -XX:+ExecuteInternalVMTests as well.

Thanks, that looks good to me.

StefanK

> 
>> 2) report_insufficient_metaspace has been removed in later JDKs. Ioi, is
>> this addition of report_insufficient_metaspace OK?
> I think yes.  JDK9 still has this, although seem to have been removed in 10.
> 
>> The backport of metaspace.cpp and metaspace.hpp looks fine but I haven't
>> tracked all changes to the metaspace files for 8u-dev so a second pair
>> of eyes would be good.
> Agreed.
> Thomas Stüfe had reviewed it, although he had the concern of execution of the test case.
> The updated webrev should address it.
> 
> Regards,
> Muthusamy C
> 
> 
> -----Original Message-----
> From: Stefan Karlsson
> Sent: Tuesday, December 5, 2017 4:53 PM
> To: Muthusamy Chinnathambi <muthusamy.chinnathambi at oracle.com>; Thomas Stüfe <thomas.stuefe at gmail.com>; Ioi Lam <ioi.lam at oracle.com>
> Cc: HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>; Per Liden <per.liden at oracle.com>; Stephen Fitch <stephen.fitch at oracle.com>
> Subject: Re: [8u] RFR for backport of JDK-8170395: Metaspace initialization queries the wrong chunk freelist
> 
> Hi Muthusamy,
> 
> The backport of metaspace.cpp and metaspace.hpp looks fine but I haven't
> tracked all changes to the metaspace files for 8u-dev so a second pair
> of eyes would be good.
> 
> Two things:
> 
> 1) Can you make sure that the unit test gets hooked into
> execute_internal_vm_test in:
> http://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/b0f7174de2c5/src/share/vm/prims/jni.cpp
> 
> And test it with:
> -XX:+ExecuteInternalVMTests
> 
> 
> 2) report_insufficient_metaspace has been removed in later JDKs. Ioi, is
> this addition of report_insufficient_metaspace OK?
> 
> Thanks,
> StefanK
> 
> 
> On 2017-11-17 09:59, Muthusamy Chinnathambi wrote:
>> Hi Thomas,
>>
>> Thanks for the review.
>>
>>> . I also would prefer that the gtests you omitted should be part of the
>> change (I think before there were gtests, these kind of tests were
>> implemented as file-local static test functions and called I think via
>> whitebox.cpp ? But I wait what others say.
>>
>> Yes, the test is part of
>> https://bugs.openjdk.java.net/browse/JDK-8169931 . I would be
>> backporting this as well, just that I didn’t want to pollute the current BP.
>>
>> Regards,
>>
>> Muthusamy C
>>
>> *From:*Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
>> *Sent:* Friday, November 17, 2017 2:21 PM
>> *To:* Muthusamy Chinnathambi <muthusamy.chinnathambi at oracle.com>
>> *Cc:* HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>; Per
>> Liden <per.liden at oracle.com>; Stefan Karlsson
>> <stefan.karlsson at oracle.com>; Stephen Fitch <stephen.fitch at oracle.com>
>> *Subject:* Re: [8u] RFR for backport of JDK-8170395: Metaspace
>> initialization queries the wrong chunk freelist
>>
>> Hi Muthusamy,
>>
>>   From the look of it updated webrev looks fine, but I do not have the
>> time to build and check on jdk8u. I also would prefer that the gtests
>> you omitted should be part of the change (I think before there were
>> gtests, these kind of tests were implemented as file-local static test
>> functions and called I think via whitebox.cpp ? But I wait what others say.
>>
>> Someone from Oracle should confirm this patch. Note that Mikael left
>> Oracle, I took him off the reply list because mail bounce.
>>
>> Kind Regards, Thomas
>>
>> On Thu, Nov 16, 2017 at 10:08 AM, Muthusamy Chinnathambi
>> <muthusamy.chinnathambi at oracle.com
>> <mailto:muthusamy.chinnathambi at oracle.com>> wrote:
>>
>>      Hi Thomas,
>>
>>      Thanks for the review.
>>
>>       > But are there gtests already in jdk8u? I thought gtests came with
>>      jdk9?
>>
>>       > http://openjdk.java.net/jeps/281
>>
>>       > If not, how does
>>      http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.00/test/native/memory/test_spaceManager.cpp.html
>>      run or even compile?
>>
>>      Yes, you are right. "test_spaceManager.cpp" should not have been
>>      included here.
>>
>>      Please find the updated webrev at
>>      http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.01/  .
>>
>>      Regards,
>>
>>      Muthusamy C
>>
>>      *From:*Thomas Stüfe [mailto:thomas.stuefe at gmail.com
>>      <mailto:thomas.stuefe at gmail.com>]
>>      *Sent:* Wednesday, November 15, 2017 6:30 PM
>>      *To:* Muthusamy Chinnathambi <muthusamy.chinnathambi at oracle.com
>>      <mailto:muthusamy.chinnathambi at oracle.com>>
>>      *Cc:* HotSpot Open Source Developers <hotspot-dev at openjdk.java.net
>>      <mailto:hotspot-dev at openjdk.java.net>>; Mikael Gerdin
>>      <mikael.gerdin at oracle.com <mailto:mikael.gerdin at oracle.com>>; Per
>>      Liden <per.liden at oracle.com <mailto:per.liden at oracle.com>>; Stefan
>>      Karlsson <stefan.karlsson at oracle.com
>>      <mailto:stefan.karlsson at oracle.com>>; Stephen Fitch
>>      <stephen.fitch at oracle.com <mailto:stephen.fitch at oracle.com>>
>>      *Subject:* Re: [8u] RFR for backport of JDK-8170395: Metaspace
>>      initialization queries the wrong chunk freelist
>>
>>      Hi Muthusamy,
>>
>>      Looks okay. But are there gtests already in jdk8u? I thought gtests
>>      came with jdk9?
>>
>>      http://openjdk.java.net/jeps/281
>>
>>      If not, how does
>>      http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.00/test/native/memory/test_spaceManager.cpp.html
>>      run or even compile?
>>
>>      Thanks, Thomas
>>
>>      On Wed, Nov 15, 2017 at 11:19 AM, Muthusamy Chinnathambi
>>      <muthusamy.chinnathambi at oracle.com
>>      <mailto:muthusamy.chinnathambi at oracle.com>> wrote:
>>
>>          Hi,
>>
>>          Could someone please review this backport.
>>
>>          Regards,
>>          Muthusamy C
>>
>>          -----Original Message-----
>>          From: Muthusamy Chinnathambi
>>
>>          Sent: Friday, November 10, 2017 3:27 PM
>>          To: HotSpot Open Source Developers <hotspot-dev at openjdk.java.net
>>          <mailto:hotspot-dev at openjdk.java.net>>
>>          Cc: Mikael Gerdin <mikael.gerdin at oracle.com
>>          <mailto:mikael.gerdin at oracle.com>>
>>          Subject: RE: [8u] RFR for backport of JDK-8170395: Metaspace
>>          initialization queries the wrong chunk freelist
>>
>>          Hi,
>>
>>          Can someone please review this.
>>
>>          Thanks!
>>          Muthusamy C
>>
>>          -----Original Message-----
>>          From: Muthusamy Chinnathambi
>>          Sent: Wednesday, November 8, 2017 11:24 AM
>>          To: Thomas Stüfe <thomas.stuefe at gmail.com
>>          <mailto:thomas.stuefe at gmail.com>>
>>          Cc: Mikael Gerdin <mikael.gerdin at oracle.com
>>          <mailto:mikael.gerdin at oracle.com>>; HotSpot Open Source
>>          Developers <hotspot-dev at openjdk.java.net
>>          <mailto:hotspot-dev at openjdk.java.net>>
>>          Subject: RE: [8u] RFR for backport of JDK-8170395: Metaspace
>>          initialization queries the wrong chunk freelist
>>
>>          Hi Thomas,
>>
>>
>>
>>           >  the webrev link seems to be dead.
>>
>>          Sorry for the trouble.
>>
>>          Please find the corrected link below
>>
>>          http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.00/
>>
>>
>>
>>          Somehow the line following the link got appended to the previous
>>          one leading to a dead link.
>>
>>
>>
>>          Regards,
>>
>>          Muthusamy C
>>
>>
>>
>>          From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com
>>          <mailto:thomas.stuefe at gmail.com>]
>>          Sent: Tuesday, November 7, 2017 8:12 PM
>>          To: Muthusamy Chinnathambi <muthusamy.chinnathambi at oracle.com
>>          <mailto:muthusamy.chinnathambi at oracle.com>>
>>          Cc: HotSpot Open Source Developers <hotspot-dev at openjdk.java.net
>>          <mailto:hotspot-dev at openjdk.java.net>>; Mikael Gerdin
>>          <mikael.gerdin at oracle.com <mailto:mikael.gerdin at oracle.com>>
>>          Subject: Re: [8u] RFR for backport of JDK-8170395: Metaspace
>>          initialization queries the wrong chunk freelist
>>
>>
>>
>>          Hi Muthusamy,
>>
>>
>>
>>          the webrev link seems to be dead.
>>
>>
>>
>>          ..Thomas
>>
>>
>>
>>          On Tue, Nov 7, 2017 at 10:05 AM, Muthusamy Chinnathambi
>>          <HYPERLINK "mailto:muthusamy.chinnathambi at oracle.com
>>          <mailto:muthusamy.chinnathambi at oracle.com>"muthusamy.chinnathambi at oracle.com
>>          <mailto:muthusamy.chinnathambi at oracle.com>> wrote:
>>
>>          Hi,
>>
>>          Please review the backport of bug: "JDK-8170395: Metaspace
>>          initialization queries the wrong chunk freelist" to jdk8u-dev
>>
>>          Please note that this is not a clean backport due to new entries
>>          in debug.cpp and copyright changes.
>>
>>
>>          Webrev: HYPERLINK
>>          "http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.00/jdk9"http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.00/
>>          jdk9
>>          <http://cr.openjdk.java.net/~mchinnathamb/8170395/webrev.00/jdk9> bug:
>>          https://bugs.openjdk.java.net/browse/JDK-8170395
>>          Original patch pushed to jdk9:
>>          http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/2e971a72675a
>>
>>          Test:  Had run jtreg and jprt hotspot testsets.
>>
>>          Regards,
>>          Muthusamy C
>>


More information about the hotspot-dev mailing list