Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-03-11 Thread Paul Sandoz
On Mar 11, 2015, at 12:34 AM, Brent Christian brent.christ...@oracle.com wrote: Hi, Paul On 3/10/15 8:29 AM, Paul Sandoz wrote: On the Map.compute* methods. Perhaps we can reuse similar language to that we added for Matcher: * The mapping function should not modify this map during

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-03-11 Thread Brent Christian
Hi, Here is an updated treatment of computeIfAbsent(): http://cr.openjdk.java.net/~bchristi/8071667/webrev.1/ There's a webrev, as well as built docs for Map[1], ConcurrentMap[2], and HashMap[3] (and I now realize there should be Hashtable as well). I've made use of @implSpec and friends in

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-03-10 Thread Paul Sandoz
Hi Brent, On the Map.compute* methods. Perhaps we can reuse similar language to that we added for Matcher: The mapping function should not modify this map during computation. This method will, on a best-effort basis, throw a ConcurrentModification if such modification is detected. It's

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-03-10 Thread Brent Christian
Hi, Paul On 3/10/15 8:29 AM, Paul Sandoz wrote: On the Map.compute* methods. Perhaps we can reuse similar language to that we added for Matcher: * The mapping function should not modify this map during computation. * This method will, on a best-effort basis, throw a * ConcurrentModification

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-03-06 Thread Ben Manes
Nevermind, sorry somehow I missed the previous emails in the thread. On Friday, March 6, 2015 3:57 PM, Ben Manes ben_ma...@yahoo.com wrote: I'd recommend that the exception thrown should be an IllegalStateException. This is documented in ConcurrentHashMap's computeIfAbsent as, * @throws

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-03-06 Thread Ben Manes
I'd recommend that the exception thrown should be an IllegalStateException. This is documented in ConcurrentHashMap's computeIfAbsent as, * @throws IllegalStateException if the computation detectably * attempts a recursive update to this map that would * otherwise never complete

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-03-06 Thread Brent Christian
Hi. I'm picking this back up now. In Map/HashMap, there are already two methods that accept lambdas and throw a CME: forEach(BiConsumer) replaceAll(BiFunction) The Map interface documents these methods to state that Exceptions are relayed to the caller, and has an @throws CME tag if an

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-06 Thread Brent Christian
On 2/5/15 12:46 AM, Paul Sandoz wrote: On Feb 5, 2015, at 1:36 AM, Brent Christian brent.christ...@oracle.com wrote: I prefer this approach of discouraging/preventing side-effects via CME, rather than allowing them. ... Regarding the default methods: Would we be able to make a best-effort

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-05 Thread Paul Sandoz
On Feb 5, 2015, at 1:45 AM, Doug Lea d...@cs.oswego.edu wrote: On 02/04/2015 05:01 AM, Paul Sandoz wrote: So i propose: - the functions should be side-effect free. ... - concurrent map implementations should, on a best-effort basis, detect non-termination situations and fail with

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-05 Thread Paul Sandoz
On Feb 5, 2015, at 1:36 AM, Brent Christian brent.christ...@oracle.com wrote: I prefer this approach of discouraging/preventing side-effects via CME, rather than allowing them. Keep the functions functional, as it were. If there are situations where determining the mapping for one key

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-04 Thread Paul Sandoz
Hi, I think we should as consistent as possible about the functions being side-effect free when applied to bulk operations. A method such as computeIfAbsent can be viewed as a bulk operation in that it may perform two or more dependent actions (they are just not as bulky as forEach). It's

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-04 Thread Brent Christian
I prefer this approach of discouraging/preventing side-effects via CME, rather than allowing them. Keep the functions functional, as it were. If there are situations where determining the mapping for one key necessitates making additional changes to the Map, that should be coded some other

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-04 Thread Doug Lea
On 02/04/2015 05:01 AM, Paul Sandoz wrote: So i propose: - the functions should be side-effect free. ... - concurrent map implementations should, on a best-effort basis, detect non-termination situations and fail with ISE. We did this as part of changes to better detect recursive

Re: RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-03 Thread Stuart Marks
On 2/3/15 4:01 PM, Brent Christian wrote: The code in bug 8071667 [1] passes a mappingFunction to computeIfAbsent() which itself put()s a sufficient number of additional entries into the HashMap to cause a resize/rehash. As a result, computeIfAbsent() doesn't add the new entry at the proper

RFC: Adding ConcurrentModificationException for HashMap.computeIfAbsent(), and JDK-8071667

2015-02-03 Thread Brent Christian
Hi, The code in bug 8071667 [1] passes a mappingFunction to computeIfAbsent() which itself put()s a sufficient number of additional entries into the HashMap to cause a resize/rehash. As a result, computeIfAbsent() doesn't add the new entry at the proper place in the HashMap. While one