<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Stefan,<br>
<br>
Thank you for finding and fixing this bug.<br>
<br>
<div class="moz-cite-prefix">On 10/30/2017 08:04 AM, Stefan
Johansson wrote:<br>
</div>
<blockquote type="cite"
cite="mid:aef9efb9-39a6-3e34-da8d-6323bdbd0110@oracle.com">Hi,
<br>
<br>
Please review this fix for:
<br>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8190353">https://bugs.openjdk.java.net/browse/JDK-8190353</a>
<br>
<br>
Webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~sjohanss/8190353/00/">http://cr.openjdk.java.net/~sjohanss/8190353/00/</a>
<br>
<br>
Summary:
<br>
The fix for JDK-8188245 was incomplete, it removed the check that
if the reported time is longer than the sum of the sub-phases
everything is ok. The new code requires the sum of the sub-phases
to be within a give tolerance from the reported time. This is true
in many cases but we can't know for sure that we won't get a
context switch or some other stall between two sub-phases that
causes the reported total time to be longer, but the sub-phases to
not include this extra time.
<br>
<br>
Testing:
<br>
Verified that the new verification method returns correct values
for different input values.
<br>
</blockquote>
Looks good.<br>
<br>
Just minor nit: you can ignore or no webrev is needed.<br>
140 // Because of this we need method to verify that our
measurements and calculations are valid.<br>
-> Because of this<u><b>,</b></u> we need <u><b>a</b></u> method
...<br>
<br>
Thanks,<br>
Sangheon<br>
<br>
<br>
<blockquote type="cite"
cite="mid:aef9efb9-39a6-3e34-da8d-6323bdbd0110@oracle.com">
<br>
Thanks,
<br>
Stefan
<br>
</blockquote>
<br>
</body>
</html>