[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR(XS): 8036666: JVMTI GetObjectMonitorUsage does not return correct recursion count
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2014-03-25 8:19:02
Message-ID: 53313BF6.2050100 () oracle ! com
[Download RAW message or body]
Hi Axel,
Sorry for the delay, I was out and busy last week.
Dmitry noted that you moved the fix to the incorrect place and asked me
to double-check it.
I think, Dmitry is right.
He asked to move the code after the line 1017 so that the moved fragment
will be inside the 'if' statement started at the line:
1003 if (owning_thread != NULL) {
But in the webrev the code was moved after the line 1019 which seems to
be incorrect.
Could you, please, double check this and confirm that you are agree with
this?
Otherwise, please, provide some explanation why you think this webrev is
correct.
Thanks!
Serguei
On 3/19/14 1:05 AM, Siebenborn, Axel wrote:
> Hi Dmitri,
> Thank you for the review.
> You're right, the code gets cleaner this way.
> New webrev:
> http://www.sapjvm.com/as/webrevs/8036666_3/
> Thanks,
> Axel
>
> On 17.03.2014 23:01, Dmitry Samersoff wrote:
> > Axel,
> >
> > The changes it self looks good for me.
> >
> > But it looks like the owning_thread is always NULL if
> > owner is NULL, so we can safely move this code
> > to ll.1017 and join two identical ifs together.
> >
> > Also comment on ll. 1019 is misleading, could you remove it?
> >
> > -Dmitry
> >
> > On 2014-03-13 12:19, Siebenborn, Axel wrote:
> > > Hi Serguei,
> > > new webrev:
> > > http://www.sapjvm.com/as/webrevs/8036666_2/
> > > I should review my own changes more carefully.
> > > Sorry for that.
> > > Thanks,
> > > Axel
> > >
> > >
> > >
> > > On 12.03.2014 18:34, serguei.spitsyn@oracle.com wrote:
> > > > Hi Axel,
> > > >
> > > > Thank you for the changes! It looks good, but one more place need a
> > > > fix (expected must be 4 now):
> > > >
> > > > 230 if (recursionCount != 4) { 231 throw new
> > > > AssertionError("recursions: expected 3, but was " + recursionCount);
> > > > 232 }
> > > >
> > > >
> > > > Thanks, Serguei
> > > >
> > > >
> > > > On 3/12/14 8:21 AM, Siebenborn, Axel wrote:
> > > > > Hi Serguei, I created a new webrev:
> > > > >
> > > > > http://www.sapjvm.com/as/webrevs/8036666_1/
> > > > >
> > > > > I incorporated your suggestions and moved the test files.
> > > > >
> > > > > Thanks, Axel
> > > > >
> > > > >
> > > > > On 11.03.2014 20:25, serguei.spitsyn@oracle.com wrote:
> > > > > > On 3/11/14 12:05 PM, Staffan Larsen wrote:
> > > > > > > On 11 mar 2014, at 16:48, Siebenborn, Axel
> > > > > > > <axel.siebenborn@sap.com <mailto:axel.siebenborn@sap.com>>
> > > > > > > wrote:
> > > > > > > > Hi Seguei, I still can't upload files to the cr.openjdk
> > > > > > > > server. Meanwhile, I use our server for the new webrev:
> > > > > > > >
> > > > > > > > http://www.sapjvm.com/as/webrevs/8036666/
> > > > > > > >
> > > > > > > > Thanks, Axel
> > > > > > > >
> > > > > > > > Comments inline:
> > > > > > > >
> > > > > > > > On 11.03.2014 09:50,serguei.spitsyn@oracle.com
> > > > > > > > <mailto:serguei.spitsyn@oracle.com>wrote:
> > > > > > > > > Hi Axel,
> > > > > > > > >
> > > > > > > > > The webrev link is resolvable now. Thank you for taking
> > > > > > > > > care about your broken account on the cr.openjdk server!
> > > > > > > > >
> > > > > > > > > I have some comments on the test case ...
> > > > > > > > >
> > > > > > > > > - This is nice test, thank you for providing it!
> > > > > > > > >
> > > > > > > > > - The location of the test must be different as it is a
> > > > > > > > > JVMTI test: test/serviceability/jvmti/8036666 instead of
> > > > > > > > > test/runtime/8036666
> > > > > > > > I moved the test.
> > > > > > > Tests should avoid the bug number in the name or path and
> > > > > > > instead use a descriptive name. See this page for some
> > > > > > > background:
> > > > > > > https://wiki.openjdk.java.net/display/HotSpot/Naming+HotSpot+JTReg+Tests
> > > > > > >
> > > The test files have already descriptive names.
> > > > > > So, it would be enough to remove the bug number from the path.
> > > > > > Sorry, I had to catch it too in the first place.
> > > > > >
> > > > > > Thanks, Serguei
> > > > > > > Thanks, /Staffan
> > > > > > > > > RecursiveObjectLock,java:
> > > > > > > > >
> > > > > > > > > - A suggestion to add a synchronized method (say,
> > > > > > > > > nestedLock3) into the chain of calls started from the
> > > > > > > > > testMethod. In order to do so, the class
> > > > > > > > > RecursiveObjectLock needs to extend the ALock class. And
> > > > > > > > > the this object needs to be used in the synchronized
> > > > > > > > > statements and for wait()? What do you think about such
> > > > > > > > > test enhancement for better coverage?
> > > > > > > > In order to have a synchronized method in the call chain, I
> > > > > > > > synchronize on the "this" object.
> > > > > > > > > GetObjectLockCount.java:
> > > > > > > > >
> > > > > > > > > - The comment line 283 seems to be obsolete as the "param
> > > > > > > > > out" is not present anymore:
> > > > > > > > >
> > > > > > > > > 283 * @param out Stream to copy to
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > - Could you, please, add e.printStackTrace() into the catch
> > > > > > > > > statements at the lines 232 and 300?
> > > > > > > > >
> > > > > > > > > - A question: It seems the errThread and outThread are
> > > > > > > > > started a little bit late. Would it be better to start them
> > > > > > > > > earlier, or it was intentional?
> > > > > > > > You're right! I moved to code up.
> > > > > > > > > Some minor style-related comments (I hope, it is easy to
> > > > > > > > > fix this before push): - Unneeded extra empty lines:
> > > > > > > > > 149, 174-175, 244 - A space is missed before the '{':
> > > > > > > > > 180, 242, 243, 246 - Unneeded extra space after and before
> > > > > > > > > the "(": 235, 297 - The curly brackets '{' do not follow
> > > > > > > > > the common style: 142, 144
> > > > > > > > I hope I fixed them all and added no new style violations. Do
> > > > > > > > you have a tool to check this?
> > > > > > > > > We still need another reviewer for this fix. I'm ready to
> > > > > > > > > be a sponsor for it.
> > > > > > > > >
> > > > > > > > > Thanks, Serguei
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On 3/10/14 12:00 AM, serguei.spitsyn@oracle.com
> > > > > > > > > <mailto:serguei.spitsyn@oracle.com> wrote:
> > > > > > > > > > Hi Axel,
> > > > > > > > > >
> > > > > > > > > > The webrev link does not work now. I'll check it again
> > > > > > > > > > tomorrow.
> > > > > > > > > >
> > > > > > > > > > Thanks, Serguei
> > > > > > > > > >
> > > > > > > > > > On 3/7/14 1:32 AM, serguei.spitsyn@oracle.com
> > > > > > > > > > <mailto:serguei.spitsyn@oracle.com> wrote:
> > > > > > > > > > > Hi Axel,
> > > > > > > > > > >
> > > > > > > > > > > Thank you for fixing this issue. I'm reviewing it. It
> > > > > > > > > > > looks good in general, but a little bit more time is
> > > > > > > > > > > needed to look at the tests.
> > > > > > > > > > >
> > > > > > > > > > > Do you need a sponsor for pushing?
> > > > > > > > > > >
> > > > > > > > > > > Thanks, Serguei
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On 3/6/14 12:08 AM, Siebenborn, Axel wrote:
> > > > > > > > > > > > Hi all,
> > > > > > > > > > > >
> > > > > > > > > > > > could I have a review for the following change?
> > > > > > > > > > > >
> > > > > > > > > > > > The recursive lock count for an object is not
> > > > > > > > > > > > correct, in cases, where a monitor is inflated after
> > > > > > > > > > > > recursive lightweight locks. In this case, the
> > > > > > > > > > > > recursion count is taken from the heavyweight
> > > > > > > > > > > > monitor, represented by the class ObjectMonitor.
> > > > > > > > > > > > ObjectMonitor::_recursions is the number of times
> > > > > > > > > > > > ObjectMonitor::enter() was called to acquire the lock
> > > > > > > > > > > > minus 1. This counter does not include the recursions
> > > > > > > > > > > > of lightweight locks, that happen before inflating
> > > > > > > > > > > > the monitor and is not equal to the recursion count
> > > > > > > > > > > > from a Java source level point of view.
> > > > > > > > > > > >
> > > > > > > > > > > > I added a test to the webrev to reproduce the
> > > > > > > > > > > > problem.
> > > > > > > > > > > >
> > > > > > > > > > > > The suggested fix is to call count_locked_objects,
> > > > > > > > > > > > even if there's a heavyweight monitor and get the
> > > > > > > > > > > > recursion count by iterating the vframes.
> > > > > > > > > > > >
> > > > > > > > > > > > Bug:
> > > > > > > > > > > >
> > > > > > > > > > > > https://bugs.openjdk.java.net/browse/JDK-8036666
> > > > > > > > > > > >
> > > > > > > > > > > > Webrev:
> > > > > > > > > > > >
> > > > > > > > > > > > http://cr.openjdk.java.net/~asiebenborn/8036666/webrev/
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > Thanks,
> > > > > > > > > > > > Axel
> > > >
> >
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Axel,<br>
<br>
Sorry for the delay, I was out and busy last week.<br>
Dmitry noted that you moved the fix to the incorrect place and
asked me to double-check it.<br>
<br>
I think, Dmitry is right.<br>
He asked to move the code after the line 1017 so that the moved
fragment<br>
will be inside the 'if' statement started at the line:
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre>1003 if (owning_thread != NULL) {</pre>
<br>
But in the webrev the code was moved after the line 1019 which
seems to be incorrect.<br>
<br>
Could you, please, double check this and confirm that you are
agree with this?<br>
Otherwise, please, provide some explanation why you think this
webrev is correct.<br>
<br>
Thanks!<br>
Serguei<br>
<br>
<br>
On 3/19/14 1:05 AM, Siebenborn, Axel wrote:<br>
</div>
<blockquote
cite="mid:02D5D45C1F8DB848A7AE20E80EE61A5C398147F8@DEWDFEMB20C.global.corp.sap"
type="cite">
<pre wrap="">Hi Dmitri,
Thank you for the review.
You're right, the code gets cleaner this way.
New webrev:
<a class="moz-txt-link-freetext" \
href="http://www.sapjvm.com/as/webrevs/8036666_3/">http://www.sapjvm.com/as/webrevs/8036666_3/</a>
Thanks,
Axel
On 17.03.2014 23:01, Dmitry Samersoff wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Axel,
The changes it self looks good for me.
But it looks like the owning_thread is always NULL if
owner is NULL, so we can safely move this code
to ll.1017 and join two identical ifs together.
Also comment on ll. 1019 is misleading, could you remove it?
-Dmitry
On 2014-03-13 12:19, Siebenborn, Axel wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Serguei,
new webrev:
<a class="moz-txt-link-freetext" \
href="http://www.sapjvm.com/as/webrevs/8036666_2/">http://www.sapjvm.com/as/webrevs/8036666_2/</a>
I should review my own changes more carefully.
Sorry for that.
Thanks,
Axel
On 12.03.2014 18:34, <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote: </pre>
<blockquote type="cite">
<pre wrap="">Hi Axel,
Thank you for the changes! It looks good, but one more place need a
fix (expected must be 4 now):
230 if (recursionCount != 4) { 231 throw new
AssertionError("recursions: expected 3, but was " + recursionCount);
232 }
Thanks, Serguei
On 3/12/14 8:21 AM, Siebenborn, Axel wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Serguei, I created a new webrev:
<a class="moz-txt-link-freetext" \
href="http://www.sapjvm.com/as/webrevs/8036666_1/">http://www.sapjvm.com/as/webrevs/8036666_1/</a>
I incorporated your suggestions and moved the test files.
Thanks, Axel
On 11.03.2014 20:25, <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote: </pre>
<blockquote type="cite">
<pre wrap="">On 3/11/14 12:05 PM, Staffan Larsen wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 11 mar 2014, at 16:48, Siebenborn, Axel
<<a class="moz-txt-link-abbreviated" \
href="mailto:axel.siebenborn@sap.com">axel.siebenborn@sap.com</a> <a \
class="moz-txt-link-rfc2396E" \
href="mailto:axel.siebenborn@sap.com"><mailto:axel.siebenborn@sap.com></a>> \
wrote: </pre>
<blockquote type="cite">
<pre wrap="">Hi Seguei, I still can't upload files to the \
cr.openjdk server. Meanwhile, I use our server for the new webrev:
<a class="moz-txt-link-freetext" \
href="http://www.sapjvm.com/as/webrevs/8036666/">http://www.sapjvm.com/as/webrevs/8036666/</a>
Thanks, Axel
Comments inline:
On 11.03.2014 09:50,<a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> <a \
class="moz-txt-link-rfc2396E" \
href="mailto:serguei.spitsyn@oracle.com"><mailto:serguei.spitsyn@oracle.com></a>wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi Axel,
The webrev link is resolvable now. Thank you for taking
care about your broken account on the cr.openjdk server!
I have some comments on the test case ...
- This is nice test, thank you for providing it!
- The location of the test must be different as it is a
JVMTI test: test/serviceability/jvmti/8036666 instead of
test/runtime/8036666
</pre>
</blockquote>
<pre wrap="">I moved the test.
</pre>
</blockquote>
<pre wrap="">Tests should avoid the bug number in the name or path \
and instead use a descriptive name. See this page for some
background:
<a class="moz-txt-link-freetext" \
href="https://wiki.openjdk.java.net/display/HotSpot/Naming+HotSpot+JTReg+Tests">https://wiki.openjdk.java.net/display/HotSpot/Naming+HotSpot+JTReg+Tests</a>
</pre>
</blockquote>
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">The test files have already descriptive names.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">So, it would be enough to remove the bug number from the \
path. Sorry, I had to catch it too in the first place.
Thanks, Serguei
</pre>
<blockquote type="cite">
<pre wrap="">Thanks, /Staffan
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">RecursiveObjectLock,java:
- A suggestion to add a synchronized method (say,
nestedLock3) into the chain of calls started from the
testMethod. In order to do so, the class
RecursiveObjectLock needs to extend the ALock class. And
the this object needs to be used in the synchronized
statements and for wait()? What do you think about such
test enhancement for better coverage?
</pre>
</blockquote>
<pre wrap="">In order to have a synchronized method in the call \
chain, I synchronize on the "this" object.
</pre>
<blockquote type="cite">
<pre wrap="">GetObjectLockCount.java:
- The comment line 283 seems to be obsolete as the "param
out" is not present anymore:
283 * @param out Stream to copy to
- Could you, please, add e.printStackTrace() into the catch
statements at the lines 232 and 300?
- A question: It seems the errThread and outThread are
started a little bit late. Would it be better to start them
earlier, or it was intentional?
</pre>
</blockquote>
<pre wrap="">You're right! I moved to code up.
</pre>
<blockquote type="cite">
<pre wrap="">Some minor style-related comments (I hope, it is \
easy to fix this before push): - Unneeded extra empty lines:
149, 174-175, 244 - A space is missed before the '{':
180, 242, 243, 246 - Unneeded extra space after and before
the "(": 235, 297 - The curly brackets '{' do not follow
the common style: 142, 144
</pre>
</blockquote>
<pre wrap="">I hope I fixed them all and added no new style \
violations. Do you have a tool to check this?
</pre>
<blockquote type="cite">
<pre wrap="">We still need another reviewer for this fix. I'm \
ready to be a sponsor for it.
Thanks, Serguei
On 3/10/14 12:00 AM, <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> <a \
class="moz-txt-link-rfc2396E" \
href="mailto:serguei.spitsyn@oracle.com"><mailto:serguei.spitsyn@oracle.com></a> \
wrote: </pre>
<blockquote type="cite">
<pre wrap="">Hi Axel,
The webrev link does not work now. I'll check it again
tomorrow.
Thanks, Serguei
On 3/7/14 1:32 AM, <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> <a \
class="moz-txt-link-rfc2396E" \
href="mailto:serguei.spitsyn@oracle.com"><mailto:serguei.spitsyn@oracle.com></a> \
wrote: </pre>
<blockquote type="cite">
<pre wrap="">Hi Axel,
Thank you for fixing this issue. I'm reviewing it. It
looks good in general, but a little bit more time is
needed to look at the tests.
Do you need a sponsor for pushing?
Thanks, Serguei
On 3/6/14 12:08 AM, Siebenborn, Axel wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi all,
could I have a review for the following change?
The recursive lock count for an object is not
correct, in cases, where a monitor is inflated after
recursive lightweight locks. In this case, the
recursion count is taken from the heavyweight
monitor, represented by the class ObjectMonitor.
ObjectMonitor::_recursions is the number of times
ObjectMonitor::enter() was called to acquire the lock
minus 1. This counter does not include the recursions
of lightweight locks, that happen before inflating
the monitor and is not equal to the recursion count
from a Java source level point of view.
I added a test to the webrev to reproduce the
problem.
The suggested fix is to call count_locked_objects,
even if there's a heavyweight monitor and get the
recursion count by iterating the vframes.
Bug:
<a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8036666">https://bugs.openjdk.java.net/browse/JDK-8036666</a>
Webrev:
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~asiebenborn/8036666/webrev/">http://cr.openjdk.java.net/~asiebenborn/8036666/webrev/</a>
<a class="moz-txt-link-rfc2396E" \
href="http://cr.openjdk.java.net/%7Easiebenborn/8036666/webrev/"><http://cr.openjdk.java.net/%7Easiebenborn/8036666/webrev/></a><a \
class="moz-txt-link-rfc2396E" \
href="http://cr.openjdk.java.net/%7Easiebenborn/8036666/webrev/"><http://cr.openjdk.java.net/%7Easiebenborn/8036666/webrev/></a>
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">Thanks,
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">
Axel
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
</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