Hi,
There is one more issues related to these tests(not directly).
Currently, dtrace tests can't be run in jprt, because dtrace require
additional previleges for launching user.
Perhaps it's just user to be granted previleges in jprt solaris hosts...
Not sure about some guard in tests... the only way i know so far is to
just try launching dtrace and parse output
saying about missing previleges, but it doesn't seem to be good solution.
I think test should assume to be launched in correct environment and in
case it's failed because of previleges, evaluating engineer starting to
work on respective env. issue...
Does anybody have some additional concerns?
Thanks,
Dmitrij
Hi Dmitrij,
There is still a typo in spelling (DESCTRUCTIVE => DESTRUCTIVE):
test/testlibrary/com/oracle/java/testlibrary/dtrace/DtraceRunner.java
42 public static final String PERMIT_DESCTRUCTIVE_ACTIONS_DTRACE_OPTION =
"w";
test/compiler/codecache/dtrace/SegmentedCodeCacheDtraceTest.java
85 DtraceRunner.PERMIT_DESCTRUCTIVE_ACTIONS_DTRACE_OPTION,
Otherwise, looks good.
Please, consider it reviewed (no need to re-review).
Thanks,
Serguei
On 12/23/14 3:36 AM, Dmitrij Pochepko wrote:
Hi,
please review updated webrev
http://cr.openjdk.java.net/~iignatyev/dpochepk/8059625/webrev.01/
Thanks,
Dmitrij
On 12/23/14 12:37 AM, Dmitrij Pochepko wrote:
Hi,
Thank you for catching these issues.
I have a question regarding last comment: does it make any
difference to change "reading of static member 3 times" to "copying
into static member of another class and then read it 3 times"?
Yes, it does (it is a minor issue though).
It is because the class names are pretty big.
It would simplify the code and improve readability, right?
You already do it two times:
179 List<Executable> tml
180 =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
...
235 List<Executable> mlist
236 =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
My suggestion is to do it once somewhere before the line 74:
static final List<Executable> MLIST =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
and then use the mlist in other places where it is needed:
75 String params = MLIST.stream()
...
182 d.put(MLIST.get(i).getName(), new
MethodData(MLIST.get(i).getName(),
...
239 for (int i = MLIST.size() - 1; i> -1; i--) {
240 sb.append(MLIST.get(i).getName()).append(delimeter);
These lines will go away:
179 List<Executable> tml
180 =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
...
235 List<Executable> mlist
236 =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
Both private static classes are inner classes of the public one.
The MLIST will be in a context for the inner classes, and so, needs
no prefixing.
Thanks,
Serguei
Thanks,
Dmitrij
Hi Dmitry,
It looks good in general.
Some minor comments are below.
test/testlibrary/com/oracle/java/testlibrary/dtrace/DtraceRunner.java
42 public static final String PERMIT_DESCTUCTIVE_ACTIONS_DTRACE_OPTION =
"w";
A typo in the constant name: DESCTUCTIVE => DESTRUCTIVE
test/compiler/codecache/dtrace/SegmentedCodeCacheDtraceTest.java
84 DtraceRunner.PERMIT_DESCTUCTIVE_ACTIONS_DTRACE_OPTION,
A typo in the constant name: DESCTUCTIVE => DESTRUCTIVE
60 private static final String WORKER_CLASS_NAME
61 = SegmentedCodeCacheDtraceTestWorker.class.getName();
...
80 runner.runDtrace(JDKToolFinder.getTestJDKTool("java"), JAVA_OPTS,
81 SegmentedCodeCacheDtraceTestWorker.class.getName(),
params,
The WORKER_CLASS_NAME can be used at 81.
75 String params =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST.stream()
...
179 List<Executable> tml
180 =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
...
235 List<Executable> mlist
236 =
SegmentedCodeCacheDtraceTestWorker.TESTED_METHODS_LIST;
The TESTED_METHODS_LIST is used three times.
It can be cached at the top and re-used.
Thanks,
Serguei
On 12/19/14 11:03 AM, Dmitrij Pochepko wrote:
Hi all,
Please review changes for
https://bugs.openjdk.java.net/browse/JDK-8059625 -
JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache
feature
Description: this fix introduce dtrace test, which verify that
different combinations of available compile levels(and, in case
compile levels allows it, different code heaps as result)
doesn't affect callstack shown by dtrace. There is a control
class SegmentedCodeCacheDtraceTest.java and class for running via
dtrace SegmentedCodeCacheDtraceTestWorker.java. A dtrace d script
is also present (SegmentedCodeCacheDtraceTestScript.d). A control
class is using DtraceRunner.java to run dtrace and then analyzing
results using class SegmentedCodeCacheDtraceResultsAnalyzer with
DtraceResultsAnalyzer interface.
There is also a small class CompilerUtils.java created for
usefull common code.
webrev:
http://cr.openjdk.java.net/~iignatyev/dpochepk/8059625/webrev.00/
Additional note: Please note that this path assumes that fix for
JDK-8066440 - Various changes in testlibrary for JDK-8059613 is
also applied.
Thanks,
Dmitrij
