Skip to content

Commit

Permalink
Respond to Oscar's review comments.
Browse files Browse the repository at this point in the history 
In particular, this adds retrying to the infiniteStream, stringOf, and
stringOfN combinators. It also slightly optimizes the loop conditions
on buildableOfN, removes lazy from most arbitrary definitions, and
cleans up a few other things.

This commit does appear to have made some benchmarks slower, although
it's possible my machine is just more busy than it was. I've also
added a few more benchmarks.

Benchmark                   (genSize)  (seedCount)  Mode  Cnt     Score      Error  Units
GenBench.arbitraryString          100          100  avgt    3   600.706 ±  569.014  us/op
GenBench.asciiPrintableStr        100          100  avgt    3   432.235 ±  229.533  us/op
GenBench.const_                   100          100  avgt    3     2.775 ±    8.017  us/op
GenBench.double_                  100          100  avgt    3     9.941 ±    3.020  us/op
GenBench.dynamicFrequency         100          100  avgt    3   481.478 ±  253.262  us/op
GenBench.eitherIntInt             100          100  avgt    3    30.911 ±   13.071  us/op
GenBench.identifier               100          100  avgt    3   186.688 ±  327.920  us/op
GenBench.int_                     100          100  avgt    3    11.266 ±    8.500  us/op
GenBench.listOfInt                100          100  avgt    3   445.506 ±  403.799  us/op
GenBench.mapOfIntInt              100          100  avgt    3  1910.653 ± 2974.722  us/op
GenBench.oneOf                    100          100  avgt    3    15.945 ±   10.462  us/op
GenBench.optionInt                100          100  avgt    3    42.815 ±   18.030  us/op
GenBench.sequence                 100          100  avgt    3   205.571 ±   42.976  us/op
GenBench.staticFrequency          100          100  avgt    3   510.956 ±  111.016  us/op
GenBench.testFilter               100          100  avgt    3  1081.890 ±  607.106  us/op
GenBench.zipIntInt                100          100  avgt    3    27.987 ±   22.614  us/op
  • Loading branch information
