<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="">
Hello Valeri,
<div class=""><br class="">
</div>
<div class="">Thank you for review. Please, have a look at updated patch at : <a class="moz-txt-link-freetext" href="https://cr.openjdk.java.net/~abakhtin/8241960/webrev.v1/">https://cr.openjdk.java.net/~abakhtin/8241960/webrev.v1/</a></div>
<div class=""><br class="">
</div>
<div class="">I’ve fixed your findings and renamed test class to ThreadSafetyTest. Jtreg already has <span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class="">ThreadSafeTest class for </span><font color="#000000" class=""><span style="caret-color: rgb(0, 0, 0);" class="">java.text.Normalizer
functionality.</span></font></div>
<div class=""><font color="#000000" class=""><span style="caret-color: rgb(0, 0, 0);" class=""><br class="">
</span></font></div>
<div class="">Thank you</div>
<div class="">Alexey</div>
<div class=""><br class="">
<div>
<blockquote type="cite" class="">
<div class="">On 3 Apr 2020, at 02:51, Valerie Peng <<a href="mailto:valerie.peng@oracle.com" class="">valerie.peng@oracle.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<p class="">Hi Alexey,</p>
<p class="">In general looks pretty good, just some comments on the regression test:</p>
<p class="">- line 28: The duration value 10 may be lowered to shorten the execution time. On a Linux machine, with threadFactor=5, each digest algo takes about (2xduration+2) sec and overall takes ~283sec. Cutting the duration value drastically to 2, I still
can reproduce the bug. Maybe 4 would be a better default value considering the execution time.
<br class="">
</p>
<p class="">- line 43: add "SHA-512/224", "SHA-512/256" to the ALGORITHM_ARRAY?<br class="">
</p>
<p class="">- line 50: instead of hardcoding 5 here, why not just use threadsFactor (assigned with default value 5 on line 48)?</p>
<p class="">- line 52-55: I think you meant to use 5 (instead of the value 180) as the default min duration on line 52. Then, use duration variable instead of the hardcoded 5 on line54. For testing purpose, why not just use the supplied value? You already
have default values if none are supplied.</p>
<p class="">- Consider iterating through existing providers and remove isSHA3supported() method. This way all providers which supports the digest algorithm are tested and no provider-specific knowledge is needed. For example:</p>
<pre class=""> Provider[] provs = Security.getProviders();</pre>
<pre class=""> for (Provider p : provs) {</pre>
<pre class=""> System.out.println("Testing provider " + p.getName() + "...");</pre>
<pre class=""> for (String alg: ALGORITHM_ARRAY) {</pre>
<pre class=""> try {</pre>
<pre class=""> MessageDigest md = MessageDigest.getInstance(alg, p);</pre>
<pre class=""> testThreadSafe(md, input, nTasks, duration, false);</pre>
<pre class=""> .... </pre>
<p class="">- line 76: missing space char between } and catch.</p>
Thanks,<br class="">
Valerie<br class="">
<div class="moz-cite-prefix"><br class="">
</div>
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</body>
</html>