[PING] RFR(s): 8176894 Provide specialized implementation for default methods putIfAbsent, computeIfAbsent, computeIfPresent, compute, merge in TreeMap

[Tagir Valeev]

Thank you for the review! Here's the updated version:

http://cr.openjdk.java.net/~tvaleev/webrev/8176894/r5/

Martin,
> 676         V newValue;
> 677         int mc = modCount;
> 678         newValue = mappingFunction.apply(key);
> I would never use a C style declaration when an initializing declaration would work equally well.

Nice catch, thanks! Fixed

> Prevailing whitespace style here is Entry<K,V> rather than Entry<K, V> but Intellij is on your side

Fixed in my changes as well. Though there are still eight occurrences
of <K, V> outside of my changes.

Thomas,
> remapValue:
>
>  711         } else {
>  712             // replace old mapping
>  713             t.value = newValue;
>  714             return newValue;
>  715         }
>
> Should we increase the modification count here? AFAICS Map::computeIfPresent(), Map::compute(), Map::merge() use put() to replace existing value which would have increased TreeMap::modCount .

Florian is right: it's not a structural modification. E.g.
Entry.setValue() doesn't increase modCount. Also, same operations on
HashMap don't update it as well.

> At various places, the "@throws ConcurrentModificationException" javadoc:
>
>  619      * @throws ConcurrentModificationException if it is detected that the
>  620      * mapping function modified this map
>
> is missing the period.

Done.

> We could exchange all the various
>
> if (key == null)
>    throw new NullPointerException();
>
> lines on comparator==null paths with
>
> Objects.requireNonNull(key).

Done. Also, updated getEntry() similarly (was not changed before).

> Style nits & bikeshedding:
>
> I would place both versions of getNewValueAndCheckModification() together with the other new low level utility functions (addEntry, addEntryToEmptyMap). I may also have named them somewhat differently, maybe "callMappingFunctionWithCheck" resp. "callRemappingFunctionWithCheck".

I grouped together all the added private methods and renamed, as per
your suggestion.

> private void addEntry(K key, V value, int cmp, Entry<K, V> parent) {
>
> For clarity, could we make cmp a boolean "leftOrRight" or are you afraid that would cost too much performance?

I don't think this should harm the performance significantly. However,
"leftOrRight" name sounds confusing for boolean parameter (it's not
evident whether 'true' means 'left' or 'right'). So I named the
parameter "addToLeft".

I'd like to push this changeset in 3-4 days unless I see any
objections or new review comments. Thanks again!

With best regards,
Tagir Valeev.