RFC: JWarmup precompile java hot methods at application startup

[yumin qi]

Hi, Tobias and all

  Have done changes based on Tobias' comments.  New webrev based on most
recent base is updated at:
   http://cr.openjdk.java.net/~minqi/8220692/webrev-03/

   Tested local for jwarmup and compiler.

  Thanks
  Yumin


On Tue, May 14, 2019 at 11:26 AM yumin qi <yumin.qi at gmail.com> wrote:

> HI, Tobias
>
>   Thanks very much for the comments.
>
> On Mon, May 13, 2019 at 2:58 AM Tobias Hartmann <
> tobias.hartmann at oracle.com> wrote:
>
>> Hi Yumin,
>>
>> > In this version, the profiled method data is not used at
>> > precomilation, it will be addressed in followed bug fix. After the
>> > first version integrated, will file bug for it.
>>
>> Why is that? I think it would be good to have everything in one JEP.
>>
>
> We have done some tests on adding profiling data and found the result is
> not as expected, and the current version is working well for internal
> online applications. There is no other reason not adding to this patch now,
> we will like to study further to see if we can improve that for a better
> performance.
>
>
>> I've looked at the compiler related changes. Here are some
>> comments/questions.
>>
>> ciMethod.cpp
>> - So CompilationWarmUp is not using any profile information? Not even the
>> profile obtained in the
>> current execution?
>>
>>
> Yes. This is also related to previous question.
>
> compile.cpp
>> - line 748: Why is that required? Couldn't it happen that a method is
>> never compiled because the
>> code that would resolve a field is never executed?
>>
>>
> Here a very aggressive decision --- to avoid compilation failure requires
> that all fields have already been resolved.
>
>
>> graphKit.cpp
>> - line 2832: please add a comment
>> - line 2917: checks should be merged into one if and please add a comment
>>
>>
> Will fix it.
>
>
>> jvm.cpp
>> - Could you explain why it's guaranteed that warmup compilation is
>> completed once the dummy method
>> is compiled? And why is it hardcoded to print
>> "com.alibaba.jwarmup.JWarmUp"?
>>
>>
> This is from practical testing of real applications. Due to the
> parallelism of compilation works, it  should check if compilation queue
> contains any of those methods --- completed if no any of them on the queue
> and it is not economic. By using of a dummy method as a simplified version
> for that, in real case, it is not observed that dummy method is not the
> last compilation for warmup. Do you have suggestion of a way to do that?
> The dummy way is not strictly a guaranteed one theoretically.
> Forgot to change the print to new name after renaming package, thanks for
> the catching.
>
>
>> - What is test_symbol_matcher() used for?
>>
>>
> This is a leftover(used to test matching patterns), will remove it from
> the file.
>
>
>> jitWarmUp.cpp:
>> - line 146: So what about methods that are only ever compiled at C1
>> level? Wouldn't it make sense to
>> keep track of the comp_level during CompilationWarmUpRecording?
>>
>>
>> Will consider your suggestion in future work on it.
>
>
>> I also found several typos while reading through the code (listed in
>> random order):
>>
>> globals.hpp
>> - "flushing profling" -> "flushing profiling"
>>
>> method.hpp
>> - "when this method first been invoked"
>>
>> templateInterpreterGenerator_x86.cpp
>> - initializition -> initialization
>>
>> dict.cpp
>> - initializated -> initialized
>>
>> jitWarmUp.cpp
>> - uninitilaized -> uninitialized
>> - inited -> should be initialized, right?
>>
>> jitWarmUp.hpp
>> . nofityApplicationStartUpIsDone -> notifyApplicationStartUpIsDone
>>
>> constantPool.cpp
>> - recusive -> recursive
>>
>> JWarmUp.java
>> - appliacation -> application
>>
>> TestThrowInitializaitonException.java ->
>> TestThrowInitializationException.java
>>
>> These tests should be renamed (it's not clear what issue the number
>> refers to):
>> - issue9780156.sh
>> - Issue11272598.java
>>
>
> Will fix all above suggestions.
>
> Thanks!
>
> Yumin
>