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

[Aleksey Shipilev]

On 28.10.2015 21:12, Vitaly Davidovich wrote:
>     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.

Yes, that's right, my bad. I must have seen something else in my
experiments.

Since you have conveniently omitted another part of my reply, here it is
again:

> On 28.10.2015 20:33, Aleksey Shipilev wrote:
>> 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.

Even after should_not_inline answers "false" for trivial methods,
accessors would then be unconditionally inlined in
InlineTree::try_to_inline, while ordinary "trivial" methods would
undergo more checks later on, including potentially stepping onto
infamous "inlining too deep" bailout.

So the hypothesis that accessors are already handled nicely by the
regular code that handles trivial methods is just not true.


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

What difference does it make for the issue where Method::is_accessor
does not match its own advertised contract? It is a road to a crippling
technical debt, asking whether something is "happening in practice" when
there is a visible mismatch between what method is doing and what it
supposed to be doing.

I am telling yet again, this issue is about providing a compiler with a
reliable hint that method is an accessor, it is *NOT* about what
compilers do with those hints. Please, please stop trying to turn this
discussion into the inline heuristics overhaul!

Thanks,
-Aleksey