<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 2013-01-15 14:32, Vitaly Davidovich wrote:<br>
    <blockquote
cite="mid:CAHjP37GPhmZzGrPUkiaeuabQRq3o1YtNXnj+4-Y4RD25+Vb-gQ@mail.gmail.com"
      type="cite">
      <p dir="ltr">Hi Jesper,</p>
      <p dir="ltr">Is NewRatio guaranteed to be non-zero when used
        inside recommended_heap_size?</p>
    </blockquote>
    As far as I can see, yes. It defaults to two and is never set to
    zero.<br>
    /Jesper<br>
    <br>
    <blockquote
cite="mid:CAHjP37GPhmZzGrPUkiaeuabQRq3o1YtNXnj+4-Y4RD25+Vb-gQ@mail.gmail.com"
      type="cite">
      <p dir="ltr">Thanks</p>
      <p dir="ltr">Sent from my phone</p>
      <div class="gmail_quote">On Jan 15, 2013 8:11 AM, "Jesper
        Wilhelmsson" <<a moz-do-not-send="true"
          href="mailto:jesper.wilhelmsson@oracle.com">jesper.wilhelmsson@oracle.com</a>>
        wrote:<br type="attribution">
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex">
          Jon,<br>
          <br>
          Thank you for looking at this! I share your concerns and I
          have moved the knowledge about policies to CollectorPolicy.
          set_heap_size() now simply asks the collector policy if it has
          any recommendations regarding the heap size.<br>
          <br>
          Ideally, since the code knows about young and old generations,
          I guess the new function "recommended_heap_size()" should be
          placed in GenCollectorPolicy, but then the code would have to
          be duplicated for G1 as well. However, CollectorPolicy already
          know about OldSize and NewSize so I think it is OK to put it
          there.<br>
          <br>
          Eventually I think that we should reduce the abstraction level
          in the generation policies and merge CollectorPolicy,
          GenCollectorPolicy and maybe even TwoGenerationCollectorPolicy
          and if possible G1CollectorPolicy, so I don't worry too much
          about having knowledge about the two generations in
          CollectorPolicy.<br>
          <br>
          <br>
          A new webrev is available here:<br>
          <a moz-do-not-send="true"
            href="http://cr.openjdk.java.net/%7Ejwilhelm/6348447/webrev.2/"
            target="_blank">http://cr.openjdk.java.net/~jwilhelm/6348447/webrev.2/</a><br>
          <br>
          Thanks,<br>
          /Jesper<br>
          <br>
          <br>
          <br>
          On 2013-01-14 19:00, Jon Masamitsu wrote:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            Jesper,<br>
            <br>
            I'm a bit concerned that set_heap_size() now knows about how<br>
            the CollectorPolicy uses OldSize and NewSize.   In the
            distant<br>
            past set_heap_size() did not know what kind of collector was<br>
            going to be used and probably avoided looking at those<br>
            parameters for that reason.  Today we know that a
            generational<br>
            collector is to follow but maybe you could hide that
            knowledge<br>
            in CollectorPolicy somewhere and have set_heap_size() call
            into<br>
            CollectorPolicy to use that information?<br>
            <br>
            Jon<br>
            <br>
            <br>
            On 01/14/13 09:10, Jesper Wilhelmsson wrote:<br>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              Hi,<br>
              <br>
              I would like a couple of reviews of a small fix for
              JDK-6348447 - Specifying -XX:OldSize crashes 64-bit VMs<br>
              <br>
              Webrev:<br>
              <a moz-do-not-send="true"
                href="http://cr.openjdk.java.net/%7Ejwilhelm/6348447/webrev/"
                target="_blank">http://cr.openjdk.java.net/~jwilhelm/6348447/webrev/</a><br>
              <br>
              Summary:<br>
              When starting HotSpot with an OldSize larger than the
              default heap size one will run into a couple of problems.
              Basically what happens is that the OldSize is ignored
              because it is incompatible with the heap size. A debug
              build will assert since a calculation on the way results
              in a negative number, but since it is a size_t an
              if(x<0) won't trigger and the assert catches it later
              on as incompatible flags.<br>
              <br>
              Changes:<br>
              I have made two changes to fix this.<br>
              <br>
              The first is to change the calculation in
              TwoGenerationCollectorPolicy::adjust_gen0_sizes so that it
              won't result in a negative number in the if statement.
              This way we will catch the case where the OldSize is
              larger than the heap size and adjust the OldSize instead
              of the young size. There are also some cosmetic changes
              here. For instance the argument min_gen0_size is actually
              used for the old generation size which was a bit confusing
              initially. I renamed it to min_gen1_size (which it already
              was called in the header file).<br>
              <br>
              The second change is in Arguments::set_heap_size. My
              reasoning here is that if the user sets the OldSize we
              should probably adjust the heap size to accommodate that
              OldSize instead of complaining that the heap is too small.
              We determine the heap size first and the generation sizes
              later on while initializing the VM. To be able to fit the
              generations if the user specifies sizes on the command
              line we need to look at the generation size flags a little
              already when setting up the heap size.<br>
              <br>
              Thanks,<br>
              /Jesper<br>
              <br>
            </blockquote>
          </blockquote>
          <br>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </body>
</html>