[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-zero-dev
Subject:    Fwd: RFR (S) 8140650: (preliminary) Method::is_accessor should cover getters and setters for all typ
From:       Aleksey Shipilev <aleksey.shipilev () oracle ! com>
Date:       2015-10-30 14:21:58
Message-ID: 56337D06.8000905 () oracle ! com
[Download RAW message or body]


Hi,

There is a suggested change that affects Zero: UseFastAccessorMethods is
used only in Zero after JDK-8003426. We would like to make the
is_accessor matcher a proper one, which raises some questions below:

-------- Forwarded Message --------
Subject: RFR (S) 8140650: (preliminary) Method::is_accessor should cover
getters and setters for all types
Date: Wed, 28 Oct 2015 18:31:46 +0300
From: Aleksey Shipilev <aleksey.shipilev@oracle.com>
To: hotspot-runtime-dev <hotspot-runtime-dev@openjdk.java.net>

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::is_accessor to detect them.

But then I realized that Method::is_accessor 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::is_simple_accessor;
 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





["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]