RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType

[Claes Redestad]

Hi Mandy,


On 2018-08-17 19:07, mandy chung wrote:
>
>
> On 8/17/18 7:38 AM, Peter Levart wrote:
>>
>>
>> On 08/17/2018 04:32 PM, Claes Redestad wrote:
>>> Hi Peter,
>>>
>>> On 2018-08-17 16:04, Peter Levart wrote:
>>>> Hi Claes,
>>>>
>>>> Nice trick.
>
> +1

thanks!

>
>>>> You made MethodType(s) and WeakEntry(s) holding them equal, 
>>>> respecting the equals()/hashCode() contract. When WeakEntry looses 
>>>> the referent it is left equal to itself only, which is enough for 
>>>> expunging it from map.
>>>
>>> Good summary. Do you think we need a few comments to spell out these 
>>> intricacies?
>>
>> Yes, perhaps just pointers from the two equals() methods to each 
>> other, explaining that they are actually one method which is split 
>> into two unrelated classes.
>
> It'd be worth to document this as javadoc of these two equals methods
> (that's probably what you are thinking).

While this could be an @implNote in the javadoc, I think as this is an 
internal implementation detail with a private type (WeakEntry) (that 
doesn't leak) we are better off just adding this as a code comment on 
MethodType#equals:

diff -r a34087e2b440 
src/java.base/share/classes/java/lang/invoke/MethodType.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodType.java Mon 
Aug 20 12:30:38 2018 +0200
+++ b/src/java.base/share/classes/java/lang/invoke/MethodType.java Mon 
Aug 20 12:46:20 2018 +0200
@@ -786,6 +786,9 @@
       * @param x object to compare
       * @see Object#equals(Object)
       */
+    // This implementation may also return true if x is a WeakEntry 
containing
+    // a method type that is equal to this. This is an internal 
implementation
+    // detail to allow for faster method type lookups.
      @Override
      public boolean equals(Object x) {
          if (this == x) {
@@ -1350,6 +1353,12 @@
                  hashcode = key.hashCode();
              }

+            /**
+             * This implementation returns true both if {@code obj} is 
another
+             * {@code WeakEntry} whose referent is equals to this 
referent, but also
+             * if {@code obj} is equals to the referent directly. This 
allows
+             * lookups to be made without wrapping in a {@code WeakEntry}.
+             */
              @Override
              public boolean equals(Object obj) {
                  Object mine = get();

/Claes