That sounds like a good idea. I like the idea of keeping the reduced
scope (private where possible) for clarity if there isn't a noticeable
performance penalty for doing so. Also, it matches our pattern for
existing accessor methods, some of which directly access private
property fields; changing them would be at odds with our pattern for
declaring properties.
-- Kevin
Jim Graham wrote:
Maybe we could ask someone in Hotspot if they recognize these bytecode
patterns and optimize out the helper methods. If that's the case,
then it is really just down to a bundled size issue...
...jim
On 5/1/2016 11:55 PM, Chien Yang wrote:
Hi Jim,
Thanks for sharing this information and your thought. I'm not sure is
this saving worth violating the principle of minimizing scope in code. I
guess you did bring up a good point let me think over it and discuss
with Kevin tomorrow.
- Chien
On 4/29/16, 4:04 PM, Jim Graham wrote:
One comment on the implementation. For the methods used by an
accessor inner class, if you make them private in the outer class then
that inner class will need a hidden accessor method to be added in the
bytecodes. If you make them package-private then they can access the
method directly.
Basically, an inner class is really "just another class in the
package, but with a special name" and actually have no access to
private methods in their outer classes at all, but javac works around
this by adding a hidden method that has more open access and using
that.
An example is Image.getPlatformImage() - you turned it from "public
and impl_" into private. Why not just leave it
package-private/default access?
For example, compiling this class:
public class InnerPrivateTest {
private void foo() {}
public class InnerClass {
public void bar() { foo(); }
}
}
yields this byte code for InnerPrivateTest.class:
public class InnerPrivateTest {
public InnerPrivateTest();
Code:
0: aload_0
1: invokespecial #2 // Method
java/lang/Object."<init>":()V
4: return
private void foo();
Code:
0: return
static void access$000(InnerPrivateTest);
Code:
0: aload_0
1: invokespecial #1 // Method foo:()V
4: return
}
and this for the InnerClass:
public class InnerPrivateTest$InnerClass {
final InnerPrivateTest this$0;
public InnerPrivateTest$InnerClass(InnerPrivateTest);
Code:
0: aload_0
1: aload_1
2: putfield #1 // Field
this$0:LInnerPrivateTest;
5: aload_0
6: invokespecial #2 // Method
java/lang/Object."<init>":()V
9: return
public void bar();
Code:
0: aload_0
1: getfield #1 // Field
this$0:LInnerPrivateTest;
4: invokestatic #3 // Method
InnerPrivateTest.access$000:(LInnerPrivateTest;)V
7: return
}
Changing the access on foo() to default (package private), yields this
byte code:
public class InnerPackageTest {
public InnerPackageTest();
Code:
0: aload_0
1: invokespecial #1 // Method
java/lang/Object."<init>":()V
4: return
void foo();
Code:
0: return
}
public class InnerPackageTest$InnerClass {
final InnerPackageTest this$0;
public InnerPackageTest$InnerClass(InnerPackageTest);
Code:
0: aload_0
1: aload_1
2: putfield #1 // Field
this$0:LInnerPackageTest;
5: aload_0
6: invokespecial #2 // Method
java/lang/Object."<init>":()V
9: return
public void bar();
Code:
0: aload_0
1: getfield #1 // Field
this$0:LInnerPackageTest;
4: invokevirtual #3 // Method
InnerPackageTest.foo:()V
7: return
}
...jim
On 4/29/16 11:50 AM, Chien Yang wrote:
Hi Kevin,
Please review the proposed fix:
JIRA: https://bugs.openjdk.java.net/browse/JDK-8155757
Webrev: http://cr.openjdk.java.net/~ckyang/JDK-8155757/webrev.00/
Thanks,
- Chien
