Details

      Description

      Vector should override isEmpty:

      override def isEmpty = length == 0

      Expect five fold performance increase in head, tail, last and init

        Activity

        Hide
        Simon Ochsenreither added a comment -

        I looked at the code. The issue seems to be that Vector#isEmpty uses IterableLike's implementation !iterator.hasNext.

        This in turn invokes Vector#iterator, which does a lot of work:

          @inline override def iterator: VectorIterator[A] = {
            val s = new VectorIterator[A](startIndex, endIndex)
            initIterator(s)
            s
          }
        

        Using Vector#length to implement Vector#isEmpty seems to make sense, because it is only a comparison and an arithmetic operation: def length = def length = endIndex - startIndex

        Show
        Simon Ochsenreither added a comment - I looked at the code. The issue seems to be that Vector#isEmpty uses IterableLike 's implementation !iterator.hasNext . This in turn invokes Vector#iterator , which does a lot of work: @inline override def iterator: VectorIterator[A] = { val s = new VectorIterator[A](startIndex, endIndex) initIterator(s) s } Using Vector#length to implement Vector#isEmpty seems to make sense, because it is only a comparison and an arithmetic operation: def length = def length = endIndex - startIndex
        Hide
        Jason Zaugg added a comment -

        I'd suggest that isEmpty should be overridden in SeqLike in terms of lengthCompare.

        Show
        Jason Zaugg added a comment - I'd suggest that isEmpty should be overridden in SeqLike in terms of lengthCompare .
        Hide
        Simon Ochsenreither added a comment - - edited

        Thanks for reporting this issue, Juha!

        Pull request: https://github.com/scala/scala/pull/1036

        Edit: Sorry, Jason, I saw your comment too late. I'll look into it.

        Show
        Simon Ochsenreither added a comment - - edited Thanks for reporting this issue, Juha! Pull request: https://github.com/scala/scala/pull/1036 Edit: Sorry, Jason, I saw your comment too late. I'll look into it.
        Hide
        Simon Ochsenreither added a comment - - edited

        Jason, are you sure? Won't this also lead to the expensive creation of Vector#iterator: https://github.com/scala/scala/blob/master/src/library/scala/collection/SeqLike.scala#L87 ?

        Show
        Simon Ochsenreither added a comment - - edited Jason, are you sure? Won't this also lead to the expensive creation of Vector#iterator : https://github.com/scala/scala/blob/master/src/library/scala/collection/SeqLike.scala#L87 ?
        Hide
        Jason Zaugg added a comment -

        lengthCompare itself is overridden in IndexedSeqOptimized.

        Show
        Jason Zaugg added a comment - lengthCompare itself is overridden in IndexedSeqOptimized .
        Hide
        Simon Ochsenreither added a comment -

        I'll close this issue because the fix has been added to both the 2.10.x branch and trunk.

        @Juha: The fix is different to the code you proposed, because we tried to enable other collection classes to benefit from this change, too. Would you be so kind to check if the performance improvement is what you expect, and re-open this ticket if not?

        Show
        Simon Ochsenreither added a comment - I'll close this issue because the fix has been added to both the 2.10.x branch and trunk. @Juha: The fix is different to the code you proposed, because we tried to enable other collection classes to benefit from this change, too. Would you be so kind to check if the performance improvement is what you expect, and re-open this ticket if not?

          People

          • Assignee:
            Simon Ochsenreither
            Reporter:
            Juha Heljoranta
          • Votes:
            1 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development