RFC: JWarmup precompile java hot methods at application startup

[yumin qi]

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