Re: Code Review Request For 8155757: Encapsulate impl_ methods in animation, canvas, image, input, layout, paint, and text packages

[Chien Yang]

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