RFR (S): [TESTBUG] Test for JDK-8180048

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR (S): [TESTBUG] Test for JDK-8180048

Thomas Schatzl
Hi all,

  can I have some reviews that adds a test for JDK-8180048? That test
has not been completed in time for JDK 9, but I think it is good to
have it in JDK 10.

It tries to detect the races fixed in JDK-8180048 by parsing NMT output
before/after symbol unloading. To be sure that we exercise the buggy
code path, the test takes some time (~30s) to stress symbol unloading.

To avoid this test clogging up valuable time in lower testing tiers,
the test has been put into the "stress" test directory so that it only
executes at higher test tiers. Further I limited it to execute only
with a release build.

CR:
https://bugs.openjdk.java.net/browse/JDK-8180280
Webrev:
http://cr.openjdk.java.net/~tschatzl/8180280/webrev
Testing:
test case passing

Thanks,
  Thomas
Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): [TESTBUG] Test for JDK-8180048

Erik Österlund-2
Hi Thomas,

Looks good.
Note though that...

   82         Asserts.assertLT(reserved, ReservedThreshold, "Reserved
memory size is " + reserved + "KB which is higher than " +
ReservedThreshold + "KB indicating a memory leak");

...I guess it should say "greater than or equal to" instead of "higher
than", but I'm not sure if I really care all that much.

Also, the copyright years might need some tweaking.

I don't need another webrev.

Thanks,
/Erik

On 2017-11-27 13:52, Thomas Schatzl wrote:

> Hi all,
>
>    can I have some reviews that adds a test for JDK-8180048? That test
> has not been completed in time for JDK 9, but I think it is good to
> have it in JDK 10.
>
> It tries to detect the races fixed in JDK-8180048 by parsing NMT output
> before/after symbol unloading. To be sure that we exercise the buggy
> code path, the test takes some time (~30s) to stress symbol unloading.
>
> To avoid this test clogging up valuable time in lower testing tiers,
> the test has been put into the "stress" test directory so that it only
> executes at higher test tiers. Further I limited it to execute only
> with a release build.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8180280
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8180280/webrev
> Testing:
> test case passing
>
> Thanks,
>    Thomas

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S): [TESTBUG] Test for JDK-8180048

Thomas Schatzl
Hi Erik and Kim,

On Wed, 2018-01-10 at 17:44 +0100, Erik Österlund wrote:

> Hi Thomas,
>
> Looks good.
> Note though that...
>
>    82         Asserts.assertLT(reserved, ReservedThreshold,
> "Reserved
> memory size is " + reserved + "KB which is higher than " +
> ReservedThreshold + "KB indicating a memory leak");
>
> ...I guess it should say "greater than or equal to" instead of
> "higher than", but I'm not sure if I really care all that much.
>
> Also, the copyright years might need some tweaking.

I will fix both before pushing.

>
> I don't need another webrev.

Thanks a lot for your reviews.

Thomas