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

Chunk support for ArraySeq #1905

Merged
merged 5 commits into from
Jun 22, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions core/shared/src/main/scala-2.12/fs2/ChunkPlatform.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package fs2

import scala.collection.mutable.WrappedArray

trait ChunkPlatform[+O] { self: Chunk[O] => }

trait ChunkCompanionPlatform { self: Chunk.type =>

protected def platformIterable[O](i: Iterable[O]): Option[Chunk[O]] =
i match {
case a: WrappedArray[O] => Some(wrappedArray(a))
case _ => None
}

/**
* Creates a chunk backed by a `WrappedArray`
*/
def wrappedArray[O](wrappedArray: WrappedArray[O]): Chunk[O] =
array(wrappedArray.array.asInstanceOf[Array[O]])

}
10 changes: 0 additions & 10 deletions core/shared/src/main/scala-2.12/fs2/internal/ArrayBackedSeq.scala

This file was deleted.

45 changes: 45 additions & 0 deletions core/shared/src/main/scala-2.13/fs2/ChunkPlatform.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package fs2

import scala.collection.immutable.ArraySeq
import scala.collection.immutable
import scala.reflect.ClassTag

trait ChunkPlatform[+O] { self: Chunk[O] =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this trait and it’s companion private[fs2] both here and in 2.13 file? Otherwise looks ready for merge. Thanks!


def toArraySeq[O2 >: O: ClassTag]: ArraySeq[O2] = {
val array: Array[O2] = new Array[O2](size)
copyToArray(array)
ArraySeq.unsafeWrapArray[O2](array)
Comment on lines +11 to +12
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the most efficient way to construct an ArraySeq, given it will often take advantage of System.arraycopy. I might be missing something, though, so I'm open to feedback if there are better ways to do this.

}

def toArraySeqUntagged: ArraySeq[O] =
self match {
case knownType: Chunk.KnownElementType[O] => toArraySeq[O](knownType.elementClassTag)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We attempt to specialise the ArraySeq even if there is no ClassTag available.

case _ =>
val buf = ArraySeq.untagged.newBuilder[O]
buf.sizeHint(size)
var i = 0
while (i < size) {
buf += apply(i)
i += 1
}
buf.result()
}

}

trait ChunkCompanionPlatform { self: Chunk.type =>

protected def platformIterable[O](i: Iterable[O]): Option[Chunk[O]] =
i match {
case a: immutable.ArraySeq[O] => Some(arraySeq(a))
case _ => None
}

/**
* Creates a chunk backed by an immutable `ArraySeq`.
*/
def arraySeq[O](arraySeq: immutable.ArraySeq[O]): Chunk[O] =
array(arraySeq.unsafeArray.asInstanceOf[Array[O]])

}
10 changes: 0 additions & 10 deletions core/shared/src/main/scala-2.13/fs2/internal/ArrayBackedSeq.scala

This file was deleted.

22 changes: 12 additions & 10 deletions core/shared/src/main/scala/fs2/Chunk.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package fs2

