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

[Jim Graham]

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