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

Dmitrij Pochepko dmitrij.pochepko at oracle.com
Wed Dec 24 20:09:01 UTC 2014


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
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141224/a136f762/attachment-0001.html>


More information about the hotspot-compiler-dev mailing list