RFR(trivial): 8222394: HashMap.compute() throws CME on an empty Map if clear() called concurrently

[Patrick Zhang OS]

> [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060068.html
Thanks for sharing this thread. 
I saw Pavel's comments: "I believe it should be incremented on an attempt to modify the collection, rather than on the effective result of that attempt"
It can be true for bulk modifications like addAll, clear, while for single-entry modifications like put, remove, merge, etc. it depends, some are unconditional ++ and most are conditional ++. 
So the clarification should be not specific to clear or addAll, but at a common place (if possible) in a higher level javadoc for modCount, or for "fail fast".

Regards
Patrick

-----Original Message-----
From: Stuart Marks <stuart.marks at oracle.com> 
Sent: Thursday, May 9, 2019 5:01 AM
To: Patrick Zhang OS <patrick at os.amperecomputing.com>
Cc: core-libs-dev <core-libs-dev at openjdk.java.net>
Subject: Re: RFR(trivial): 8222394: HashMap.compute() throws CME on an empty Map if clear() called concurrently



On 5/4/19 12:21 AM, Patrick Zhang OS wrote:
> Thank you very much for the detailed explanation and examples, which solved most of my previous questions and confusion. Really appreciate that. I agree with the opinion about "effort vs benefit".
> 
> Re-thought my original tests concerning CMEs that passed with jdk8u but failed with jdk11 (should be 9+, but I only checked LTS builds), I was struggling between "fixing" the "problems" in jdk11 and "making jdk8 fail similarly". However so far it looks these tests might be over strict, or "verifying CMEs" itself might be an invalid (or unreliable) operation, perhaps I should drop all of them. Lastly, a suggestion would be: adding more comments for this in case anyone else would revisit it with similar confusions, e.g. HashMap.clear.

Great, I'm glad this was helpful.

Regarding the tests, yes, I'm afraid those might be overly strict. The specification is admittedly quite loose in this area. This means that the exact behavior may vary from release to release. If a collection is optimized, for example, sometimes it's quite difficult to preserve the exact same behavior in all cases. It's for this reason we avoid specifying the exact behaviors around CME. Of course, when making any changes, we usually do try to preserve the general behavior, just not every specific edge case.

I generally welcome code comments to improve clarity, but I'm not sure one is warranted for HashMap.clear(). The placement of modCount++ seems quite reasonable. There are other cases where the choice of conditional-vs-unconditional is made (see [1]) and I don't think we want to go sprinkling explanations of this all around the code.

[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060068.html

s'marks