On Fri, 28 Feb 2025 18:49:58 GMT, Alexey Ivanov <[email protected]> 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
