[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR(S) (8u): 8035938: Memory leak in JvmtiEnv::GetConstantPool
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2015-01-28 21:51:35
Message-ID: 54C959E7.1040008 () oracle ! com
[Download RAW message or body]
Looks good.
Thanks,
Serguei
On 1/28/15 9:34 AM, Kevin Walls wrote:
> Hi,
>
> I'd like a sanity check review for backporting this from latest/9 to
> jdk8u. It's changing os:free to delete.
>
> In 8u the same change is relevant, the changeset does not import
> directly as we have to remove an (unnecessary) NMT parameter (which
> gets removed by 8060074 in jdk9).
>
> Thanks
> Kevin
>
> diff --git a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp \
> b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> --- a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> +++ b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> @@ -68,11 +68,11 @@
>
> ~JvmtiConstantPoolReconstituter() {
> if (_symmap != NULL) {
> - os::free(_symmap, mtClass);
> + delete _symmap;
> _symmap = NULL;
> }
> if (_classmap != NULL) {
> - os::free(_classmap, mtClass);
> + delete _classmap;
> _classmap = NULL;
> }
> }
>
>
>
> -------- Forwarded Message --------
> Subject: Re: RFR: 8035938: Memory leak in JvmtiEnv::GetConstantPool
> Date: Fri, 16 Jan 2015 18:28:50 +0000
> From: Kevin Walls <kevin.walls@oracle.com>
> Organization: Oracle Corporation
> To: daniel.daugherty@oracle.com, Mattis Castegren
> <mattis.castegren@oracle.com>
> CC: serviceability-dev@openjdk.java.net
>
>
>
> Thanks Serguei, thanks Dan!
>
>
> On 16/01/2015 17:51, Daniel D. Daugherty wrote:
> > > src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> >
> > These lines allocate:
> >
> > line 59: _symmap = new SymbolHashMap();
> > line 60: _classmap = new SymbolHashMap();
> >
> > and these lines free incorrectly:
> >
> > line 71: os::free(_symmap);
> > line 75: os::free(_classmap);
> >
> > so the proposed fix looks good!
> >
> >
> > Dan
> >
> >
> > On 1/16/15 3:17 AM, Mattis Castegren wrote:
> > > Hi
> > >
> > > This bug is targeted for 7u80, with rdp2 next Tuesday. It would be
> > > great to get a review for this fix as soon as possible, so that we
> > > can get this fix out in the last public JDK 7 release.
> > >
> > > Kind Regards
> > > /Mattis
> > >
> > > -----Original Message-----
> > > From: Kevin Walls
> > > Sent: den 15 januari 2015 15:18
> > > To:serviceability-dev@openjdk.java.net
> > > Subject: RFR: 8035938: Memory leak in JvmtiEnv::GetConstantPool
> > >
> > > Hi,
> > >
> > > This is a review request for a change in JVMTI, where we os::free() some
> > > objects where we should delete them. The problem was logged against 7,
> > > though it exists up to the present day, below is a diff in current
> > > latesthttp://hg.openjdk.java.net/jdk9/hs-rt/hotspot
> > >
> > >
> > > Testing:
> > > I've used a native JVMTI agent to call GetConstantPool() during an
> > > object allocation callback. Memory usage does appear less after the
> > > change, it can be 5-6MB in a trivial testcase with a small number of
> > > allocations monitored, each inoking GetConstantlPool().
> > >
> > > (In my test agent the GetConstantPool() call returns a JVMTI error 101
> > > after a fairly short time and a relatively small number of allocations
> > > are monitored.)
> > >
> > > If this is OK to submit without testcase that's great. I don't see
> > > other examples of native agents in the standard hotspot tests.
> > >
> > > bug
> > > https://bugs.openjdk.java.net/browse/JDK-8035938
> > >
> > > diff:
> > > $ hg diff src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> > > diff --git a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> > > b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> > > --- a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> > > +++ b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
> > > @@ -68,11 +68,11 @@
> > >
> > > ~JvmtiConstantPoolReconstituter() {
> > > if (_symmap != NULL) {
> > > - os::free(_symmap);
> > > + delete _symmap;
> > > _symmap = NULL;
> > > }
> > > if (_classmap != NULL) {
> > > - os::free(_classmap);
> > > + delete _classmap;
> > > _classmap = NULL;
> > > }
> > > }
> > >
> > >
> > >
> > > Thanks
> > > Kevin
> > >
> > >
> > >
> >
>
>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Looks good.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
On 1/28/15 9:34 AM, Kevin Walls wrote:<br>
</div>
<blockquote cite="mid:54C91DB3.5000609@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
Hi,<br>
<br>
I'd like a sanity check review for backporting this from latest/9
to jdk8u. It's changing os:free to delete.<br>
<br>
In 8u the same change is relevant, the changeset does not import
directly as we have to remove an (unnecessary) NMT parameter
(which gets removed by 8060074 in jdk9).<br>
<br>
Thanks<br>
Kevin<br>
<div class="moz-forward-container"><br>
<pre>diff --git a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp \
b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
--- a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
+++ b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
@@ -68,11 +68,11 @@
~JvmtiConstantPoolReconstituter() {
if (_symmap != NULL) {
- os::free(_symmap, mtClass);
+ delete _symmap;
_symmap = NULL;
}
if (_classmap != NULL) {
- os::free(_classmap, mtClass);
+ delete _classmap;
_classmap = NULL;
}
}
</pre>
<br>
<br>
-------- Forwarded Message --------
<table class="moz-email-headers-table" cellpadding="0"
cellspacing="0" border="0">
<tbody>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Subject:
</th>
<td>Re: RFR: 8035938: Memory leak in
JvmtiEnv::GetConstantPool</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Date:
</th>
<td>Fri, 16 Jan 2015 18:28:50 +0000</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">From:
</th>
<td>Kevin Walls <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:kevin.walls@oracle.com"><kevin.walls@oracle.com></a></td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Organization:
</th>
<td>Oracle Corporation</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">To: </th>
<td><a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:daniel.daugherty@oracle.com">daniel.daugherty@oracle.com</a>,
Mattis Castegren <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:mattis.castegren@oracle.com"><mattis.castegren@oracle.com></a></td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">CC: </th>
<td><a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a></td>
</tr>
</tbody>
</table>
<br>
<br>
<pre>Thanks Serguei, thanks Dan!
On 16/01/2015 17:51, Daniel D. Daugherty wrote:
> > src/share/vm/prims/jvmtiClassFileReconstituter.hpp
>
> These lines allocate:
>
> line 59: _symmap = new SymbolHashMap();
> line 60: _classmap = new SymbolHashMap();
>
> and these lines free incorrectly:
>
> line 71: os::free(_symmap);
> line 75: os::free(_classmap);
>
> so the proposed fix looks good!
>
>
> Dan
>
>
> On 1/16/15 3:17 AM, Mattis Castegren wrote:
>> Hi
>>
>> This bug is targeted for 7u80, with rdp2 next Tuesday. It would be
>> great to get a review for this fix as soon as possible, so that we
>> can get this fix out in the last public JDK 7 release.
>>
>> Kind Regards
>> /Mattis
>>
>> -----Original Message-----
>> From: Kevin Walls
>> Sent: den 15 januari 2015 15:18
>> To: <a moz-do-not-send="true" class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a>
>> Subject: RFR: 8035938: Memory leak in JvmtiEnv::GetConstantPool
>>
>> Hi,
>>
>> This is a review request for a change in JVMTI, where we os::free() some
>> objects where we should delete them. The problem was logged against 7,
>> though it exists up to the present day, below is a diff in current
>> latest <a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="http://hg.openjdk.java.net/jdk9/hs-rt/hotspot">http://hg.openjdk.java.net/jdk9/hs-rt/hotspot</a>
>>
>>
>> Testing:
>> I've used a native JVMTI agent to call GetConstantPool() during an
>> object allocation callback. Memory usage does appear less after the
>> change, it can be 5-6MB in a trivial testcase with a small number of
>> allocations monitored, each inoking GetConstantlPool().
>>
>> (In my test agent the GetConstantPool() call returns a JVMTI error 101
>> after a fairly short time and a relatively small number of allocations
>> are monitored.)
>>
>> If this is OK to submit without testcase that's great. I don't see
>> other examples of native agents in the standard hotspot tests.
>>
>> bug
>> <a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8035938">https://bugs.openjdk.java.net/browse/JDK-8035938</a>
>>
>> diff:
>> $ hg diff src/share/vm/prims/jvmtiClassFileReconstituter.hpp
>> diff --git a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
>> b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
>> --- a/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
>> +++ b/src/share/vm/prims/jvmtiClassFileReconstituter.hpp
>> @@ -68,11 +68,11 @@
>>
>> ~JvmtiConstantPoolReconstituter() {
>> if (_symmap != NULL) {
>> - os::free(_symmap);
>> + delete _symmap;
>> _symmap = NULL;
>> }
>> if (_classmap != NULL) {
>> - os::free(_classmap);
>> + delete _classmap;
>> _classmap = NULL;
>> }
>> }
>>
>>
>>
>> Thanks
>> Kevin
>>
>>
>>
>
</pre>
<br>
</div>
<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