<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Thomas, JC<div class=""><br class=""></div><div class="">I tried to use a test fixture and the code became very clumsy, so I went w/ a simple RAII. </div><div class=""><a href="http://cr.openjdk.java.net/~iignatyev//8177709/webrev.0-1/index.html" class="">http://cr.openjdk.java.net/~iignatyev//8177709/webrev.0-1/index.html</a></div><div class=""><br class=""></div><div class="">I've also noticed that we have same problem in TestReservedSpace_test tests -- <a href="http://cr.openjdk.java.net/~iignatyev//8171097/webrev.0-1/index.html" class="">http://cr.openjdk.java.net/~iignatyev//8171097/webrev.0-1/index.html</a></div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">-- Igor</div><div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Oct 17, 2018, at 12:04 PM, Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" class="">thomas.stuefe@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class=""><br class=""></div><div class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed 17. Oct 2018 at 21:00, Igor Ignatyev <<a href="mailto:igor.ignatyev@oracle.com" class="">igor.ignatyev@oracle.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">EXPECT would just mark a test as failed and allow the method to continue, so rs.release() will be called. </blockquote><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Oh, okay. Thanks for clarifying.</div><div dir="auto" class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br class="">
<br class="">
test fixture will basically do the same as an RAII object, but it will be a bit more obvious as test preparation and cleaning steps will be separated from test actions, so I'll go w/ setUp/tearDown.</blockquote><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Okay fine by me.</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">..thomas</div><div dir="auto" class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br class="">
<br class="">
-- Igor<br class="">
<br class="">
> On Oct 17, 2018, at 11:53 AM, Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" target="_blank" class="">thomas.stuefe@gmail.com</a>> wrote:<br class="">
> <br class="">
> On Wed, Oct 17, 2018 at 8:29 PM Igor Ignatyev <<a href="mailto:igor.ignatyev@oracle.com" target="_blank" class="">igor.ignatyev@oracle.com</a>> wrote:<br class="">
>> <br class="">
>> Hi Thomas,<br class="">
>> <br class="">
>> 1st of all, thanks for looking at it, appreciate that.<br class="">
>> <br class="">
>> it seems we can. there are two ways I can fix that either replace ASSERT by EXCEPT or introduce a text fixture w/ SetUp and TearDown. which do you prefer?<br class="">
>> <br class="">
>> -- Igor<br class="">
> <br class="">
> Slight preference for SetUp/TearDown (EXCEPT would quit the testing<br class="">
> altogether, wont it?).<br class="">
> <br class="">
> Lazy me would have just used an RAII object like this:<br class="">
> <br class="">
> struct Cleaner {<br class="">
> ReservedSpace* rs;<br class="">
> ~Cleaner() { rs.release(); }<br class="">
> };<br class="">
> <br class="">
> But I leave it up to you.<br class="">
> <br class="">
> ... Thomas<br class="">
> <br class="">
>> <br class="">
>>> On Oct 17, 2018, at 11:16 AM, Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" target="_blank" class="">thomas.stuefe@gmail.com</a>> wrote:<br class="">
>>> <br class="">
>>> Hi Igor,<br class="">
>>> <br class="">
>>> test_virtual_space_actual_committed_space() and<br class="">
>>> actual_committed_space_one_large_page():<br class="">
>>> <br class="">
>>> Won't we leak the reserved space now when we run into an ASSERT and<br class="">
>>> then continue with the next gtests?<br class="">
>>> <br class="">
>>> Cheers, Thomas<br class="">
>>> <br class="">
>>> On Wed, Oct 17, 2018 at 7:55 PM Igor Ignatyev <<a href="mailto:igor.ignatyev@oracle.com" target="_blank" class="">igor.ignatyev@oracle.com</a>> wrote:<br class="">
>>>> <br class="">
>>>> <a href="http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html" rel="noreferrer" target="_blank" class="">http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html</a><br class="">
>>>>> 141 lines changed: 140 ins; 1 del; 0 mod;<br class="">
>>>> <br class="">
>>>> Hi all,<br class="">
>>>> <br class="">
>>>> could you please review this small (and hopefully trivial) patch which converts internal TestVirtualSpace_test to gtest?<br class="">
>>>> again, the old test is still used by WhiteBox::runMemoryUnitTests, the old code hasn't been removed. it will be removed later when all 4 tests used by runMemoryUnitTests are converted.<br class="">
>>>> <br class="">
>>>> for the sake of reviewer, <a href="http://hg.openjdk.java.net/jdk/jdk/file/2e928420389d/src/hotspot/share/memory/virtualspace.cpp#l1261" rel="noreferrer" target="_blank" class="">http://hg.openjdk.java.net/jdk/jdk/file/2e928420389d/src/hotspot/share/memory/virtualspace.cpp#l1261</a> is the old version.<br class="">
>>>> <br class="">
>>>> webrev: <a href="http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html" rel="noreferrer" target="_blank" class="">http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html</a><br class="">
>>>> JBS: <a href="https://bugs.openjdk.java.net/browse/JDK-8177709" rel="noreferrer" target="_blank" class="">https://bugs.openjdk.java.net/browse/JDK-8177709</a><br class="">
>>>> testing:<br class="">
>>>> - converted tests on linux-x64, windows-x64, macosx-x64, solaris-sparcv9 in product and fastdebug variants<br class="">
>>>> - build w/ precompiled-headers enabled and disabled<br class="">
>>>> <br class="">
>>>> Thanks,<br class="">
>>>> -- Igor<br class="">
>> <br class="">
<br class="">
</blockquote></div></div>
</div></blockquote></div><br class=""></div></body></html>