RFR(L): 8005071: Incremental inlining for JSR 292

[Christian Thalinger]

On Dec 20, 2012, at 9:30 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:

> Hi Christian,
> 
> Thanks for the review.
> 
>> src/share/vm/opto/bytecodeInfo.cpp:
>> 
>> -  if (callee_method->force_inline() || callee_method->should_inline()) {
>> +  if ( callee_method->should_inline()) {
>> 
>> Are ForceInline methods always inlined after this change?  Because there are more checks after that line.
> 
> Is there anything in the logic that you think can be a problem?

Only this one:

    } else if (!callee_method->was_executed_more_than(MIN2(MinInliningThreshold,
                                                           CompileThreshold >> 1))) {
      return "executed < MinInliningThreshold times";
    }

-- Chris

> 
>> I'm not so happy with your additional boolean arguments:
>> 
>> delay
>> no_late
>> not_const
>> 
>> We probably can't live without them (can we?) but more meaningful names would be good.
> 
> Ok.
> 
>> 
>> src/share/vm/opto/compile.cpp:
>> 
>> +  // clean up the late inline lists
>> +  for (int j = 0; j < 2; j++) {
>> +    int shift = 0;
>> +    GrowableArray<CallGenerator*>* inlines = j == 0 ? &_string_late_inlines : &_late_inlines;
>> 
>> Why 2?  Maybe you should move that loop into a method and call it with the arrays as an argument.
> 
> Ok.
> 
>> src/share/vm/opto/compile.hpp:
>> 
>> +  uint                          _nb_mh_late;          // number of method handle late inlining still pending
>> 
>> Usually we have a _nof prefix for "number of" but can't we just call it like that?
>> 
>> +  uint                          _number_of_mh_late_inlines;          // number of method handle late inlining still pending
>> 
>> I don't like these abbreviations.
> 
> Ok.
> 
>> +  void Incremental_Inline(PhaseIterGVN& igvn, bool eager = false);
>> +  void String_Inline(bool parse_time);
>> 
>> Let's not add new camel-case methods.
> 
> Ok.
> 
> Roland.
>