Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType
Ok, will do. On 2018-08-20 13:31, Peter Levart wrote: Hi Claes, That's ok. You could also add @see tag to WeakEntry.equals pointing to MethodType.equals. The other way around is not possible, but you could spell-out the same: "See also WeakEntry.equals()"
Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType
Hi Claes,
That's ok. You could also add @see tag to WeakEntry.equals pointing to
MethodType.equals. The other way around is not possible, but you could
spell-out the same: "See also WeakEntry.equals()"
Regards, Peter
On 08/20/2018 12:56 PM, Claes Redestad wrote:
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
Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType
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
Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType
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 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). Mandy
Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType
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. 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. Peter
Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType
Hi Peter, On 2018-08-17 16:04, Peter Levart wrote: Hi Claes, Nice trick. 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? /Claes
Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType
Hi Claes, Nice trick. 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. Regards, Peter On 08/17/2018 12:56 PM, Claes Redestad wrote: Hi, a small improvement can be seen on startup tests that exercise lambdas and ISC if we change the interning mechanism of MethodTypes slightly so that lookup can be done without wrapping the lookup key in a WeakEntry. Bug: https://bugs.openjdk.java.net/browse/JDK-8209633 Webrev: http://cr.openjdk.java.net/~redestad/8209633/open.00/ Testing: jdk-tier1+jdk-tier2 Thanks! /Claes
