RFR: 8341964: Add mechanism to disable different parts of TLS cipher suite [v4]

[Artur Barashev]

On Thu, 7 Nov 2024 20:31:58 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Actually if we do `patternCache.putIfAbsent(pattern, Pattern.compile(pattern.replace("*", ".*")))` we'll be computing Pattern on every call regardless if it's already present in cache. Also `putIfAbsent` makes no guarantees of atomicity. Better candidate here seems to be `computeIfAbsent` method, but still it would not be as fast as the current solution because it would add an extra `get` call to the cache interaction. That's how exactly `computeIfAbsent` method works:
>> 
>> `     * <pre> {@code
>>      * if (map.get(key) == null) {
>>      *     V newValue = mappingFunction.apply(key);
>>      *     if (newValue != null)
>>      *         map.put(key, newValue);
>>      * }
>>      * }</pre>
>> `
>
> Good point, `computeIfAbsent` is what you would want. Both `putIfAbsent` and `computeIfAbsent` are done atomically though (the javadoc states that).
> 
> I think it boils down to whether the existing code is ok and would not cause any unexpected behavior. AFAICT, there is a small chance for a race condition where more than one thread could end up computing and storing the same Pattern, but it should be the same algorithm, so there should be no negative side effects.

Oh, I was reading the javadoc for the base `Map` class, not for the `ConcurrentHashMap`. Yes, those operations are atomic. Also, I kind of missed the fact that `computeIfAbsent` returns the value, so it basically would be the same functionality as the current implementation but more concise and atomic. I'll make the change.
You are right that there would be no negative side effects anyhow, it's the same pattern.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1833356025