[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8230942: Support compressed cores in SA tests
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2019-09-27 10:22:02
Message-ID: cd6d820b-84dd-b410-405d-f4af7f5ccf7a () oracle ! com
[Download RAW message or body]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Leonid,<br>
<br>
Looks good in general.<br>
<br>
<br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~lmesnik/8230942/webrev.01/test/lib/jdk/test/lib/SA/S \
ATestUtils.java.udiff.html">http://cr.openjdk.java.net/~lmesnik/8230942/webrev.01/test/lib/jdk/test/lib/SA/SATestUtils.java.udiff.html</a><br>
<br>
Minor:<br>
<pre><span class="new">+import java.io.File;</span>
import java.io.IOException;
<span class="removed">-import java.util.List;</span>
<span class="new">+import java.io.FileInputStream;</span>
<span class="new">+import java.io.FileOutputStream;</span></pre>
Newly added "<span class="new">import java.io.File;</span>" is out
of order.<br>
<br>
The order of imports in the other two files is also not always
correct.<br>
<br>
For instance, the imports for Platform, <span class="new">SA.SATestUtils
and Utils below are out of order:</span>
<pre> import jdk.test.lib.Platform;
import jdk.test.lib.classloader.GeneratingClassLoader;
import jdk.test.lib.hprof.HprofParser;
import jdk.test.lib.process.ProcessTools;
import jdk.test.lib.process.OutputAnalyzer;
<span class="new">+import jdk.test.lib.SA.SATestUtils;</span>
import jdk.test.lib.Utils;
import jtreg.SkippedException;</pre>
<span class="new"></span><br>
No need in new webrev if you fix it.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<br>
On 9/26/19 23:28, Leonid Mesnik wrote:<br>
</div>
<blockquote type="cite"
cite="mid:85EA401F-84ED-48F1-B125-539C80EC0B5F@oracle.com">
<div class="">Thanks for feedback.</div>
<div class=""><br class="">
</div>
<div class="">I checked time execution. Tests takes several
seconds longer on the hosts which compress cores. Also, tests
TestJmapCore.java, TestJmapCoreMetaspace.java are slow and not
included in tier1_serviceability anyway.</div>
<div class=""><br class="">
</div>
<div class="">Updated version here. I fixed it accordingly with
your comments:
<div class=""><a
href="http://cr.openjdk.java.net/~lmesnik/8230942/webrev.01/"
class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8230942/webrev.01/</a></div>
<div class=""><br class="">
</div>
<div class="">Leonid</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">On Sep 26, 2019, at 7:00 PM, David Holmes <<a
href="mailto:david.holmes@oracle.com" class=""
moz-do-not-send="true">david.holmes@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">Hi Leonid,<br class="">
<br class="">
On 27/09/2019 7:18 am, Leonid Mesnik wrote:<br class="">
<blockquote type="cite" class="">Hi<br class="">
Some hosts used for JDK testing have customized core
dump settings. They compress core files saved in
current directory on-the-fly to reduce required disk
space.<br class="">
This fix adopt several SA tests, trying to unpack
core.pid.gz before test process it with jhsdb. It
affects only execution in the case if core.pid.gz
files are actually generated.<br class="">
Verified that tests are passed and not skipped anymore
on default and new configurations.<br class="">
webrev: <a
href="http://cr.openjdk.java.net/~lmesnik/8230942/webrev.00/"
class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8230942/webrev.00/</a><br \
class=""> bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8230942"
class="" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8230942</a><br \
class=""> </blockquote>
<br class="">
Overall seems fine. I hope it doesn't take too long to
do the unzipping. :)<br class="">
<br class="">
A few minor items<br class="">
<br class="">
test/lib/jdk/test/lib/SA/SATestUtils.java<br class="">
<br class="">
+ for(File gzCore : gzCores) {<br class="">
<br class="">
Nit: add space after for<br class="">
<br class="">
+ } catch (IOException e) {<br class="">
+ throw new SkippedException("Not \
able to unzip core file.");<br class="">
+ }<br class="">
<br class="">
Please add the IOException as a cause for the
SkippedException so that we have some diagnostics on why
it couldn't be unzipped.<br class="">
<br class="">
---<br class="">
<br class="">
test/hotspot/jtreg/serviceability/sa/TestJmapCore.java<br
class="">
<br class="">
32 import java.io.File;<br class="">
<br class="">
File is already imported at line 46.<br class="">
<br class="">
+ SATestUtils.unzipCores(new File("."));<br
class="">
...<br class="">
File[] cores = new File(".").listFiles((dir,
name) -> name.matches(pattern));<br class="">
<br class="">
Suggest:<br class="">
<br class="">
File pwd = new File(".");<br class="">
SATestUtils.unzipCores(pwd);<br class="">
...<br class="">
File[] cores = pwd.listFiles((dir, name) ->
name.matches(pattern));<br class="">
<br class="">
and also at line 117:<br class="">
+ ": " + String.join(",",
pwd.list()) + ".");<br class="">
<br class="">
Thanks,<br class="">
David<br class="">
-----<br class="">
<br class="">
<blockquote type="cite" class="">Leonid<br class="">
</blockquote>
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic