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

List:       openjdk-openjfx-dev
Subject:    Re: Code Review Request For 8155757: Encapsulate impl_ methods in animation, canvas, image, input, l
From:       Jim Graham <james.graham () oracle ! com>
Date:       2016-04-29 23:04:00
Message-ID: fb4b2694-1f4e-8dbf-1894-d8d74c288d24 () oracle ! com
[Download RAW message or body]

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


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