On Fri, 27 Oct 2023 22:22:46 GMT, Ben Perez <d...@openjdk.org> wrote:
>> Modified `getService` method to prevent caching of `ServiceKey`, which was >> negatively impacting multithreaded performance > > Ben Perez has updated the pull request incrementally with one additional > commit since the last revision: > > changed fix to align with bug report The problem with not caching the `ServiceKey`, AFAICS, is that we are going to do `toUpperCase` and `intern` in `ServiceKey` constructor. The problem *with* caching the key comes from the contention on a single variable, if we ping-pong different `ServiceKey`-s through that single-slot cache. Dropping `volatile` should not help much, because the first-order effect is contention, not additional synchronization. The easy mitigation would be updating `previousKey` conditionally. For example, when the cache is `null` and have not been initialized. Single-slot caches are problematic like that. Something like `ConcurrentMap<Pair<String,String>, WeakReference<ServiceKey>>` would mitigate update-side problems with single-slot cache. But it remains to be seen if it is better than accepting `toUpperCase` and `intern`. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16403#issuecomment-1784965220
