<div dir="ltr">Hi Thomas and Thomas,<div><br></div><div>Thanks for the reviews!</div><div><br></div><div>@Thomas Schatzl: I looked for using a MXBean but I'm confused about using this. What this fix+test is trying to ascertain is that the moment of the logging of the metaspace data, the new size has been calculated; how do I do that with a MXBean? Would I not always get the current size? So if I did before FullGC, I'd get the previous size, after FullGC, I'd always get the new size, no? I am most likely missing something and I apologize!<br></div><div><br></div><div>@Thomas Stufe: I think that is exactly what we could (should?) use; however there are a few questions here:<br></div><div>   1) There is no license to that code, could you add one so that we can see if it is compatible for the OpenJDK project?</div><div>   2) Will bringing this code in for this test be useful for future/other tests? </div><div>   3) If so, I would actually recommending making a RFE for the test to bring in similar code like that and remove my testcase.jar. That way this bug focuses solely on just getting the print out right with a test and we can refactor it in a second step with a useful common library that can come it slowly with reviews that are concentrating on that library instead, what do you think?</div><div><br></div><div>Thanks,<br></div><div>Jc</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 27, 2018 at 7:17 AM Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com">thomas.stuefe@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
On Thu, Sep 27, 2018 at 2:09 PM, Thomas Schatzl<br>
<<a href="mailto:thomas.schatzl@oracle.com" target="_blank">thomas.schatzl@oracle.com</a>> wrote:<br>
> Hi JC,<br>
><br>
> On Wed, 2018-09-26 at 19:21 -0700, JC Beyler wrote:<br>
>> Hi Thomas et al,<br>
>><br>
>> Thanks for moving this thread to a RFR. Here is the webrev and bug:<br>
>> Webrev: <a href="http://cr.openjdk.java.net/~jcbeyler/8211123/webrev.00/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~jcbeyler/8211123/webrev.00/</a><br>
>> Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8211123" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8211123</a><br>
>><br>
>> For the test, I inspired myself of a few different tests to get this<br>
>> up and running. I didn't find a means to use only the WhiteBox to get<br>
>> the metaspace filled. I did not want to use the .jar that the<br>
>> original reproducer was using since it was using a library. So I used<br>
>> the testcases.jar from the runtime/7116786 test.<br>
>><br>
>> I also use a separate process to test the various GCs and ensure it<br>
>> works for all.<br>
>><br>
>> Btw, I've put nijiaben as the contributor for the webrev.<br>
>><br>
>> Let me know what you think, I imagine I've made the test a bit more<br>
>> complicated than need be,<br>
>> Jc<br>
><br>
> I do not know a better way than having an external jar to do class<br>
> loading into a separate class loader in a test. It's safest to<br>
> duplicate the jar file.<br>
<br>
Just as a suggestion, I did tests - but never really got to jtreg-ify<br>
them - for JDK-8198423.<br>
<br>
They use javax.tools.JavaCompiler to compile classes on the fly and<br>
load them into separate class loaders.<br>
<br>
See e.g. <a href="https://github.com/tstuefe/ojdk-repros/blob/master/src/test3/Example2.java" rel="noreferrer" target="_blank">https://github.com/tstuefe/ojdk-repros/blob/master/src/test3/Example2.java</a><br>
<br>
(Not the prettiest program ever, but enough to demonstrate the point).<br>
<br>
Best, Thomas<br>
<br>
><br>
> The test could be simplified a bit by using the Metaspace MemoryMXBean<br>
> to get metaspace memory readings (there is an MXBean called<br>
> "Metaspace"), removing the need for invoking a secondary JVM in the<br>
> test and parsing the log :)<br>
><br>
> You can then run the test with the different collectors using<br>
> "main/othervm -XX:+<some-gc> MyTest" lines.<br>
><br>
> Otherwise the change itself looks good. I would appreciate if the test<br>
> could be improved as mentioned above, but if you do not want to I could<br>
> live with it as is.<br>
><br>
> Btw, you can have multiple contributors in that line (or probably<br>
> multiple Contributed-by lines).<br>
><br>
> Thanks,<br>
>   Thomas<br>
><br>
><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><br></div>Thanks,<div>Jc</div></div></div>