scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

.toMap seems to be significantly slower than .to(mutable.Map) #12400

Closed pshirshov closed 3 years ago

pshirshov commented 3 years ago

I have some chunk of code which looks like:

            val r =
              (mv.offsets(row) until mv.offsets(row) + mv.lengths(
                row
              ))
                .map { i =>
                  val ii = i.intValue()
                  val name =
                    new String(kv.vector(ii), kv.start(ii), kv.length(ii))
                  val value = vv.vector(ii)
                  (name, value)

                }
                .toMap

           evaluateJsPolyglot(r, expr1)
           evaluateJsPolyglot(r, expr2)

This snippet is being invoked about 40 million times and my test takes about 8 minutes on my machine. When I replace .toMap with .to(mutable.Map) the test takes just 4 minutes.

So, I'm building one such map per two javascript expression evalutations (with graal polyglot). I believe js evaluation should be orders of magnitude slower than map creation but in fact map creations account for HALF of the CPU time.

This is counterintuitive and, I believe that shouldn't be the case at all. Why does it happen?.. Can this behavior be improved?..

I didn't make a repro yet nor tried to actually profile this, but I hope it shouldn't be necessary.

From what I can see in Map#addOne, Growable#addAll and Map#from, toMap uses a mutable buffer, so something seems to be odd here.

Ichoran commented 3 years ago

It is entirely possible to have an immutable map that simply is a proxy to a mutable map that you can never get access to so it's totally immutable. It may be slow for many operations, but so is ListMap. There may be some performance penalty from the proxying, but it should still be better at construction speed than the immutable map.

johnynek commented 3 years ago

@Ichoran yes that has been proposed in a few variants in this thread. My point is that the slow methods are about modification and often you never modify a Map.

Ichoran commented 3 years ago

@johnynek - I've noticed, but people keep bringing up objections as if the point hasn't been made. So I'm making it again.

SethTisue commented 3 years ago

closing as per my previous remark:

I suggest we close this ticket because there are too many issues here. Discussion can continue here, but if any concrete changes are to be made, let's open new, more specific tickets about them.

NthPortal commented 3 years ago

we could add a CopyOnWrite family of collections, though collection-contrib seems like a more appropriate location for them