import scala.annotation.tailrec
import scala.collection.immutable.{Queue => SQueue}
import scala.collection.{IndexedSeq => GIndexedSeq, Seq => GSeq}
import scala.collection.{mutable, IndexedSeq => GIndexedSeq, Seq => GSeq}
import scala.reflect.ClassTag
import scodec.bits.{BitVector, ByteVector}
import java.nio.{
Expand All @@ -19,7 +19,6 @@ import java.nio.{
import cats.{Alternative, Applicative, Eq, Eval, Monad, Traverse, TraverseFilter}
import cats.data.{Chain, NonEmptyList}
import cats.implicits._
import fs2.internal.ArrayBackedSeq

/**
* Strict, finite sequence of values that allows index-based random access of elements.
Expand All @@ -33,7 +32,7 @@ import fs2.internal.ArrayBackedSeq
* The operations on `Chunk` are all defined strictly. For example, `c.map(f).map(g).map(h)` results in
* intermediate chunks being created (1 per call to `map`).
*/
abstract class Chunk[+O] extends Serializable { self =>
abstract class Chunk[+O] extends Serializable with ChunkPlatform[O] { self =>

/** Returns the number of elements in this chunk. */
def size: Int
Expand Down Expand Up @@ -495,7 +494,7 @@ abstract class Chunk[+O] extends Serializable { self =>
iterator.mkString("Chunk(", ", ", ")")
}

object Chunk extends CollectorK[Chunk] {
object Chunk extends CollectorK[Chunk] with ChunkCompanionPlatform {

/** Optional mix-in that provides the class tag of the element type in a chunk. */
trait KnownElementType[A] { self: Chunk[A] =>
Expand Down Expand Up @@ -593,11 +592,8 @@ object Chunk extends CollectorK[Chunk] {

/** Creates a chunk from a `scala.collection.Iterable`. */
def iterable[O](i: collection.Iterable[O]): Chunk[O] =
i match {
case ArrayBackedSeq(arr) =>
// arr is either a primitive array or a boxed array
// cast is safe b/c the array constructor will check for primitive vs boxed arrays
array(arr.asInstanceOf[Array[O]])
platformIterable(i).getOrElse(i match {
case a: mutable.ArraySeq[O] => arraySeq(a)
case v: Vector[O] => vector(v)
case b: collection.mutable.Buffer[O] => buffer(b)
case l: List[O] =>
Expand All @@ -621,7 +617,13 @@ object Chunk extends CollectorK[Chunk] {
buffer(bldr.result)
} else singleton(head)
}
}
})

/**
* Creates a chunk backed by a mutable `ArraySeq`.
*/
def arraySeq[O](arraySeq: mutable.ArraySeq[O]): Chunk[O] =
array(arraySeq.array.asInstanceOf[Array[O]])

/** Creates a chunk backed by a `Chain`. */
def chain[O](c: Chain[O]): Chunk[O] =
Expand Down
106 changes: 106 additions & 0 deletions core/shared/src/test/scala-2.13/fs2/ChunkPlatformSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package fs2

import org.scalatest.prop.{CommonGenerators, Generator}

import scala.collection.immutable.ArraySeq
import scala.collection.{immutable, mutable}
import scala.reflect.ClassTag

class ChunkPlatformSpec extends Fs2Spec {

private implicit val genUnit: Generator[Unit] = CommonGenerators.specificValue(())
private implicit def genArraySeq[A: Generator: ClassTag]: Generator[ArraySeq[A]] =
Generator.vectorGenerator[A].map(ArraySeq.from)
private implicit def genMutableArraySeq[A: Generator: ClassTag]: Generator[mutable.ArraySeq[A]] =
implicitly[Generator[ArraySeq[A]]].map(_.to(mutable.ArraySeq))

private def testChunkToArraySeq[A](
testName: String,
shouldSpecialise: Boolean,
f: Chunk[A] => ArraySeq[A]
)(implicit generator: Generator[Chunk[A]]): Unit =
s"Chunk toArraySeq $testName" - {
"values" in forAll { chunk: Chunk[A] =>
assert(f(chunk).toVector == chunk.toVector)
}

if (shouldSpecialise)
"specialised" in forAll { chunk: Chunk[A] =>
f(chunk) match {
case _: ArraySeq.ofRef[A] =>
fail("Expected specialised ArraySeq but was not specialised")
case _ => succeed
}
}
}

private def testChunkToArraySeqTagged[A: ClassTag](testName: String, shouldSpecialise: Boolean)(
implicit generator: Generator[Chunk[A]]
): Unit =
testChunkToArraySeq[A](testName, shouldSpecialise, f = _.toArraySeq[A])

private def testChunkToArraySeqUntagged[A](
testName: String
)(implicit generator: Generator[Chunk[A]]): Unit =
testChunkToArraySeq[A](testName, shouldSpecialise = false, f = _.toArraySeqUntagged)

testChunkToArraySeqTagged[Int]("Int tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[Double]("Double tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[Long]("Long tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[Float]("Float tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[Char]("Char tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[Byte]("Byte tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[Short]("Short tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[Boolean]("Boolean tagged", shouldSpecialise = true)
testChunkToArraySeqTagged[String]("String tagged", shouldSpecialise = false)
testChunkToArraySeqTagged[Unit]("Unit tagged", shouldSpecialise = false)

testChunkToArraySeqUntagged[Int]("Int untagged")
testChunkToArraySeqUntagged[Double]("Double untagged")
testChunkToArraySeqUntagged[Long]("Long untagged")
testChunkToArraySeqUntagged[Float]("Float untagged")
testChunkToArraySeqUntagged[Char]("Char untagged")
testChunkToArraySeqUntagged[Byte]("Byte untagged")
testChunkToArraySeqUntagged[Short]("Short untagged")
testChunkToArraySeqUntagged[Boolean]("Boolean untagged")
testChunkToArraySeqUntagged[Unit]("Unit untagged")
testChunkToArraySeqUntagged[String]("String untagged")

private def testChunkFromArraySeq[A: ClassTag: Generator]: Unit = {
val testTypeName: String = implicitly[ClassTag[A]].runtimeClass.getSimpleName

s"fromArraySeq ArraySeq[$testTypeName]" - {
"mutable" in forAll { arraySeq: mutable.ArraySeq[A] =>
assert(Chunk.arraySeq(arraySeq).toVector == arraySeq.toVector)
}

"immutable" in forAll { arraySeq: immutable.ArraySeq[A] =>
assert(Chunk.arraySeq(arraySeq).toVector == arraySeq.toVector)
}
}
}

"Chunk from ArraySeq" - {
testChunkFromArraySeq[Int]
testChunkFromArraySeq[Double]
testChunkFromArraySeq[Long]
testChunkFromArraySeq[Float]
testChunkFromArraySeq[Char]
testChunkFromArraySeq[Byte]
testChunkFromArraySeq[Short]
testChunkFromArraySeq[Boolean]
testChunkFromArraySeq[Unit]
testChunkFromArraySeq[String]
}

"Chunk iterable" - {
"mutable ArraySeq" in forAll { a: mutable.ArraySeq[String] =>
assert(Chunk.iterable(a).toVector == a.toVector)
}

"immutable ArraySeq" in forAll { a: immutable.ArraySeq[String] =>
assert(Chunk.iterable(a).toVector == a.toVector)
}
}

}