RFR (S) 8140650: (preliminary) Method::is_accessor should cover getters and setters for all types (original) (raw)
Aleksey Shipilev aleksey.shipilev at oracle.com
Wed Oct 28 17:33:44 UTC 2015
- Previous message: RFR (S) 8140650: (preliminary) Method::is_accessor should cover getters and setters for all types
- Next message: RFR (S) 8140650: (preliminary) Method::is_accessor should cover getters and setters for all types
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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 codesize <= MaxTrivialSize, which defaults to 6 -- this is right below the isaccessor 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
- Previous message: RFR (S) 8140650: (preliminary) Method::is_accessor should cover getters and setters for all types
- Next message: RFR (S) 8140650: (preliminary) Method::is_accessor should cover getters and setters for all types
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]