RFR: 8344892: beans/finder/MethodFinder.findMethod incorrectly returns null [v2]
Alexander Zvegintsev
azvegint at openjdk.org
Mon Mar 3 14:36:32 UTC 2025
On Fri, 28 Feb 2025 18:49:58 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:
>> Sorry for my last premature comment.
>>
>> I have looked at `Cache`, and believe `create` should be made protected to indicate it is not intended to be called, but only overridden.
>>
>> The preexisting code called `create` to reuse the creation mechanism without going through the cache - such usage is dubious. Per my experience, similar conditional caches are better implemented by enclosing the logic into the cache structure and use a common endpoint to access, so we have one universal site to determine the conditionality of caching. In this case, such a condition is better included in the `Cache` class itself, and the `create` method should not be publicly exposed.
>
> Yes, I'd prefer `CACHE.get(signature)`.
>
> At the same time, I've got doubts that Alexander has…
>
> Although, strictly speaking, it is not “implementation dependent” because the `Cache` class is part of OpenJDK, and I think it's reasonable to depend on its implementation.
At this point I changed it to `return CACHE.get(signature)` to fix an obvious bug and go back to the execution path that has been around for years to regain the former performance
The thread safety issue and the get/create logic will be dealt with separately in [JDK-8351043](https://bugs.openjdk.org/browse/JDK-8351043)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23845#discussion_r1977621067
More information about the client-libs-dev
mailing list