RFR (S) 8140650: (preliminary) Method::is_accessor should cover getters and setters for all types

[Vitaly Davidovich]

>
> You also miss the entire story about the interaction with profile
> hotness, see e.g. InlineTree::should_not_inline and checks via
> Method::was_executed_more_than. Cold "trivial" methods would not be
> inlined, but accessors would.


Right before the was_executed_more_than check in
InlineTree:should_not_inline is a check for code <= MaxTrivialSize; if that
condition is true (as it will be for these accessors), should_not_inline
returns false.

Again, my question is really whether this is a theoretical problem you're
addressing or you actually have a concrete example (real or conceivably
real); the existing inlining policy code appears to cover these trivial
methods under other conditions/flags.


On Wed, Oct 28, 2015 at 1:33 PM, Aleksey Shipilev <
aleksey.shipilev at oracle.com> wrote:

> On 10/28/2015 07:49 PM, Vitaly Davidovich wrote:
> > There's also code in the inline policy which skips a bunch of checks if
> > code_size <= MaxTrivialSize, which defaults to 6 -- this is right below
> > the is_accessor check, and also talks about trivial methods and
> > accessors.
>
> If you take a closer look, then you'll notice that a large number of
> checks is done for both trivial and non-trivial methods, after the ">
> MaxTrivialSize" block ends. AFAIR, this is because method size *does
> not* tell e.g. how many nodes it will bring: there might be a method
> call, or special code that is intrinsified, or... etc, etc, etc.
> Accessors are much safer in this regard.
>
> You also miss the entire story about the interaction with profile
> hotness, see e.g. InlineTree::should_not_inline and checks via
> Method::was_executed_more_than. Cold "trivial" methods would not be
> inlined, but accessors would. In vast majority of cases the profile
> itself would tell to inline the accessor method, even if accessor check
> fails. But the profile is not a completely reliable instrument for
> inlining policy, as even the cold methods may break optimizations
> downstream.
>
> >     The actual issue is not about treating the accessors in compiler, but
> >     rather about runtime disregarding many methods which are, in fact,
> >     trivial accessors.
> >
> >
> > You mean in interpreter? Where else does this play a role?
>
> I am saying that the goal for this change is *not* about either
> discussing or changing the inlining policy decisions within the
> compiler. Accessors are treated differently for a reason. This issue is
> about matching all actual accessors, not some small subset of them.
>
> Let's not blow up the contained tech debt improvement into a full-scale
> overhaul of the inlining heuristics.
>
> Thanks,
> -Aleksey
>
>