On Fri, 7 Jul 2023 16:31:47 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Chen Liang has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> Fix broken null behaviors > > FYI. You can do git revert instead of force push. @mlchung Thank you for the new round of review. I have split the large test into 3 parts, testing general contracts, against different types of interfaces, and implementation-related tests. The old MethodHandleProxies test, which tests against different types of interfaces, is merged into the interface one. The module one, being an implementation detail, is merged into the implementation test. I have a few problems that need help: 1. I think the type profile pollution only happens with the instance-field approach, as I recall different instance fields' MHs pollute profiling. The comment need to be corrected if I'm right. 2. The weak reference test in the impl-related test breaks in weird ways: 1. https://github.com/openjdk/jdk/pull/13197/files#diff-effc9e10eca79d537b6321e9d72b8bbcf0fc6c1c2ac9d13f3524951bc54d48fdR196 When this line is removed, the next call to `asInterfaceInstance` will return a wrapper with a new implementation class, even if the old instance is still in a local variable; as in jshell: ```java import java.lang.invoke.*; var mh = MethodHandles.zero(void.class); var c1 = MethodHandleProxies.asInterfaceInstance(Runnable.class, mh); System.gc(); var c2 = MethodHandleProxies.asInterfaceInstance(Runnable.class, mh); c1.getClass() == c2.getClass() ``` The last `==` line shows that `c1` and `c2` are both reachable, so there are 2 coexistent implementation classes somehow; not sure how the original one is lost in gc. 2. https://github.com/openjdk/jdk/pull/13197/files#diff-effc9e10eca79d537b6321e9d72b8bbcf0fc6c1c2ac9d13f3524951bc54d48fdR202 The MHP implementation class weak reference is not cleared by gc, even though the wrapper is no longer reachable. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13197#issuecomment-1630814119
