On Mon, 3 Mar 2025 16:52:04 GMT, Alexander Zvegintsev <[email protected]> wrote:
>> During the [JDK-8344891 Remove uses of sun.misc.ReflectUtil in >> java.desktop](https://bugs.openjdk.org/browse/JDK-8344891) it was discovered >> that `beans/finder/MethodFinder.findMethod' incorrectly returned null if a >> signature was not in the cache and added it to the cache if it was already >> there: >> >> >> Method method = CACHE.get(signature); >> return (method == null) ? method : CACHE.create(signature); >> >> This resulted in a [significant drop in >> performance](https://bugs.openjdk.org/browse/JDK-8350573). >> >> ---- >> >> Before ReflectUtil was removed, it worked by coincidence: >> >> >> Method method = CACHE.get(signature); >> return (method == null) || isPackageAccessible(method.getDeclaringClass()) ? >> method : CACHE.create(signature); >> >> >> >> >> 1. `Cache#get` non-obviously creates a value in Cache, this in turn allowed >> us to avoid the NPE in the `(method == null) || >> isPackageAccessible(method.getDeclaringClass())` condition >> >> >> https://github.com/openjdk/jdk/blob/d6c4be672f6348f8ed985416ed90d0447f5d5bb3/src/java.desktop/share/classes/com/sun/beans/util/Cache.java#L120-L126 >> >> 2. `isPackageAccessible(method.getDeclaringClass())` was evaluated as true >> >> This is how we previously returned the cached value. >> >> --- >> >> So the solution is obvious: >> >> >> Method method = CACHE.get(signature); >> return (method != null) ? method : CACHE.create(signature); >> >> >> Testing is green. > > Alexander Zvegintsev has updated the pull request incrementally with one > additional commit since the last revision: > > year update >Cache#get non-obviously creates a value in Cache Then why we need to call CACHE.create the second time? Can we just return it as is? `return CACHE.get(signature);` I think previously, we couldn't return the cached data as is, since it could be cached by one applet and requested by another. So, in the case of an inaccessible package, we call CACHE.create() and return a duplicate. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23845#issuecomment-2695593424
