RFR: 8059625 - JEP-JDK-8043304: Test task: DTrace- tests for segmented codecache feature

Igor Ignatyev igor.ignatyev at oracle.com
Thu Dec 25 10:05:27 UTC 2014 

I'd prefer to check privileges by running dtrace against dtrace;
and do it in DtraceRunner::dtraceAvailable, smth like

> result = false;
> dtrace = getDtracePath();
> if ($dtrace) {
>   $dtrace $dtrace
>   result = $? == 0;
> }
> return $result;


Igor

On 12/24/2014 11:09 PM, Dmitrij Pochepko wrote:
> Hi,
>
> i've placed a guard which skips test in case dtrace exits with non-zero
> code.
>
> http://cr.openjdk.java.net/~iignatyev/dpochepk/8059625/webrev.02/
>
> Thanks,
> Dmitrij
>> 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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

More information about the serviceability-dev mailing list