Scala Programming Language
  1. Scala Programming Language
  2. SI-4821

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: Scala 2.8.1, Scala 2.9.0
    • Fix Version/s: None
    • Component/s: Misc Library
    • Labels:
      None
    • Environment:

      Mac OS X 10.6.8

      Description

      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.

        Activity

        Hide
        Commit Message Bot added a comment -

        (extempore in r25509) A conceivably pretty bad performance bug in builders.

        SI-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 SI-4821, no review.

        Show
        Commit Message Bot added a comment - (extempore in r25509 ) A conceivably pretty bad performance bug in builders. SI-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 SI-4821 , no review.

          People

          • Assignee:
            Paul Phillips
            Reporter:
            Jean-Philippe Pellet
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development