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
