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

Roland Westrelin roland.westrelin at oracle.com
Thu Dec 20 09:30:12 PST 2012


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?

> 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.



More information about the hotspot-compiler-dev mailing list