<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">I've pushed another update that has putIfAbsent treat null values as absent. This allows putIfAbsent to be restored in a couple of the defaults. This feels like the right thing to do&nbsp;though some null-lover will eventually be unhappy that his null value got trampled.&nbsp;The remove() and replace() methods still distinguish between absent keys and present keys with null values. I think this is appropriate.<div><div><br></div><div>The alternative to treating null values as absent is to add additional code to distinguish between absent and nulls values everywhere. I attempted this in&nbsp;<div><br></div><div><a href="http://hg.openjdk.java.net/lambda/lambda/jdk/rev/8561d74a9e8f">http://hg.openjdk.java.net/lambda/lambda/jdk/rev/8561d74a9e8f</a><br><div><br></div><div>It wasn't much more successful because the value mappers use null for signalling.</div><div><div><br></div><div>The current specification and behaviour seems about as good as can be achieved while still tolerating nulls and without introducing Optional.</div><div><br></div><div><div>Mike<br><div><br><div><div>On Mar 25 2013, at 16:41 , Mike Duigou wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">My intention with the first cut of the unit test was to avoid changing the method specification while having consistent behaviour across all of the Map implementations. It's certainly possible I made mistakes and changed the contract of the defaults in ways I didn't intend.&nbsp;<div><br></div><div><div>I think perhaps that changing putIfAbsent to allow replacement of null values would help a lot to making the operations more consistent. I am also going to look at Sam's suggestion of using replace() rather than put() in a few places. Using replace() almost ensures though that we'll introduce more retry loop implementations. This may be fine though.</div><div><br></div><div>Mike</div><div><br></div><div><br><div><div>On Mar 25 2013, at 14:57 , Peter Levart wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
  
    <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
  
  <div bgcolor="#FFFFFF" text="#000000">
    Hi Mike,<br>
    <br>
    I see default Map.computeIfAbsent has been chosen to not be atomic
    and rather support null values more naturally. That's ok if JDK
    ConcurrentMap implementations provide atomic overrides. Other 3rd
    party implementations will follow soon.<br>
    <br>
    &nbsp;But I have doubts about default Map.compute(). On one hand it
    contains the usual disclaimer:<br>
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 900&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * &lt;p&gt;The default implementation makes no
    guarantees about<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 901&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * synchronization or atomicity properties of this
    method or the<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 902&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * application of the remapping function. Any class
    overriding<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 903&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * this method must specify its concurrency
    properties.&nbsp; In<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 904&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * particular, all implementations of subinterface
    {@link<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 905&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * java.util.concurrent.ConcurrentMap} must document
    whether the<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 906&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * function is applied exactly once atomically. Any
    class that<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 907&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * permits null values must document whether and how
    this method<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 908&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * distinguishes absence from null mappings.<br>
    <br>
    ...but on the other hand it tries to be smart:<br>
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 924&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * In concurrent contexts, the default implementation
    may retry<br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 925&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * these steps when multiple threads attempt updates.<br>
    <br>
    Now this last sentence may indicate that a ConcurrentMap
    implementation that does not override compute() might safely be used
    with default Map.compute() and be atomic. It was atomic until
    putIfAbsent() was replaced with plain put() to correct the "existing
    null value" behavior. The retry-loop is only needed when the
    optimistic operation is not successful. But put() is not optimistic.
    It always "succeeds". And retrying after the put() only makes things
    worse: "Oh, somebody was quicker than me and I have just overwritten
    his value - never mind, I'll try to make some more damage in next
    loop..."<br>
    <br>
    If the damage was done already, then there's no point in repeating
    the loop. Further, what's the point in using optimistic operations:
    replace(key, oldValue, newValue) and remove(key, oldValue) with
    retry-loop on one hand and then just stomping over with put() on the
    other hand. If the default Map.compute() method is declared
    non-atomic, then plain put(key, newValue) instead of replace(key,
    oldValue, newValue) and remove(key) instead of remove(key, oldValue)
    could be used and no retry-loop...<br>
    <br>
    The same goes for default Map.merge().<br>
    <br>
    Regards, Peter<br>
    <br>
    P.S. What do you think about changing the specification of
    putIfAbsent to always overwrite null values? It could make all these
    things simpler and more consistent. And it would not break anything.<br>
    <br>
    <br>
    <div class="moz-cite-prefix">On 03/25/2013 08:21 PM,
      <a class="moz-txt-link-abbreviated" href="mailto:mike.duigou@oracle.com">mike.duigou@oracle.com</a> wrote:<br>
    </div>
    <blockquote cite="mid:20130325192220.C64A0483B3@hg.openjdk.java.net" type="cite"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px solid #009900;  padding: 0px 15px; margin: 2px 0px;"><![endif]-->
      <pre wrap="">Changeset: c8d40b7e6de3
Author:    mduigou
Date:      2013-03-20 20:32 -0700
URL:       <a class="moz-txt-link-freetext" href="http://hg.openjdk.java.net/lambda/lambda/jdk/rev/c8d40b7e6de3">http://hg.openjdk.java.net/lambda/lambda/jdk/rev/c8d40b7e6de3</a>

bug fixes and unit test for Map Defaults

! src/share/classes/java/util/HashMap.java
! src/share/classes/java/util/Map.java
+ test/java/util/Map/Defaults.java


</pre>
      <!--[if !IE]></DIV><![endif]--></blockquote>
    <br>
  </div>

</blockquote></div><br></div></div></div></blockquote></div><br></div></div></div></div></div></div></div></body></html>