@non
non committed Sep 28, 2019
1 parent 126168f commit 3cbcf07
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 70 deletions.
64 changes: 32 additions & 32 deletions src/main/scala/org/scalacheck/Arbitrary.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,29 +80,22 @@ private[scalacheck] sealed trait ArbitraryLowPriority {

/**** Arbitrary instances for each AnyVal ****/

/** Arbitrary AnyVal */
implicit lazy val arbAnyVal: Arbitrary[AnyVal] = Arbitrary(oneOf(
arbitrary[Unit], arbitrary[Boolean], arbitrary[Char], arbitrary[Byte],
arbitrary[Short], arbitrary[Int], arbitrary[Long], arbitrary[Float],
arbitrary[Double]
))

/** Arbitrary instance of Boolean */
implicit lazy val arbBool: Arbitrary[Boolean] =
implicit val arbBool: Arbitrary[Boolean] =
Arbitrary(oneOf(true, false))

/** Arbitrary instance of Int */
implicit lazy val arbInt: Arbitrary[Int] = Arbitrary(
implicit val arbInt: Arbitrary[Int] = Arbitrary(
Gen.chooseNum(Int.MinValue, Int.MaxValue)
)

/** Arbitrary instance of Long */
implicit lazy val arbLong: Arbitrary[Long] = Arbitrary(
implicit val arbLong: Arbitrary[Long] = Arbitrary(
Gen.chooseNum(Long.MinValue, Long.MaxValue)
)

/** Arbitrary instance of Float */
implicit lazy val arbFloat: Arbitrary[Float] = Arbitrary(
implicit val arbFloat: Arbitrary[Float] = Arbitrary(
for {
s <- choose(0, 1)
e <- choose(0, 0xfe)
Expand All @@ -111,7 +104,7 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
)

/** Arbitrary instance of Double */
implicit lazy val arbDouble: Arbitrary[Double] = Arbitrary(
implicit val arbDouble: Arbitrary[Double] = Arbitrary(
for {
s <- choose(0L, 1L)
e <- choose(0L, 0x7feL)
Expand All @@ -120,53 +113,60 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
)

/** Arbitrary instance of Char */
implicit lazy val arbChar: Arbitrary[Char] =
implicit val arbChar: Arbitrary[Char] =
Arbitrary(Gen.unicodeChar)

/** Arbitrary instance of Byte */
implicit lazy val arbByte: Arbitrary[Byte] =
implicit val arbByte: Arbitrary[Byte] =
Arbitrary(Gen.chooseNum(Byte.MinValue, Byte.MaxValue))

/** Arbitrary instance of Short */
implicit lazy val arbShort: Arbitrary[Short] =
implicit val arbShort: Arbitrary[Short] =
Arbitrary(Gen.chooseNum(Short.MinValue, Short.MaxValue))

/** Absolutely, totally, 100% arbitrarily chosen Unit. */
implicit lazy val arbUnit: Arbitrary[Unit] =
implicit val arbUnit: Arbitrary[Unit] =
Arbitrary(Gen.const(()))

/** Arbitrary AnyVal */
implicit val arbAnyVal: Arbitrary[AnyVal] = Arbitrary(oneOf(
arbitrary[Unit], arbitrary[Boolean], arbitrary[Char], arbitrary[Byte],
arbitrary[Short], arbitrary[Int], arbitrary[Long], arbitrary[Float],
arbitrary[Double]
))

/**** Arbitrary instances of other common types ****/

/** Arbitrary instance of String */
implicit lazy val arbString: Arbitrary[String] =
implicit val arbString: Arbitrary[String] =
Arbitrary(Gen.unicodeStr)

/** Arbitrary instance of Date */
implicit lazy val arbDate: Arbitrary[java.util.Date] =
implicit val arbDate: Arbitrary[java.util.Date] =
Arbitrary(Gen.calendar.map(_.getTime))

/** Arbitrary instance of Calendar */
implicit lazy val arbCalendar: Arbitrary[java.util.Calendar] =
implicit val arbCalendar: Arbitrary[java.util.Calendar] =
Arbitrary(Gen.calendar)

/** Arbitrary instance of Throwable */
implicit lazy val arbThrowable: Arbitrary[Throwable] =
implicit val arbThrowable: Arbitrary[Throwable] =
Arbitrary(oneOf(const(new Exception), const(new Error)))

/** Arbitrary instance of Exception */
implicit lazy val arbException: Arbitrary[Exception] =
implicit val arbException: Arbitrary[Exception] =
Arbitrary(const(new Exception))

/** Arbitrary instance of Error */
implicit lazy val arbError: Arbitrary[Error] =
implicit val arbError: Arbitrary[Error] =
Arbitrary(const(new Error))

/** Arbitrary instance of UUID */
implicit lazy val arbUuid: Arbitrary[java.util.UUID] =
implicit val arbUuid: Arbitrary[java.util.UUID] =
Arbitrary(Gen.uuid)

/** Arbitrary BigInt */
implicit lazy val arbBigInt: Arbitrary[BigInt] = {
implicit val arbBigInt: Arbitrary[BigInt] = {
val long: Gen[Long] =
Gen.choose(Long.MinValue, Long.MaxValue).map(x => if (x == 0) 1L else x)

Expand All @@ -191,7 +191,7 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
}

/** Arbitrary BigDecimal */
implicit lazy val arbBigDecimal: Arbitrary[BigDecimal] = {
implicit val arbBigDecimal: Arbitrary[BigDecimal] = {
import java.math.MathContext, MathContext._

val genMathContext0: Gen[MathContext] =
Expand Down Expand Up @@ -243,7 +243,7 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
}

/** Arbitrary java.lang.Number */
implicit lazy val arbNumber: Arbitrary[Number] = {
implicit val arbNumber: Arbitrary[Number] = {
val gen = Gen.oneOf(
arbitrary[Byte], arbitrary[Short], arbitrary[Int], arbitrary[Long],
arbitrary[Float], arbitrary[Double]
Expand All @@ -254,7 +254,7 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
}

/** Arbitrary instance of FiniteDuration */
implicit lazy val arbFiniteDuration: Arbitrary[FiniteDuration] =
implicit val arbFiniteDuration: Arbitrary[FiniteDuration] =
Arbitrary(Gen.finiteDuration)

/**
Expand All @@ -263,11 +263,11 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
* In addition to `FiniteDuration` values, this can generate `Duration.Inf`,
* `Duration.MinusInf`, and `Duration.Undefined`.
*/
implicit lazy val arbDuration: Arbitrary[Duration] =
implicit val arbDuration: Arbitrary[Duration] =
Arbitrary(Gen.duration)

/** Generates an arbitrary property */
implicit lazy val arbProp: Arbitrary[Prop] = {
implicit val arbProp: Arbitrary[Prop] = {
import Prop._
val undecidedOrPassed = forAll { b: Boolean =>
b ==> true
Expand All @@ -283,7 +283,7 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
}

/** Arbitrary instance of test parameters */
implicit lazy val arbTestParameters: Arbitrary[Test.Parameters] =
implicit val arbTestParameters: Arbitrary[Test.Parameters] =
Arbitrary(for {
_minSuccTests <- choose(10,200)
_maxDiscardRatio <- choose(0.2f,10f)
Expand All @@ -300,7 +300,7 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
)

/** Arbitrary instance of gen params */
implicit lazy val arbGenParams: Arbitrary[Gen.Parameters] =
implicit val arbGenParams: Arbitrary[Gen.Parameters] =
Arbitrary(for {
sz <- arbitrary[Int] suchThat (_ >= 0)
} yield Gen.Parameters.default.withSize(sz))
Expand All @@ -309,7 +309,7 @@ private[scalacheck] sealed trait ArbitraryLowPriority {
// Specialised collections //

/** Arbitrary instance of scala.collection.BitSet */
implicit lazy val arbBitSet: Arbitrary[collection.BitSet] = Arbitrary(
implicit val arbBitSet: Arbitrary[collection.BitSet] = Arbitrary(
buildableOf[collection.BitSet,Int](sized(sz => choose(0,sz)))
)

Expand Down
96 changes: 58 additions & 38 deletions src/main/scala/org/scalacheck/Gen.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ object Gen extends GenArities with GenVersionSpecific {
r(None, seed).copy(l = labels)
case Some(t) =>
val r = f(t)
r.copy(l = labels ++ r.labels, sd = r.seed)
r.copy(l = labels | r.labels, sd = r.seed)
}
}

Expand Down Expand Up @@ -626,25 +626,27 @@ object Gen extends GenArities with GenVersionSpecific {
evb: Buildable[T,C], evt: C => Traversable[T]
): Gen[C] = {
require(n >= 0, s"invalid size given: $n")
gen { (p, seed0) =>
var seed: Seed = seed0
val bldr = evb.builder
val allowedFailures = Integer.max(10, n / 10)
var failures = 0
var count = 0
while (count < n && failures < allowedFailures) {
val gres = g.doApply(p, seed)
gres.retrieve match {
case Some(t) =>
bldr += t
count += 1
case None =>
failures += 1
new Gen[C] {
def doApply(p: P, seed0: Seed): R[C] = {
var seed: Seed = seed0
val bldr = evb.builder
val allowedFailures = Gen.collectionRetries(n)
var failures = 0
var count = 0
while (count < n) {
val res = g.doApply(p, seed)
res.retrieve match {
case Some(t) =>
bldr += t
count += 1
case None =>
failures += 1
if (failures >= allowedFailures) return r(None, res.seed)
}
seed = res.seed
}
seed = gres.seed
r(Some(bldr.result), seed)
}
val res = if (count == n) Some(bldr.result) else None
r(res, seed)
}
}

Expand Down Expand Up @@ -711,20 +713,27 @@ object Gen extends GenArities with GenVersionSpecific {
* is equal to calling <code>containerOfN[Map,(T,U)](n,g)</code>. */
def mapOfN[T,U](n: Int, g: Gen[(T, U)]) = buildableOfN[Map[T, U],(T, U)](n, g)

/** Generates an infinite stream. */
/**
* Generates an infinite stream.
*
* Failures in the underlying generator may terminate the stream.
* Otherwise it will continue forever.
*/
def infiniteStream[T](g: => Gen[T]): Gen[Stream[T]] = {
def unfold(p: P, seed: Seed): Stream[T] = {
val r = g.doPureApply(p, seed)
r.retrieve match {
case Some(t) => t #:: unfold(p, r.seed)
case None => Stream.empty
val attemptsPerItem = 10
def unfold(p: P, seed: Seed, attemptsLeft: Int): Stream[T] =
if (attemptsLeft <= 0) {
Stream.empty
} else {
val r = g.doPureApply(p, seed)
r.retrieve match {
case Some(t) => t #:: unfold(p, r.seed, attemptsPerItem)
case None => unfold(p, r.seed, attemptsLeft - 1)
}
}
}
gen { (p, seed0) =>
new R[Stream[T]] {
val result: Option[Stream[T]] = Some(unfold(p, seed0))
val seed: Seed = seed0.slide
}
val stream = unfold(p, seed0, attemptsPerItem)
r(Some(stream), seed0.slide)
}
}

Expand Down Expand Up @@ -765,7 +774,7 @@ object Gen extends GenArities with GenVersionSpecific {
buf += t
} else {
val (x, s) = seed.long
val i = (x & 0x7fffffff).toInt % count
val i = (x & Long.MaxValue % count).toInt
if (i < n) buf(i) = t
seed = s
}
Expand Down Expand Up @@ -848,19 +857,25 @@ object Gen extends GenArities with GenVersionSpecific {

//// String Generators ////

@tailrec private def mkString(n: Int, sb: StringBuilder, gc: Gen[Char], p: P, seed0: Seed): R[String] =
if (n <= 0) {
r(Some(sb.toString), seed0)
} else {
val res = gc.doApply(p, seed0)
private def mkString(n: Int, sb: StringBuilder, gc: Gen[Char], p: P, seed0: Seed): R[String] = {
var seed: Seed = seed0
val allowedFailures = Gen.collectionRetries(n)
var failures = 0
var count = 0
while (count < n) {
val res = gc.doApply(p, seed)
res.retrieve match {
case Some(c) =>
sb += c
count += 1
case None =>
()
failures += 1
if (failures >= allowedFailures) return r(None, res.seed)
}
mkString(n - 1, sb, gc, p, res.seed)
seed = res.seed
}
r(Some(sb.toString), seed)
}

def stringOfN(n: Int, gc: Gen[Char]): Gen[String] =
gen { (p, seed) =>
Expand Down Expand Up @@ -1058,9 +1073,14 @@ object Gen extends GenArities with GenVersionSpecific {
1 -> const(Duration.Zero),
6 -> finiteDuration)

// used to compute a uniformly-distributed size
private def mkSize(p: Gen.Parameters, seed0: Seed): (Int, Seed) = {
val maxSize = Integer.max(p.size + 1, 1)
val (x, seed1) = seed0.long
((x % maxSize).toInt, seed1)
(((x & Long.MaxValue) % maxSize).toInt, seed1)
}

// used to calculate how many per-item retries we should allow.
private def collectionRetries(n: Int): Int =
Integer.max(10, n / 10)
}

0 comments on commit 3cbcf07

Please sign in to comment.