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