<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
Hi John,<br>
<br>
On 2011-11-18 18:11, John Cuthbertson wrote:
<blockquote cite="mid:4EC691AD.20901@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<title></title>
Hi Bengt,<br>
<br>
The .03 version looks good to me. One minor nit - can you place
parentheses around the MarkSweepAlwaysCompactCount < 1
expressions?
O know they're not required but, to my eye at least, thy help
separating the expressions used in the or.<br>
</blockquote>
<br>
Good point. I'll add the parentheses.<br>
<br>
All set to push this now. Thanks Ramki, Jon, John and Tony for the
reviews!<br>
<br>
Bengt<br>
<br>
<blockquote cite="mid:4EC691AD.20901@oracle.com" type="cite">
<br>
Thanks,<br>
<br>
JohnC<br>
<br>
On 11/17/11 23:53, Bengt Rutisson wrote:
<blockquote cite="mid:4EC60EDD.2010106@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<br>
Ramki,<br>
<br>
Thanks for the extra testing!<br>
<br>
Jon, John and Tony,<br>
<br>
Thanks for the reviews! Are you OK with pushing this (handle
values
< 1 as never force full compaction):<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.03/">http://cr.openjdk.java.net/~brutisso/7110718/webrev.03/</a><br>
<br>
Instead of this (forbid values < 1):<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.02/">http://cr.openjdk.java.net/~brutisso/7110718/webrev.02/</a><br>
<br>
I think your review comments were more related to webrev.02, so
I would
like to double check with you before I push my changes in
webrev.03.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
On 2011-11-18 08:11, Srinivas Ramakrishna wrote:
<blockquote
cite="mid:CABzyjymetvJfuZF_bTAh_sTc3Kc8BrfQmfUDRTTwCVBtxyFPmw@mail.gmail.com"
type="cite">Hi Bengt -- this looks good to me. It also passed
my
testing...<br>
I also confirmed what Jon indicated -- that
VerifyUpdateClosure is dead
code, as far as my cscope<br>
navigation showed and can be deleted (in a separate CR as Jon
said).<br>
<br>
Rebiewed; thanks!<br>
-- ramki<br>
<br>
<div class="gmail_quote">On Thu, Nov 17, 2011 at 6:13 AM,
Bengt
Rutisson <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px
solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex;
padding-left: 1ex;">
<div bgcolor="#ffffff" text="#000000"> <br>
Hi Ramki,<br>
<br>
Is this what you were considering?<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.03/"
target="_blank">http://cr.openjdk.java.net/~brutisso/7110718/webrev.03/</a><br>
<br>
I think it should interpret MarkSweepAlwaysCompactCount
<= 0 as
"never force full compaction". It would be great if you
could do some
testing. There is one assert in
PSParallelCompact::VerifyUpdateClosure::do_addr() that
worries me a bit:<br>
<br>
assert(HeapMaximumCompactionInterval > 1 ||
MarkSweepAlwaysCompactCount > 1 ||<br>
forwarding_ptr == new_pointer, "new location is
incorrect");<br>
<br>
I think this should be safe since
MarkSweepAlwaysCompactCount == 1 was
an acceptable value even before my change. But I have to
admit that I
don't really understand what the assert is trying to
verify.<br>
<font color="#888888"> <br>
Bengt</font>
<div>
<div class="h5"><br>
<br>
<br>
On 2011-11-16 19:01, Srinivas Ramakrishna wrote:
<blockquote type="cite">Hi Bengt, Not sure how much
customers use
this option. Its useful for "serviceability in
production" kind of
scenarios<br>
to have the code be more robust. I think it would
be useful. I
appreciate the need for more testing<br>
of course, and I am happy to do that testing for
you -- just let me
know and I'll grab yr patch and test<br>
here.<br>
<br>
thanks!<br>
-- ramki<br>
<br>
<div class="gmail_quote">On Tue, Nov 15, 2011 at
6:50 AM, Bengt
Rutisson <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com"
target="_blank">bengt.rutisson@oracle.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote"
style="border-left: 1px solid rgb(204, 204,
204); margin: 0pt 0pt 0pt 0.8ex; padding-left:
1ex;">
<div bgcolor="#FFFFFF" text="#000000"> <br>
Ramki,
<div><br>
<br>
On 2011-11-14 20:32, Srinivas Ramakrishna
wrote:
<blockquote type="cite"><br>
Thanks, Bengt, for the super-quick
turnaround!! A comment below on the
choice of <= 0 for the option
value....<br>
</blockquote>
<br>
</div>
Thanks for the review! See comments below.
<div><br>
<br>
<blockquote type="cite">
<div class="gmail_quote">On Mon, Nov 14,
2011 at 1:25 AM,
Bengt Rutisson <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com"
target="_blank">bengt.rutisson@oracle.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote"
style="border-left: 1px solid
rgb(204, 204, 204); margin: 0pt 0pt
0pt 0.8ex; padding-left: 1ex;"><br>
Hi all,<br>
<br>
Can I have a couple of reviews for
this small change?<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ebrutisso/7110718/webrev.01/"
target="_blank">http://cr.openjdk.java.net/~brutisso/7110718/webrev.01/</a><br>
<br>
It is a fix for the issue that Ramki
reported recently.
MarkSweepAlwaysCompactCount is used
for division and Hotspot crashes if
it is set to 0.<br>
<br>
I choose to log an error and exit
the VM if someone tries to start
with
-XX:MarkSweepAlwaysCompactCount=0.
An alternative is to just log a
warning and set it to 1.<br>
<br>
I prefer the error way since it is
not really clear what one wants to
achieve with
MarkSweepAlwaysCompactCount=0.
Always do full compactions
or never do full compactions? So I
am not convinced that 1 is an
appropriate value.<br>
<br>
Also, since the VM, up until now,
has crashed if someone tried
-XX:MarkSweepAlwaysCompactCount=0 I
think we can be sure that there are
no customers that are running with
that setting. It should be safe to
forbid it.<br>
</blockquote>
<div><br>
I agree with that statement.
However, given that the value 0 was
producing crashes, proving that no
production code would have been
using that setting,<br>
and based on yr comment above that
the 0 value could as well have been
used to denote "never force full
compaction", it seems as though an
alternative to exiting with<br>
an error, is now to define all
values <=0 to mean "never _force_
full compaction" <br>
<br>
Especially since tolerating allowed
input values and mapping them to
specific non-exiting behaviours
allows us to modify production JVM's
on
the fly<br>
without causing loss of
availability. (Consider a future in
which this
option becomes a "manageable"; you
would then be faced with the same<br>
question, and it seems as though
making this choice now would help
maintain consistency and robustness
going forward -- we could of course<br>
always throw a "illegal value
exception" or such at that point,
but
allowing the specification of "never
_force_ full compaction" (unless
the JVM<br>
otherwise chooses to) would appear
to be a choice to allow users;
mapping negative and 0 values to
that setting would avoid having to<br>
throw an error.) However, I
understand that this is somewhat
subjective, so I am willing to go
with whatever the majority consensus
here<br>
mght be. It just seemed more
pleasant to:<br>
(1) allow the specification of
reasonable behaviour (i.e. never
_force_
...)<br>
(2) map the full domain of the
option to a reasonable behaviour
(i.e.
allow <= 0 to map to never
_force_ ..)<br>
<br>
Comments?<br>
</div>
</div>
</blockquote>
<br>
</div>
I see your point, and I think this should be
fairly straight forward to
fix. However it will require some more
testing etc. I can do that, but
I don't think I know enough to say whether
or not the extra work is
worth it. How important is this option? Is
it something that customers
use a lot?<br>
<br>
Thanks,<br>
<font color="#888888"> Bengt</font>
<div><br>
<br>
<blockquote type="cite">
<div class="gmail_quote">
<div><br>
-- ramki<br>
<br>
<br>
</div>
<blockquote class="gmail_quote"
style="border-left: 1px solid
rgb(204, 204, 204); margin: 0pt 0pt
0pt 0.8ex; padding-left: 1ex;"> <br>
CR:<br>
<br>
7110718
-XX:MarkSweepAlwaysCompactCount=0
crashes the JVM<br>
<a moz-do-not-send="true"
href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7110718"
target="_blank">http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7110718</a><br>
<br>
Thanks,<br>
<font color="#888888"> Bengt<br>
</font></blockquote>
</div>
<br>
</blockquote>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>