JConcurrentMapWrapper does not override default updateWith implementation

[igabaydulin]

Reproduction steps

Scala version: 2.13.8

Java's ConcurrentMap#compute default implementation and Scala's concurrent.Map#updateWith default implementation have the same logic and the supplied function may be invoked multiple times in both cases.

Java's ConcurrentHashMap extends ConcurrentMap and overrides compute implementation. It gives improved performance and guarantees that supplied function is invoked only once.

We can wrap ConcurrentHashMap into concurrent.Map

{
  import scala.jdk.CollectionConverters._
  val map: concurrent.Map[String, Int] = new ConcurrentHashMap[String, Int]().asScala
}

It will create an instance of JConcurrentMapWrapper which extends concurrent.Map, but it does not override updateWith default implementation using compute method of wrapped ConcurrentHashMap. Implementing overridden updateWith should be quite simple:

override def updateWith(key: K)(remappingFunction: Option[V] => Option[V]): Option[V] =
  Option(underlying.compute(key, (_: K, v: V) => remappingFunction(Option(v)).getOrElse(null.asInstanceOf[V])))

[SethTisue]

@scala/collections ?

[som-snytt]

The recent tweak says it is OK to add an override to the wrapper.

The suggested fix looks reasonable.

As with the recent tweak, the interface is so fat that it's difficult to correlate the delegated methods without a spreadsheet.

I'm not especially familiar with either side, but I would expect that if someone has to wrap their beloved Java Map asScala, they might know they want to use underlying.m so what must they invoke on the wrapper? I just took a few minutes to look, but it's not obvious that updateWith is the final loose end.

Possibly the delegate belongs on JMapWrapperLike, but I will desist now before 🤯 happens.

[som-snytt]

I pushed that PR.

[som-snytt]

JMapWrapperLike was not correct for the concurrent mix-in, but it seems it should delegate for all wrapped maps. The other API that could be delegated is default value. (I just wanted to contribute the desired behavior requested here.)

[SethTisue]

(There is now some discussion on the PR (scala/scala#10027) about what the right fix is.)