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

Vitaly Davidovich vitalyd at gmail.com
Wed Oct 28 21:31:25 UTC 2015

I'll run more testing, after we figure the fate for interpreter.cpp change. It is prompted by Zero's fast accessor implementation that only accepts the specific getter shape. Now, we can go three routes: a) Ignore the issue, and keep Method::issimpleaccessor; b) Fix Zero's fast accessor to accept all the shapes. c) Remove fast accessors from Zero (I see UseFastAccessorMethods is marked as obsolete), and thus probably remove the notion of "accessor" from interpreter completely (?);

Another option to consider is #ifdef is_accessor for Zero differently from the rest so you don't need two different methods.

On Wed, Oct 28, 2015 at 11:31 AM, Aleksey Shipilev < aleksey.shipilev at oracle.com> wrote:

Hi,

I have been reading the compiler code recently to check if setters/getters are actually treated specially in inline policy. They do, and inliner relies on Method::isaccessor to detect them. But then I realized that Method::isaccessor implementation only accepts the specific shapes of getters, and completely ignores setters (contrary to what is spelled in the "doc" comment!): https://bugs.openjdk.java.net/browse/JDK-8140650 This makes compilers to ignore many trivial methods that we might otherwise inline when all other inline hints have failed. With that in mind, I did a proof-of-concept change, which passes JPRT and a new compiler-specific regression test: http://cr.openjdk.java.net/~shade/8140650/webrev.00/ I'll run more testing, after we figure the fate for interpreter.cpp change. It is prompted by Zero's fast accessor implementation that only accepts the specific getter shape. Now, we can go three routes: a) Ignore the issue, and keep Method::issimpleaccessor; b) Fix Zero's fast accessor to accept all the shapes. c) Remove fast accessors from Zero (I see UseFastAccessorMethods is marked as obsolete), and thus probably remove the notion of "accessor" from interpreter completely (?); Current patch does (a), and I'm leaning to keep it that way, letting Zero to handle more in future. If we care more about Zero, we might go for (b) -- although it seems to deserve a separate follow-up RFE. And if we don't, we can go for (c). Thoughts? Thanks, -Aleksey

More information about the hotspot-runtime-dev mailing list