RFR: 8059625 - JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache feature
Dmitrij Pochepko
dmitrij.pochepko at oracle.com
Tue Dec 23 13:35:35 UTC 2014
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
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20141223/b8314d4e/attachment.html>
More information about the serviceability-dev
mailing list