<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
Hi Tony,<br>
<br>
<div class="moz-cite-prefix">On 2015-06-25 15:29, Tony Printezis
wrote:<br>
</div>
<blockquote
cite="mid:etPan.558c022e.23da99d5.1215a@tw-mbp-tprintezis"
type="cite">
<style>body{font-family:Helvetica,Arial;font-size:13px}</style>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Bengt,</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Thanks for
looking at this again. See inline.</div>
<br>
<p class="airmail_on" style="color:#000;">On June 25, 2015 at
4:06:40 AM, Bengt Rutisson (<a moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>)
wrote:</p>
<div>
<blockquote type="cite" class="clean_bq" style="color: rgb(0, 0,
0); font-family: Helvetica, Arial; font-size: 13px;
font-style: normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: normal; orphans: auto;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div><br>
Hi Tony,<br>
<br>
No worries about the delay, I'm also quite busy with
other things here. ;)<br>
<br>
<div class="moz-cite-prefix">On 2015-06-24 20:41, Tony
Printezis wrote:<br>
</div>
<blockquote
cite="mid:etPan.558af9e9.4affa36.1215a@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color: rgb(0, 0,
0); margin: 0px;">New webrev that covers all the
suggestions:</div>
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color: rgb(0, 0,
0); margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color: rgb(0, 0,
0); margin: 0px;"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Etonyp/8086056/webrev.1/">http://cr.openjdk.java.net/~tonyp/8086056/webrev.1/</a></div>
</blockquote>
<br>
Thanks for providing a new webrev!<br>
<br>
Some comments:<br>
<br>
parNewGeneration.cpp<br>
<br>
The flag validation code on lines 886-912 can be
replaced by the recently added support for command line
verification. See JEP-245:<br>
<br>
JEP 245: Validate JVM Command-Line Flag Arguments<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://openjdk.java.net/jeps/245">http://openjdk.java.net/jeps/245</a><br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/rev/5bbf25472731">http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/rev/5bbf25472731</a><br>
<br>
You can simply specify a constraint function in the
definition of the flags in globals.hpp.</div>
</div>
</span></blockquote>
</div>
<p><br>
</p>
<p>Thanks for pointing me to this. Given the flags are manageable,
do the constraints fire when the flags are updated dynamically?
If they do, what happens if the contrain fails?</p>
</blockquote>
<br>
<br>
Well, this is very new to me too. :)<br>
<br>
The JEP says:<br>
<br>
"The range and constraints checks are done every time a flag
changes, as well as late in the JVM initialization routine (i.e., in
init_globals() after stubRoutines_init2()) at the time when all
flags have their final values set. We will continue to check the
manageable flags as long as the JVM runs."<br>
<br>
I haven't actively been using this myself, but I assume that the
constraints will be checked every time a managaeble flag is changed
and that the one trying to change it will get an error reported back
if they try to change it in an invalid way.<br>
<br>
I'm copying Gerard on this email. He has built this support and can
surely answer the details.<br>
<br>
<blockquote
cite="mid:etPan.558c022e.23da99d5.1215a@tw-mbp-tprintezis"
type="cite">
<p><br>
</p>
<div>
<div>
<div>
<blockquote type="cite" class="clean_bq" style="color:
rgb(0, 0, 0); font-family: Helvetica, Arial; font-size:
13px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal; line-height:
normal; orphans: auto; text-align: start; text-indent:
0px; text-transform: none; white-space: normal; widows:
auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div><br>
I have a very hard time following all the type
conversions going on in adjust_cards_per_stride().<br>
<br>
The capacity flags are size_t, the
DynamicParGCStrides[Min/Max]Size flags are uintx, we
then convert the result through intptr_t,<span
class="Apple-converted-space"> </span></div>
</div>
</span></blockquote>
</div>
<p><br>
</p>
<p>(to my defense!) I had to convert to intptr_t to use the
log2_intptr() function. And I assume the stride size values
will be relatively small (i.e., maybe several K). So,
casting to intptr_t should be OK. But I do like to contrain
things as much as possible. Should we add an artificial
upper bound just in case?</p>
<p><br>
</p>
<div>
<div>
<blockquote type="cite" class="clean_bq" style="color:
rgb(0, 0, 0); font-family: Helvetica, Arial; font-size:
13px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div>int and uintx to eventually assign back to
ParGCCardsPerStrideChunk as intx.</div>
</div>
</span></blockquote>
</div>
<p><br>
</p>
<p>So, I was going for the capacities being size_t and the
stride sizes being uintx. ParGCCardsPerStrideChunk is intx
and I thought that uintx would be more appropriate as it
cannot be negative. I suggested to also make
ParGCCardsPerStrideChunk uintx but was I encouraged
against it. To keep the calculations simpler I’ll cast
everything to size_t locally and use log2_long() instead
of log2_intrptr(). </p>
<p>What do I do with the types of the args? Let’s go for
consistency: if you guys want me to leave
ParGCCardsPerStrideChunk as intx, I’ll leave everything as
intx. My preference would be to change everything to
uintx.</p>
<p><br>
</p>
<div>
<blockquote type="cite" class="clean_bq" style="color:
rgb(0, 0, 0); font-family: Helvetica, Arial; font-size:
13px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div><span class="Apple-converted-space"> </span>The
value of ParGCCardsPerStrideChunk is then used in
the code to offset pointers of the jbyte* type.
How can we be sure that this works correctly?</div>
</div>
</span></blockquote>
</div>
</div>
</div>
<p><br>
</p>
<p>I’d feel better if it was uintx. I’ll also check what happens
if the arg is not aligned properly.</p>
</div>
</blockquote>
<br>
This all sounds good. I need to see the new webrev to comment in
detail, but in general I just think we should try to get rid of as
much casting as we possibly can.<br>
<br>
I wouldn't have minded changing ParGCCardsPerStrideChunk to uintx or
size_t. But I read your comment to Thomas. I think it is fine to
leave that for another change if you prefer.<br>
<br>
<blockquote
cite="mid:etPan.558c022e.23da99d5.1215a@tw-mbp-tprintezis"
type="cite">
<div>
<p><br>
</p>
<div>
<div>
<div>
<blockquote type="cite" class="clean_bq" style="color:
rgb(0, 0, 0); font-family: Helvetica, Arial; font-size:
13px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div><br>
globalDefinitions.hpp<br>
<br>
I don't think that globalDefinitions.hpp is the
right place to put linearly_interpolate_bounded().
I would prefer to have it as a local method in
parNewGeneration.cpp.<span
class="Apple-converted-space"> </span>Or if you
think it is actually going to be used more widely
I think it should go somewhere in the utilities/
directory.</div>
</div>
</span></blockquote>
</div>
<p><br>
</p>
<p>Well, it is a general-purpose method, if you want to do
such an interpolation. Whether it will be needed by
anything else, I don’t know. If you don’t think I should
package it up in this general-purpose way, I’ll just go
back to what I had before and not factor it out, as the
ParNewGeneration class would not be the right place for it
either.</p>
</div>
</div>
</div>
</blockquote>
<br>
Given that you can remove all the flag checking code I don't think I
mind inining the logic inside
ParNewGeneration::adjust_cards_per_stride() again.<br>
<br>
<blockquote
cite="mid:etPan.558c022e.23da99d5.1215a@tw-mbp-tprintezis"
type="cite">
<div>
<div>
<div>
<div><br class="Apple-interchange-newline">
</div>
</div>
<div>
<div>
<blockquote type="cite" class="clean_bq" style="color:
rgb(0, 0, 0); font-family: Helvetica, Arial; font-size:
13px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div>Does it really make sense to use different
types for X and Y in
linearly_interpolate_bounded()?<span
class="Apple-converted-space"> </span></div>
</div>
</span></blockquote>
</div>
<p><br>
</p>
<p>I think it does. The two ranges are separate and don’t
have to be of the same type. In this case, you can have
the strides size as, say, ushort and the capacity as
size_t and it’d work. Note that the ratio is calculated
based on one range and then applied to the other range so
I think the calculation should be safe for separate types.</p>
</div>
</div>
</div>
</blockquote>
<br>
Ok.<br>
<br>
<blockquote
cite="mid:etPan.558c022e.23da99d5.1215a@tw-mbp-tprintezis"
type="cite">
<div>
<div>
<div>
<p><br>
</p>
<div>
<div>
<blockquote type="cite" class="clean_bq" style="color:
rgb(0, 0, 0); font-family: Helvetica, Arial;
font-size: 13px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div>I would think that one type is enough. Also,
it does not look safe to just cast the type to
double for a general purpose template method.</div>
</div>
</span></blockquote>
</div>
<p><br>
</p>
<p>Why (and how else would you do this)? This should be
safe for numetic types, which X and Y will be.</p>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>
Right, as long as they are primitive numeric types it will work. And
I guess that's most likely what you'd use. The template code itself
would otherwise work with a class for for example BitIntegers if it
had overloaded the comparison and numeric operators. But that's a
bit far fetched I guess. ;)<br>
<br>
Anyway, if you inline this code again you will just be casting a
size_t to a double which should be safe. So, the discussion is kind
of moot.<br>
<br>
Finally, just a heads up. I will be on vacation for the coming three
weeks. I doubt that I will be able to review any new version during
that time. If you get reviews from others I'm perfectly fine with
this being pushed. Depending on how different the changes that you
push are compared to the versions I've reviewed I'll leave it up to
you to decide whether you want to list me as a reviewer or not. I'm
fine either way. It's not a problem to leave me out of the reviewer
list if you are unsure.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote
cite="mid:etPan.558c022e.23da99d5.1215a@tw-mbp-tprintezis"
type="cite">
<div>
<div>
<div>
<div>
<p>Tony</p>
<p><br>
</p>
<div>
<blockquote type="cite" class="clean_bq" style="color:
rgb(0, 0, 0); font-family: Helvetica, Arial;
font-size: 13px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;"><span>
<div bgcolor="#FFFFFF" text="#000000">
<div><br>
Thanks,<br>
Bengt<br>
<br>
<blockquote
cite="mid:etPan.558af9e9.4affa36.1215a@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;">(and I also
found a version of hg that works OK with
webrev; thanks to Jon Masa for the help)</div>
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;">I’m not really
happy with the way I deal with the 32-bit
case. Better suggestions are welcome. Also,
I’m not attached to what the cmd line args
are called. Better suggestions on that are
welcome too.</div>
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family:
Helvetica, Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;">Tony</div>
<br>
<p class="airmail_on" style="color: rgb(0, 0,
0);">On June 23, 2015 at 12:19:12 PM, Tony
Printezis (<a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a>)
wrote:</p>
<blockquote type="cite" class="clean_bq">
<div style="word-wrap: break-word;
-webkit-nbsp-mode: space;
-webkit-line-break: after-white-space;">
<div>
<div id="bloop_customfont"
style="font-family: Helvetica, Arial;
font-size: 13px; color: rgb(0, 0, 0);
margin: 0px;"><span>Hi Bengt,</span></div>
<div id="bloop_customfont"
style="font-family: Helvetica, Arial;
font-size: 13px; color: rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div id="bloop_customfont"
style="font-family: Helvetica, Arial;
font-size: 13px; color: rgb(0, 0, 0);
margin: 0px;"><span>I’ll try factoring
out the interpolation and tell me
what you think. Sorry, I’m a bit
behind on this as I’ve been busy
with a couple of more urgent tasks
hee. I’ll get this done shortly.</span></div>
<div id="bloop_customfont"
style="font-family: Helvetica, Arial;
font-size: 13px; color: rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div id="bloop_customfont"
style="font-family: Helvetica, Arial;
font-size: 13px; color: rgb(0, 0, 0);
margin: 0px;"><span>Tony</span></div>
<span><br>
</span>
<p class="airmail_on" style="color:
rgb(0, 0, 0);"><span>On June 22, 2015
at 3:36:10 AM, Bengt Rutisson (<a
moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>)
wrote:</span></p>
<blockquote type="cite" class="clean_bq">
<div bgcolor="#FFFFFF" text="#000000">
<div><span><span><br>
Hi Tony,<br>
<br>
I agree with your comments.
Thanks for fixing this!<br>
<br>
One answer at the end of this
message...<br>
<br>
</span></span>
<div class="moz-cite-prefix"><span>On
2015-06-18 15:50, Tony
Printezis wrote:<br>
</span></div>
<blockquote
cite="mid:etPan.5582cc8d.1bcb7375.1215a@tw-mbp-tprintezis"
type="cite">
<div id="bloop_customfont"
style="font-family: Helvetica,
Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;"><span>Hi
Bengt,</span></div>
<div id="bloop_customfont"
style="font-family: Helvetica,
Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;"><span><br>
</span></div>
<div id="bloop_customfont"
style="font-family: Helvetica,
Arial; font-size: 13px; color:
rgb(0, 0, 0); margin: 0px;"><span>Thanks
for looking at it. Inline.</span></div>
<span><br>
</span>
<p class="airmail_on"
style="color: rgb(0, 0, 0);"><span>On
June 18, 2015 at 3:02:45 AM,
Bengt Rutisson (<a
moz-do-not-send="true"
href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>)
wrote:</span></p>
<div>
<blockquote type="cite"
class="clean_bq"
style="color: rgb(0, 0, 0);
font-family: Helvetica,
Arial; font-size: 13px;
font-style: normal;
font-variant: normal;
font-weight: normal;
letter-spacing: normal;
line-height: normal;
orphans: auto; text-align:
start; text-indent: 0px;
text-transform: none;
white-space: normal; widows:
auto; word-spacing: 0px;
-webkit-text-stroke-width:
0px;">
<div bgcolor="#FFFFFF"
text="#000000">
<div>
<div
class="moz-cite-prefix"><span><span><br>
Hi Tony,<br>
<br>
NIce to hear from
you!<br>
<br>
<br>
On 18/06/15 00:30,
Tony Printezis
wrote:<br>
</span></span></div>
<blockquote
cite="mid:etPan.5581f51b.699a1ea0.1215a@tw-mbp-tprintezis"
type="cite">
<div
id="bloop_customfont"
style="font-family:
Helvetica, Arial;
font-size: 13px;
color: rgb(0, 0, 0);
margin: 0px;"><span>Hi
all,</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica, Arial;
font-size: 13px;
color: rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica, Arial;
font-size: 13px;
color: rgb(0, 0, 0);
margin: 0px;"><span>A
small patch for
your
consideration:</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica, Arial;
font-size: 13px;
color: rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica, Arial;
font-size: 13px;
color: rgb(0, 0, 0);
margin: 0px;"><span><a
moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Etonyp/8086056/webrev.0/">http://cr.openjdk.java.net/~tonyp/8086056/webrev.0/</a></span></div>
</blockquote>
<span><br>
A couple of style
comments:<br>
<br>
891 const size_t
MinOldGenCapacity
= G;<br>
892 const size_t
MaxOldGenCapacity = 16
* G;<br>
893
assert(MinOldGenCapacity
<=
MaxOldGenCapacity,
"sanity");<br>
<br>
Local variables are
normmaly named using
lower case and
underscore.<span
class="Apple-converted-space"> </span></span></div>
</div>
</blockquote>
</div>
<p><br>
</p>
<p>Yeah, I of course usually do
that. But as they are
constants I thought I’d make
them look like constants. :-)</p>
<p><br>
</p>
<div>
<div>
<div>
<blockquote type="cite"
class="clean_bq"
style="color: rgb(0, 0,
0); font-family:
Helvetica, Arial;
font-size: 13px;
font-style: normal;
font-variant: normal;
font-weight: normal;
letter-spacing: normal;
line-height: normal;
orphans: auto;
text-align: start;
text-indent: 0px;
text-transform: none;
white-space: normal;
widows: auto;
word-spacing: 0px;
-webkit-text-stroke-width:
0px;">
<div bgcolor="#FFFFFF"
text="#000000">
<div><span>So,
min_old_gen_capacity
and
max_old_gen_capacity.
Same for the rest
of the local
variables in
ParNewGeneration::adjust_cards_per_stride().<br>
<br>
Having said that,
I think it would
be good to turn
some of these
constants into
flags (which means
that the naming
should be as it is
now ;) ).<span
class="Apple-converted-space"> </span></span></div>
</div>
</blockquote>
</div>
<p><br>
</p>
<p>Hang on! Are you and Masa
encourating me to add 5
(!!!) more cmd line args
to HotSpot? ;-)</p>
<p><br>
</p>
<div>
<blockquote type="cite"
class="clean_bq"
style="color: rgb(0, 0,
0); font-family:
Helvetica, Arial;
font-size: 13px;
font-style: normal;
font-variant: normal;
font-weight: normal;
letter-spacing: normal;
line-height: normal;
orphans: auto;
text-align: start;
text-indent: 0px;
text-transform: none;
white-space: normal;
widows: auto;
word-spacing: 0px;
-webkit-text-stroke-width:
0px;">
<div bgcolor="#FFFFFF"
text="#000000">
<div><span>Especially
since you say that
you haven't done
much performance
testing of these
values.</span></div>
</div>
</blockquote>
</div>
</div>
<p><br>
</p>
<p>To be clear: I did
performance testing with
chunks sizes in the 256-8k
range and with three old gen
sizes (2g, 10g, and 20g) and
auto-tuning comes pretty
close to optimal; I have
numbers on the JIRA. But,
yes, that was only with a
couple of (synthetic) tests
and only on one architecture
(Linux/x64). If someone
there could try this on a
couple more apps and maybe
on sparc, as we don’t happen
to have any sparc boxes
lying around :-), it’d be
helpful.</p>
<p>But, yes, your suggestion
of making the chunk size /
old gen capacity limits into
cmd line args definitely
makes sense and I had also
considered that myself. But
I had decided against it as
I thought you wouldn’t want
more cmd line args. :-) How
about:</p>
<p>+UseDynamicParGCStrides</p>
<p>DynamicParGCStrides{Min,Max}OldGenCapacity</p>
<p>DynamicParGCStrides{Min,Max}Size</p>
<p>I should also make them all
manageable.</p>
<p><br>
</p>
<div>
<div>
<blockquote type="cite"
class="clean_bq"
style="color: rgb(0, 0,
0); font-family:
Helvetica, Arial;
font-size: 13px;
font-style: normal;
font-variant: normal;
font-weight: normal;
letter-spacing: normal;
line-height: normal;
orphans: auto;
text-align: start;
text-indent: 0px;
text-transform: none;
white-space: normal;
widows: auto;
word-spacing: 0px;
-webkit-text-stroke-width:
0px;">
<div bgcolor="#FFFFFF"
text="#000000">
<div><span><span
class="Apple-converted-space"> </span>I
would prefer that
we could adjust
them at runtime.
Best would of
course be if you
could take the
time to do the
performance
testing.</span></div>
</div>
</blockquote>
</div>
<p><br>
</p>
<p>So, as I said earlier, I
did. The numbers are on
the JIRA. Is there
anything else I could
maybe run? Maybe larger
old gen than 20g? (but
that’s reaching the memory
limits of my workstation,
FWIW)</p>
<p><br>
</p>
<div>
<div>
<blockquote type="cite"
class="clean_bq"
style="color: rgb(0,
0, 0); font-family:
Helvetica, Arial;
font-size: 13px;
font-style: normal;
font-variant: normal;
font-weight: normal;
letter-spacing:
normal; line-height:
normal; orphans: auto;
text-align: start;
text-indent: 0px;
text-transform: none;
white-space: normal;
widows: auto;
word-spacing: 0px;
-webkit-text-stroke-width:
0px;">
<div bgcolor="#FFFFFF"
text="#000000">
<div><span><br>
I realize that
the assert on
line 893 might
have been useful
if you were
playing around
with the numbers
when you were
experimenting
with the patch.
But now it looks
mostly like line
noise to me.
Same with the
assert on line
898.</span></div>
</div>
</blockquote>
</div>
<p><br>
</p>
<p>Actually, it as
basically stating the
invariant for those
parameters (in case
someone wants to play
around with them). But
if we make the constants
into cmd line args this
will go away...</p>
<p><br>
</p>
<div>
<div>
<blockquote
type="cite"
class="clean_bq"
style="color: rgb(0,
0, 0); font-family:
Helvetica, Arial;
font-size: 13px;
font-style: normal;
font-variant:
normal; font-weight:
normal;
letter-spacing:
normal; line-height:
normal; orphans:
auto; text-align:
start; text-indent:
0px; text-transform:
none; white-space:
normal; widows:
auto; word-spacing:
0px;
-webkit-text-stroke-width:
0px;">
<div
bgcolor="#FFFFFF"
text="#000000">
<div><span><br>
I would also
consider
factoring the
actual
computation
out into a
separate
helper method.</span></div>
</div>
</blockquote>
</div>
<p><br>
</p>
<p>You mean the
interpolation? Like:</p>
<p>(base_min, base_max,
value_min, value_max,
value) -> result in
range [base_min,
base_max]?</p>
</div>
</div>
</div>
</div>
</blockquote>
<br>
Yes, that's what I meant. But if
you, as suggested above, get rid
of most (all?) of the constant
definitions in
adjust_cards_per_stride() then I
guess it is mostly only the
interpolation left. So, in that
case it may not be necessary to
factor it out. I would have to see
a new webrev to have more
comments.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote
cite="mid:etPan.5582cc8d.1bcb7375.1215a@tw-mbp-tprintezis"
type="cite">
<div>
<div>
<div>
<div>
<p>Tony</p>
<p><br>
</p>
<div>
<blockquote
type="cite"
class="clean_bq"
style="color: rgb(0,
0, 0); font-family:
Helvetica, Arial;
font-size: 13px;
font-style: normal;
font-variant:
normal; font-weight:
normal;
letter-spacing:
normal; line-height:
normal; orphans:
auto; text-align:
start; text-indent:
0px; text-transform:
none; white-space:
normal; widows:
auto; word-spacing:
0px;
-webkit-text-stroke-width:
0px;">
<div
bgcolor="#FFFFFF"
text="#000000">
<div><span><br>
I also second
Jon's comment
on that it
would be good
with a more
explicit way
of turning
this feature
off.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
</span>
<blockquote
cite="mid:etPan.5581f51b.699a1ea0.1215a@tw-mbp-tprintezis"
type="cite">
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span>(BTW,
for some
reason some of
the webrev
output is a
bit messed up.
Not sure why,
maybe some hg
incompatibility
I guess. The
diffs look OK
though. I also
attached the
patch to this
e-mail.)</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span>There’s
a bit of info
on the JIRA on
the rationale
for the patch:</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span><a
moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8086056">https://bugs.openjdk.java.net/browse/JDK-8086056</a></span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span>The
min / max
values for the
old gen
capacity
and ParGCCardsPerStrideChunk
were chosen
empircally
after running
a few (mostly
synthetic
tests) on
Linux x64. If
someone has
the cycles to
do a more
extensive
performance
study, I’d be
happy to
revise them
accordingly.</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span>Regards,</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span><br>
</span></div>
<div
id="bloop_customfont"
style="font-family:
Helvetica,
Arial;
font-size:
13px; color:
rgb(0, 0, 0);
margin: 0px;"><span>Tony</span></div>
<span><br>
</span>
<div
id="bloop_sign_1434579909530055936"
class="bloop_sign">
<div
style="font-family:
helvetica,
arial;
font-size:
13px;">
<div><span>-----</span></div>
<div><span><br>
</span></div>
<div><span>Tony
Printezis |
JVM/GC
Engineer / VM
Team | Twitter</span></div>
<div><span><br>
</span></div>
<div><span>@TonyPrintezis</span></div>
<div><span><a
moz-do-not-send="true" href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></span></div>
<div><span><br>
</span></div>
</div>
</div>
</blockquote>
<span><br>
</span></div>
</div>
</blockquote>
<br
class="Apple-interchange-newline">
</div>
<br
class="Apple-interchange-newline">
</div>
<br
class="Apple-interchange-newline">
</div>
<br
class="Apple-interchange-newline">
</div>
<br
class="Apple-interchange-newline">
</div>
<div
id="bloop_sign_1434634117464453888"
class="bloop_sign">
<div style="font-family:
helvetica, arial; font-size:
13px;">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC
Engineer / VM Team |
Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a
moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</blockquote>
<div id="bloop_sign_1435076298751951872"
class="bloop_sign">
<div style="font-family: helvetica,
arial; font-size: 13px;">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC
Engineer / VM Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<div id="bloop_sign_1435171154189988096"
class="bloop_sign">
<div style="font-family: helvetica, arial;
font-size: 13px;">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC Engineer / VM
Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
</span></blockquote>
<br class="Apple-interchange-newline">
</div>
<br class="Apple-interchange-newline">
</div>
<br class="Apple-interchange-newline">
</div>
<br class="Apple-interchange-newline">
</div>
<br class="Apple-interchange-newline">
</div>
<div id="bloop_sign_1435237738450878976" class="bloop_sign">
<div style="font-family:helvetica,arial;font-size:13px">
<div>-----</div>
<div><br>
</div>
<div>Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div>
<div><br>
</div>
<div>@TonyPrintezis</div>
<div><a moz-do-not-send="true"
href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>