<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Hi Jon,<br>
<br>
Thanks for looking at this!<br>
<br>
On 4/4/13 9:54 PM, Jon Masamitsu wrote:<br>
</div>
<blockquote cite="mid:515DDA75.1090707@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Bengt,<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebrutisso/7197666/webrev.01/src/share/vm/memory/allocation.inline.hpp.frames.html">http://cr.openjdk.java.net/~brutisso/7197666/webrev.01/src/share/vm/memory/allocation.inline.hpp.frames.html</a><br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre><span class="new"> 118 if (_use_malloc) {</span>
<span class="new"> 119 _addr = AllocateHeap(_size, F);</span>
<span class="new"> 120 return (E*)_addr;</span>
<span class="new"> 121 }
</span><span class="new"></span>
</pre>
Did you consider checking the value of _addr and going on<br>
to the mmap based allocation if the malloc() failed?<br>
</div>
</blockquote>
<br>
That's a good idea! I added that logic. I only try mmap if the size
is larger than one page. It seems to me that if we are failing
really small allocations we should not be trying to continue and if
we would start mmapping really small sizes we will waste a lot of
memory since mmapped memory needs to be page aligned.<br>
<br>
Here is an updated webrev including the fixed comment suggested by
John Cuthbertson too:<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~brutisso/7197666/webrev.02/">http://cr.openjdk.java.net/~brutisso/7197666/webrev.02/</a><br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:515DDA75.1090707@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Jon<br>
<pre><span class="new"></span></pre>
<br>
On 4/4/13 5:17 AM, Bengt Rutisson wrote:<br>
</div>
<blockquote cite="mid:515D6F44.4050909@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi all,<br>
<br>
Coleen and Thomas have already looked at the preliminary
version of this request. Thanks!<br>
<br>
Removing the preliminary part of this request now and asking
for full reviews.<br>
<br>
Here is an updated webrev based on the comments from Coleen
and Thomas:<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebrutisso/7197666/webrev.01/">http://cr.openjdk.java.net/~brutisso/7197666/webrev.01/</a><br>
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
On 3/28/13 11:09 PM, Bengt Rutisson wrote:<br>
</div>
<blockquote cite="mid:5154BF9A.2080208@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<br>
Hi all,<br>
<br>
Sending this to both runtime and GC since I think it concerns
both areas.<br>
<br>
I'd like some feedback on this preliminary change. I still
want to do some more testing and evaluation before I ask for
final reviews:<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebrutisso/7197666/webrev.00/">http://cr.openjdk.java.net/~brutisso/7197666/webrev.00/</a><br>
<br>
In particular I would like some feedback on these questions:<br>
<br>
- I am adding a flag that has the same value on all platforms
except Solaris x86. There is the product_pd flag macro to
support this. But there is no experimental_pd marcro. I would
have preferred to make my new flag experimental. Should I add
experimental_pd or should I just use a product flag?<br>
<br>
- Even with product_pd I think I still have to go in to all
the different platform files and add the exact same code to
give the flag a default value on all platforms. Is there a way
to have a default value and only override it on Solaris x86?<br>
<br>
- The class I am adding,
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
ArrayAllocator, wants to choose between doing malloc and mmap.
Normally we use ReservedSpace and VirtualSpace to get mapped
memory. However, those classes are kind of clumsy when I just
want to allocate one chunk of memory. It is much simpler to
use the
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
os::reserve_memory() and os::commit_memory() methods directly.
I think my use case here motivate using these methods
directly, but is there some reason not to do that?<br>
<br>
Some background on the change:<br>
<br>
The default implementation of malloc on Solaris has several
limitation compared to malloc on other platforms. One
limitation is that it can only use one consecutive chunk of
memory. Another limitation is that it always allocates in this
single chunk of memory no matter how large the requested
amount of memory is. Other malloc implementations normally use
mapped memory for large allocations.<br>
<br>
The Java heap is mapped in memory and we try to pick a good
address for it. The lowest allowed address is controlled by
HeapBaseMinAddress. This is only 256 MB on Solaris x86 (other
platforms have at least 2 GB). Since the C heap ends up below
the Java heap it means that in some cases it is limited to 256
MB.<br>
<br>
When we run with ParallelOldGC we get three task queues per GC
thread. Each task queue takes mallocs 1MB. The failing machine
in the bug report has lots of CPUs and ends up with 83 GC
threads. This is 249 MB, which is more than we can get out of
the 256 MB limited C heap considering that there are other
things that get malloced too.<br>
<br>
So, the problems occur mostly on Solaris x86. My suggested fix
tries to address this by letting the task queues be mapped
instead of malloced on Solaris x86. Instead of inlining this
logic in taskqueue.cpp I added a more general class. The
reason for this is that I think we need to use the same logic
in more places, especially for G1, which is mallocing quite a
lot.<br>
<br>
Since I think malloc on other platforms use mapped memory for
large malloc requests I think it is enough for this change to
have effect on Solaris. The other platforms probably have
better heuristics than I can come up with for which sizes
should be mapped. On Sparc we have the same limitation with
malloc, but we have more memory available for the C heap. This
is why I have only enabled this for Solaris x86.<br>
<br>
Also, I will be on vacation for a few days. Back in the office
Thrusday April 4. I'm happy for any feedback on this, but if I
don't respond until next week you know why :)<br>
<br>
Thanks,<br>
Bengt<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>