RFR(S) : 8011675: adding compilation level to replay data

[Vladimir Kozlov]

I would set CrashAfterCompileId default to -1 as indication it is not 
set. Then you don't need to check CrashAfterCompileId == 0 since 
compile_id is positive. By the way, it is very nice idea but add a 
comment about what that code for. I did not get it first :)

About name. For such flags we use CI: CIBreakAt, CIStop. So I would 
suggest to use CICrashAt. Which means VM crash with that compile id, not 
when it is greater.

If you replaying C1 compilation next setting will be incorrect. Should 
we use CompLevel_highest_tier instead so that following check pass?:

+     if (had_error() && (error_message() == comp_level_label)) {
+       comp_level = CompLevel_full_optimization;


About tests.

Add -Xmx32m, -XX:ParallelGCThreads=2 and other flags (codecache 
,metaspace) to limit core size on big machines.

Add tests for tiered with different -XX:TieredStopAtLevel=n.

I would add more info in output why the test is skipped:
      echo TEST SKIPPED

typo 'java':
  124 is_tiered=`${JAVA} ${TESTVMOPTS} java -XX:+PrintFlagsFinal 
-version 2>&1 | grep TieredCompilation | grep -c true`


I don't like next:

  130 ${JAVA} ${TESTVMOPTS} -Xcomp -XX:CrashAfterCompileId=1 
-XX:+CreateMinidumpOnCrash -XX:+DumpReplayDataOnError -version &
  131 crash_pid=$!
  132 wait $crash_pid

  On windows 'wait $crash_pid' may not work (happened before) because 
before you execute 'wait' OS may assign the same pid to different process.

You may still not find core file. Or ulimit -c unlimited will not work
(windows, OS X?). You need to bailout in such case as success. Next code 
is incorrect for that:

   48 if [ -z "${core_file}" -o ! -r "${core_file}" ]
   49 then
   50     echo TEST FAILED: core wasn\'t generated
   51     exit 2
   52 fi

Thanks
Vladimir

On 4/12/13 8:27 AM, Igor Ignatyev wrote:
> updated webrev: http://cr.openjdk.java.net/~iignatyev/8011675/webrev.02/
> added:
>    1. more informative message on unsupported comp_level
>    2. not-product flag 'CrashAfterCompileId' and assert in
> compileBroker.cpp for assured generation of replay.txt
>    2. tests for ciReplay
>
> Best regards,
> Igor Ignatyev
>
> On 04/09/2013 11:40 AM, Vladimir Kozlov wrote:
>> Igor,
>>
>> Sorry, I may said it not clear. What I meant is to set error (call
>> report_error()) in such case and return from process_compile() without
>> compilation. But let ciReplay::replay() finish and exit. replay_impl()
>> should print the problem so you don't need to do it in
>> process_compile(). And, please, test it.
>> The message should be more informative, something like:
>> "compilation level %d requires TieredCompilation"
>> An other bad cases is when failure happened in Client (C1) or Server
>> (C2) but the replay is run with opposite VM.
>>
>> thanks,
>> Vladimir
>>
>> On 4/9/13 12:04 AM, Igor Ignatyev wrote:
>>> Vladimir,
>>>
>>> Thank you for review.
>>>
>>> updated webrev: http://cr.openjdk.java.net/~iignatyev/8011675/webrev.01/
>>>
>>> Best regards,
>>> Igor Ignatyev
>>>
>>> On 04/09/2013 04:19 AM, Vladimir Kozlov wrote:
>>>> Igor,
>>>>
>>>> Thank you for fixing and cleaning this.
>>>>
>>>> In vmStructs.cpp move _comp_level after _compile_id as in nmethod
>>>> class.
>>>>
>>>> Typo in ciReplay.cpp:
>>>> +      comp_level != CompLevel_highest_tier;
>>>>
>>>> Actually it is useless to replay compilation at different level, it
>>>> should exit with error. Could happened because incorrectly run without
>>>> Tiered when the crush happened with Tiered.
>>>>
>>>> In ciReplay.cpp use parenthesis around equality checks: if (. && (. ==
>>>> .)).
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 4/8/13 12:30 PM, Igor Ignatyev wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review patch.
>>>>>
>>>>> Problem:
>>>>> Replay of compilations supports c1 and tiered as well as c2, the only
>>>>> thing missing is comp_level.
>>>>>
>>>>> Fix:
>>>>> 1. added comp_level into dump and usage during replay
>>>>> 2. minor changes in agent/doc:
>>>>>    - escaped '<', '>'
>>>>>    - renamed 'C2 Replay'/'C2 compiler replay' to 'Replay'/'Compiler
>>>>> replay'
>>>>>
>>>>> Testing:
>>>>> manually launched the replay from JDK-8010934 (assert specifically to
>>>>> c1) on jvm w/o corresponded fix. assert was triggered w/ patch and
>>>>> wasn't triggered w/o.
>>>>>
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8011675/webrev.00/
>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-8011675