[13] RFR JDK-7107615 "scalability bloker in javax.crypto.JceSecurity"

[Valerie Peng]

I updated the webrev to switch to ConcurrentHashMap. The javadoc spec of 
computeIfAbsent method cautioned that the mapping func should be short 
and simple and must not attempt to update other mappings of this map. 
Provider verification code does not quite fit the above criteria for the 
mapping. So, I did not use computeIfAbsent method and just made minor 
update to webrev.01 with Xuelei's suggestion of re-checking the cache 
again inside the synchronized block.

http://cr.openjdk.java.net/~valeriep/7107615/webrev.03/

Comments?

Thanks,
Valerie

On 5/13/2019 3:32 PM, Valerie Peng wrote:
>
> Hmm, thanks much for the feedback on how to use WeakReference. This 
> bug/rfe is filed to address the scalability. However, it is noticed 
> that storing Provider objects into the map have some side effect on 
> memory, so I was trying to address both with minimum number of new 
> code. Guess it does not quite work as I expected...
>
> The priority is the scalability.  I am not sure if it's worthwhile to 
> address the memory-side-effect by adding another reference queue + 
> purge the map + re-verify the provider if the underlying provider is 
> GC'ed. So, I will update the webrev with CHM then.
>
> Thanks again for the feedback,
> Valerie
> On 5/13/2019 2:36 AM, Alan Bateman wrote:
>>
>>
>> On 13/05/2019 10:04, Daniel Fuchs wrote:
>>> Hi Valery,
>>>
>>> On 11/05/2019 00:36, Valerie Peng wrote:
>>>> http://cr.openjdk.java.net/~valeriep/7107615/webrev.02/
>>>>
>>>> Please let me know if you have more comments.
>>>
>>> If I'm not mistaken, the only thing that references
>>> the IdentityWrapper<Provider> is the key in the WeakHashMap.
>>> Therefore, it is only weakly referenced and can be immediatly
>>> garbage collected. I don't think this is what you want?
>>>
>>> I believe what you are trying to achieve there is rather to use
>>> a plain ConcurrentHashMap, and have IdentityWrapper extend
>>> WeakReference<Provider> instead. You may need to store
>>> the hashCode in IdentityWrapper so that it doesn't change
>>> when the underlying Provider is garbage collected.
>>>
>>> Then you can use a ReferenceQueue to purge the map regularly.
>> Right, plus getVerificationResult is accessing the WeakHashMap 
>> without synchronization.
>>
>> I think it would be useful to get a summary on whether this issue is 
>> trying to address one or two points. For the scalability point then I 
>> assume a CHM + computeIfAbsent would help. If the issue is also that 
>> you want the key (Provider) to be weak then it will require 
>> additional work, as Daniel points out.
>>
>> -Alan
>>