review(M): 7057120: Tiered: Allow C1 to inline methods with loops
Igor Veresov
igor.veresov at oracle.com
Wed Jun 22 14:49:55 PDT 2011
Thanks for the review, Vladimir! Webrev updated. Please find the replies
inline.
On 6/22/11 9:51 AM, Vladimir Kozlov wrote:
> Can you explain why in counter_overflow_helper() (c1_Runtime1.cpp) you
> expect the caller (enclosing_method) to be nmethod?
It's not the caller. The enclosing_method is the actual method to which
the nmethod belongs. In other words: enclosing_method is a method who's
frame is current on stack and "method" is possibly the inlinee inside
the enclosing method that generated the event.
>
> Could you rename should_not_inline() to should_not_inline_for_profile()
> or something since it is only for C1 compilation with profiling?
>
I'd rather leave it generalized if you're not totally against it. We can
use it later as a wrapper for oracle to control inlining in both compilers.
> Add {} for else:
>
> + } else
> + if (next_level != CompLevel_full_optimization) {
> + // next_level is not full opt, so we need to recompile the
> + // enclosing method without the inlinee
> + cur_level = CompLevel_none;
> + make_not_entrant = true;
> + }
> + if (make_not_entrant) {
>
Ok.
> I don't understand why you need to recompile caller. It comes from a
> different frame so it did not inline callee before :
>
> + if (max_osr_level == CompLevel_full_optimization) {
> + // The inlinee OSRed to full opt, we need to modify the enclosing
> method to avoid deopts
>
If we're are in this clause (that is we passed the mh() != imh() check)
that means that the event was generated by the imh() that was inlined in
mh(). So, we need to recompile mh() because otherwise we would be still
executing the inline version of imh() and would be deoptimizing and
switching to the full opt version.
> Also next check should be at the beginning of this logic since "Fix up"
> could trigger unneeded recompilation:
>
> + if (!CompileBroker::compilation_is_in_queue(mh, InvocationEntryBci) &&
> cur_level != next_level) {
But that would mean that we won't be able to make mh() non-entrant in
case mh() is in the queue already. I move the fix up down since the code
that makes the enclosing method non-entrant doesn't depend on it.
igor
>
> Vladimir
>
> Igor Veresov wrote:
>> This allows inlining of methods with loops by C1 on levels with
>> profiling (2 and 3). The solution is to recompile the enclosing
>> methods without inlining of the method that has OSRed to level 4 or
>> recompile the enclosing method at level 4.
>>
>> There's no significant impact on performance because of these changes.
>>
>> Webrev: http://cr.openjdk.java.net/~iveresov/7057120/webrev.00/
>>
>> The change is for 7u2.
>>
>> Thanks,
>> igor
More information about the hotspot-compiler-dev
mailing list