<div dir="ltr">Hi Erik,<div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 24, 2016 at 4:01 PM, Erik Helin <span dir="ltr"><<a href="mailto:erik.helin@oracle.com" target="_blank">erik.helin@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_-1084605148404323272HOEnZb"><div class="m_-1084605148404323272h5">On 2016-10-13, Thomas Stüfe wrote:<br>
> Hi Erik,<br>
><br>
> On Thu, Oct 13, 2016 at 2:15 PM, Erik Helin <<a href="mailto:erik.helin@oracle.com" target="_blank">erik.helin@oracle.com</a>> wrote:<br>
><br>
> > Hi Thomas,<br>
> ><br>
> > thanks for submitting the JEP and proposing this feature!<br>
> ><br>
> > On 2016-10-10, Thomas Stüfe wrote:<br>
> > > Hi all,<br>
> > ><br>
> > > May I have please some feedback for this enhancement proposal?<br>
> > ><br>
> > > <a href="https://bugs.openjdk.java.net/browse/JDK-8166690" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/<wbr>browse/JDK-8166690</a><br>
> > ><br>
> > ><br>
> > > In one very short sentence it proposes a better allocation scheme for<br>
> > > Metaspace Chunks in order to reduce fragmentation and metaspace waste.<br>
> > ><br>
> > > I also added a short presentation which describes the problem and how we<br>
> > > solved it in our VM port.<br>
> > ><br>
> > > <a href="https://bugs.openjdk.java.net/secure/attachment/63894/" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/<wbr>secure/attachment/63894/</a><br>
> > Metaspace%20Coalescation%20in%<wbr>20the%20SAP%20JVM.pdf<br>
> ><br>
> > Do we really need the flag -XX:+CoalesceMetaspace? Having two differnent<br>
> > ways to handle the chunk free lists in Metaspace is unfortunate, it<br>
> > might introduce hard to detect bugs and will also require much more<br>
> > testing (runnings lots of tests with the flag both on and off).<br>
> ><br>
><br>
> You are right. If the new allocator works well, there is no reason to keep<br>
> the old allocator around.<br>
><br>
> We wanted for a temporary time to be able to switch between both old and<br>
> new allocator. Just to have a fallback if problems occur. But if it works,<br>
> it makes sense to only have one allocator, and the "CoalesceMetaspace" flag<br>
> can be removed, and also the code can be made a lot simpler because we do<br>
> not need both code paths.<br>
<br>
</div></div>Yeah, I would strongly prefer to not introduce a new flag for this. Have<br>
you thought about testing? Do you intend to write new tests to stress<br>
the coalescing?<br></blockquote><div><br></div><div>The current version of my patch contains a lot of verification code, which is activated by default</div><div>for the debug case. Nightly we run lots of test suites and benchmarks on all our platforms,</div><div>so the patch already got stressed a lot. We also have tests specific for the metaspace.</div><div><br></div><div>The patch only makes sense with thorough testing, so I consider writing tests just a part of</div><div>implementing the JEP.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><br>
> ><br>
> > Do you think your proposed solution has low enough overhead (in terms<br>
> > of CPU and memory) to be on "by default"?<br>
> ><br>
><br>
> We decided to switch it on by default in our VM.<br>
><br>
> Memory overhead can be almost exactly calculated. Bitmasks take 2 bits per<br>
> specialized-chunk-sized-area. That means, for specialized-chunk-size = 1k<br>
> (128 meta words): metaspace size / 8192. So, for 1G of metaspace we pay<br>
> 132KB overhead for the bitmasks, or roughly 0.1%.<br>
><br>
> There is some CPU overhead, but in my tests I could not measure anything<br>
> above noise level.<br>
<br>
</span>Those numbers seems low enough to me in order to not warrant a new flag.<br>
<span><br>
> > Thanks,<br>
> > Erik<br>
> ><br>
> ><br>
> Btw, I understand that it is difficult to estimate this proposal without a<br>
> prototype to play around. As I already mentioned, the patch right now only<br>
> exists in our code base and not yet in the OpenJDK. If you guys are<br>
> seriously interested in this JEP, I will invest the time to port the patch<br>
> to the OpenJDK, so that you can check it out for yourself.<br>
<br>
</span>Yes, we are seriously interested :) I think the proposal sounds good. I guess<br>
the devil will be in the details, so I (we) would really appreciate if<br>
you want to port your internal patch to OpenJDK.<br>
<br></blockquote><div><br></div><div>Ok, thanks Erik. I will implement a prototype then and come back to you once it is done.</div><div><br></div><div>Kind Regards, Thomas</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks,<br>
Erik<br>
<div class="m_-1084605148404323272HOEnZb"><div class="m_-1084605148404323272h5"><br>
> Kind Regards, Thomas<br>
><br>
><br>
><br>
><br>
> > > Thank you very much!<br>
> > ><br>
> > > Kind Regards, Thomas<br>
> > ><br>
> > ><br>
> > > On Tue, Sep 27, 2016 at 10:45 AM, Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" target="_blank">thomas.stuefe@gmail.com</a>><br>
> > > wrote:<br>
> > ><br>
> > > > Dear all,<br>
> > > ><br>
> > > > please take a look at this Enhancement Proposal for the metaspace<br>
> > > > allocator. I hope these are the right groups for this discussion.<br>
> > > ><br>
> > > > <a href="https://bugs.openjdk.java.net/browse/JDK-8166690" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/<wbr>browse/JDK-8166690</a><br>
> > > ><br>
> > > > Background:<br>
> > > ><br>
> > > > We at SAP see at times at customer installations OOMs in Metaspace<br>
> > > > (usually, with compressed class pointers enabled, in Compressed Class<br>
> > > > Space). The VM attempts to allocate metaspace and fails, hitting the<br>
> > > > CompressedClassSpaceSize limit. Note that we usually set the limit<br>
> > lower<br>
> > > > than the default, typically at 256M.<br>
> > > ><br>
> > > > When analyzing, we observed that a large part of the metaspace is<br>
> > indeed<br>
> > > > free but "locked in" into metaspace chunks of the wrong size: often we<br>
> > > > would find a lot of free small chunks, but the allocation request was<br>
> > for<br>
> > > > medium chunks, and failed.<br>
> > > ><br>
> > > > The reason was that if at some point in time a lot of class loaders<br>
> > were<br>
> > > > alive, each with only a few small classes loaded. This would lead to<br>
> > the<br>
> > > > metaspace being swamped with lots of small chunks. This is because each<br>
> > > > SpaceManager first allocates small chunks, only after a certain amount<br>
> > of<br>
> > > > allocation requests switches to larger chunks.<br>
> > > ><br>
> > > > These small chunks are free and wait in the freelist, but cannot be<br>
> > reused<br>
> > > > for allocation requests which require larger chunks, even if they are<br>
> > > > physically adjacent in the virtual space.<br>
> > > ><br>
> > > > We (at SAP) added a patch which allows on-the-fly metaspace chunk<br>
> > merging<br>
> > > > - to merge multiple adjacent smaller chunk to form a larger chunk.<br>
> > This, in<br>
> > > > combination with the reverse direction - splitting a large chunk to get<br>
> > > > smaller chunks - partly negates the "chunks-are-locked-in-into-<br>
> > their-size"<br>
> > > > limitation and provides for better reuse of metaspace chunks. It also<br>
> > > > provides better defragmentation as well.<br>
> > > ><br>
> > > > I discussed this fix off-list with Coleen Phillimore and Jon Masamitsu,<br>
> > > > and instead of just offering this as a fix, both recommended to open a<br>
> > JEP<br>
> > > > for this, because its scope would be beyond that of a simple fix.<br>
> > > ><br>
> > > > So here is my first JEP :) I hope it follows the right form. Please, if<br>
> > > > you have time, take a look and tell us what you think.<br>
> > > ><br>
> > > > Thank you, and Kind Regards,<br>
> > > ><br>
> > > > Thomas Stüfe<br>
> > > ><br>
> > > ><br>
> > > ><br>
> > > ><br>
> ><br>
</div></div></blockquote></div><br></div></div>