Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

++= in ArrayBuffer is optimized only for collection.mutable.IndexedSeq and not for all IndexedSeqs #4821

Closed
scabug opened this issue Jul 20, 2011 · 2 comments
Assignees
Labels

Comments

@scabug
Copy link

@scabug scabug commented Jul 20, 2011

The code for ++= in ArrayBuffer.scala is currently this:

  override def ++=(xs: TraversableOnce[A]): this.type = xs match {
    case v: IndexedSeq[_] =>
      val n = v.length
      ensureSize(size0 + n)
      v.copyToArray(array.asInstanceOf[scala.Array[Any]], size0, n)
      size0 += n
      this
    case _ =>
      super.++=(xs)
  }

However, the "case v: IndexedSeq[_]" only matches mutable IndexedSeq, whereas this optimization would also be valid for structures like the immutable Vector.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 20, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4821?orig=1
Reporter: Jean-Philippe Pellet (jpp)
Affected Versions: 2.8.1, 2.9.0

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Aug 15, 2011

Commit Message Bot (anonymous) said:
(extempore in r25509) A conceivably pretty bad performance bug in builders.

#4821 pointed out that ArrayBuffer's ++ checks for a cheap size
method by matching on IndexedSeq, but mutable.IndexedSeq, so all
immutable collections are thrown in the same group as linear seqs.
I went looking for other examples of this and found them, in
key classes like Builder.

The "type shadowing trap" is a serious issue in the collections.
Closes #4821, no review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.