From b00c4c911e4bdc7ae3739942e9158ef24ad0d00a Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Wed, 11 Mar 2020 19:27:03 +0100 Subject: [PATCH 01/10] Predicate: add benchmark --- .../src/main/scala/PredicateBenchmarks.scala | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 bench/src/main/scala/PredicateBenchmarks.scala diff --git a/bench/src/main/scala/PredicateBenchmarks.scala b/bench/src/main/scala/PredicateBenchmarks.scala new file mode 100644 index 00000000..6685e39f --- /dev/null +++ b/bench/src/main/scala/PredicateBenchmarks.scala @@ -0,0 +1,24 @@ +package cats.collections +package bench + +import org.openjdk.jmh.annotations.{Benchmark, Param, Scope, Setup, State} +import cats._ +import cats.implicits._ + +@State(Scope.Benchmark) +class ChainedPredicateBench { + @Param(Array("10", "100", "1000", "10000")) + var n: Int = _ + + var pred: Predicate[Int] = _ + + @Setup + def setup: Unit = { + pred = Iterator.iterate(Predicate.empty.negate)(_ - Predicate.empty).drop(n).next() + } + + @Benchmark + def catsCollectionsPredicateUnravel: Unit = { + pred.contains(0) + } +} From fc6bacc9497337e9b5c878bdd3602d0535a8a922 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Wed, 4 Mar 2020 23:05:20 +0100 Subject: [PATCH 02/10] Predicate: stack safety tests --- .../cats/collections/PredicateSpec.scala | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/src/test/scala/cats/collections/PredicateSpec.scala b/tests/src/test/scala/cats/collections/PredicateSpec.scala index 3235e4ea..e787709d 100644 --- a/tests/src/test/scala/cats/collections/PredicateSpec.scala +++ b/tests/src/test/scala/cats/collections/PredicateSpec.scala @@ -72,4 +72,31 @@ class PredicateSpec extends CatsSuite { bs.forall(b => (s1(b) != (as.contains(b) && (b % 2 != 0)))) should be(true) }) + + { + def testStackSafety(name: String, deepSet: => Predicate[Any]) = + test(name) { + noException should be thrownBy { + deepSet.contains(0) + } + } + val Depth = 200000 + testStackSafety("union is stack safe on the left hand side", + Iterator.fill(Depth)(Predicate.empty).reduceLeft(_ union _)) + testStackSafety("union is stack safe on the right hand side", + Iterator.fill(Depth)(Predicate.empty).reduceRight(_ union _)) + testStackSafety("intersection is stack safe on the left hand side", + Iterator.fill(Depth)(!Predicate.empty).reduceLeft(_ intersection _)) + testStackSafety("intersection is stack safe on the right hand side", + Iterator.fill(Depth)(!Predicate.empty).reduceRight(_ intersection _)) + testStackSafety("negation is stack safe", + Iterator.iterate(Predicate.empty)(_.negate).drop(Depth).next()) + testStackSafety("contramap() is stack safe", + Iterator.iterate(Predicate.empty)(_.contramap(identity _)).drop(Depth).next()) + val everything = Predicate.empty.negate + testStackSafety("diff is stack safe on the left hand side", + Iterator.fill(Depth)(everything).reduceLeft(_ diff _)) + testStackSafety("diff is stack safe on the right hand side", + Iterator.fill(Depth)(everything).reduceRight(_ diff _)) + } } From 76069db2899421e1425a189e4405a55bf9057c58 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Wed, 4 Mar 2020 18:35:48 +0100 Subject: [PATCH 03/10] Make Predicate stack-safe using Eval --- .../scala/cats/collections/Predicate.scala | 103 ++++++++++++++++-- 1 file changed, 93 insertions(+), 10 deletions(-) diff --git a/core/src/main/scala/cats/collections/Predicate.scala b/core/src/main/scala/cats/collections/Predicate.scala index b6933e71..12991c47 100644 --- a/core/src/main/scala/cats/collections/Predicate.scala +++ b/core/src/main/scala/cats/collections/Predicate.scala @@ -1,17 +1,39 @@ package cats.collections import cats._ +import cats.data.Kleisli /** * An intensional set, which is a set which instead of enumerating its * elements as a extensional set does, it is defined by a predicate * which is a test for membership. + * + * All combinators in this class are implemented in a stack-safe way. */ -abstract class Predicate[-A] extends scala.Function1[A, Boolean] { self => +sealed abstract class Predicate[-A] extends scala.Function1[A, Boolean] { self => + def run: Kleisli[Eval, A, Boolean] + + /** + * Returns true if the value satisfies the predicate. + */ + def apply(a: A): Boolean = run(a).value + /** * returns a predicate which is the union of this predicate and another */ - def union[B <: A](other: Predicate[B]): Predicate[B] = Predicate(a => apply(a) || other(a)) + def union[B <: A](other: Predicate[B]): Predicate[B] = + self match { + case Predicate.Empty => other + case Predicate.Everything => self + case _ => + other match { + case Predicate.Empty => self + case Predicate.Everything => other + case _ => Predicate.Lift { + self.run.flatMap(if (_) Predicate.True else other.run) + } + } + } /** * returns a predicate which is the union of this predicate and another @@ -21,7 +43,19 @@ abstract class Predicate[-A] extends scala.Function1[A, Boolean] { self => /** * returns a predicate which is the intersection of this predicate and another */ - def intersection[B <: A](other: Predicate[B]): Predicate[B] = Predicate(a => apply(a) && other(a)) + def intersection[B <: A](other: Predicate[B]): Predicate[B] = + self match { + case Predicate.Empty => self + case Predicate.Everything => other + case _ => + other match { + case Predicate.Empty => other + case Predicate.Everything => self + case _ => Predicate.Lift { + self.run.flatMap(if (_) other.run else Predicate.False) + } + } + } /** * returns a predicate which is the intersection of this predicate and another @@ -36,7 +70,7 @@ abstract class Predicate[-A] extends scala.Function1[A, Boolean] { self => /** * Returns the predicate which is the the difference of another predicate removed from this predicate */ - def diff[B <: A](remove: Predicate[B]): Predicate[B] = Predicate(a => apply(a) && !remove(a)) + def diff[B <: A](remove: Predicate[B]): Predicate[B] = self intersection remove.negate /** * Returns the predicate which is the the difference of another predicate removed from this predicate @@ -46,28 +80,77 @@ abstract class Predicate[-A] extends scala.Function1[A, Boolean] { self => /** * Return the opposite predicate */ - def negate: Predicate[A] = Predicate(a => !apply(a)) + def negate: Predicate[A] /** * Return the opposite predicate */ def unary_!(): Predicate[A] = negate + + /** + * Compose the predicate with a function. + * + * A value is a member of the resulting predicate iff its image through f is a + * member of this predicate. + */ + def contramap[B](f: B => A): Predicate[B] + + /** + * Alias for contramap. + */ + final override def compose[B](f: B => A): Predicate[B] = contramap(f) } object Predicate extends PredicateInstances { - def apply[A](f: A => Boolean): Predicate[A] = new Predicate[A] { - def apply(a: A) = f(a) + private val True = Kleisli.liftF(Eval.True) + private val False = Kleisli.liftF(Eval.False) + + private[collections] case object Empty extends Predicate[Any] { + override def run: Kleisli[Eval, Any, Boolean] = Predicate.False + override def negate: Predicate[Any] = Everything + override def contramap[B](f: B => Any): Predicate[B] = this + } + + private[collections] case object Everything extends Predicate[Any] { + override def run: Kleisli[Eval, Any, Boolean] = Predicate.True + override def negate: Predicate[Any] = Empty + override def contramap[B](f: B => Any): Predicate[B] = this + } + + private[collections] final case class Lift[A](run: Kleisli[Eval, A, Boolean]) extends Predicate[A] { + override def negate: Predicate[A] = Negate(this) + override def contramap[B](f: B => A): Predicate[B] = Lift(run.compose(b => Eval.now(f(b)))) } - def empty: Predicate[Any] = apply(_ => false) + private[collections] final case class Negate[A](negate: Predicate[A]) extends Predicate[A] { + override def run: Kleisli[Eval, A, Boolean] = negate.run.map(!_) + override def contramap[B](f: B => A): Predicate[B] = Negate(negate contramap f) + } + + /** + * build a set from a membership function. + */ + def apply[A](p: A => Boolean): Predicate[A] = Lift { + Kleisli(a => Eval.now(p(a))) + } + + /** + * The empty set: a predicate that rejects all values. + */ + def empty[A]: Predicate[A] = Empty + + /** + * The set of everything: a predicate that accepts all values. + */ + def everything[A]: Predicate[A] = Everything } trait PredicateInstances { implicit def predicateContravariantMonoidal: ContravariantMonoidal[Predicate] = new ContravariantMonoidal[Predicate] { override def contramap[A, B](fb: Predicate[A])(f: B => A): Predicate[B] = - Predicate(f andThen fb.apply) + fb.contramap(f) override def product[A, B](fa: Predicate[A], fb: Predicate[B]): Predicate[(A, B)] = - Predicate(v => fa(v._1) || fb(v._2)) + fa.contramap[(A,B)](_._1) union fb.contramap(_._2) override def unit: Predicate[Unit] = Predicate.empty } From cadc03c874176f6323dcbde4384601ffe1139b26 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Thu, 30 Jul 2020 23:35:47 +0200 Subject: [PATCH 04/10] Avoid trivial benchmarks and safety tests If the set is simplified it defeats the point --- .../src/main/scala/PredicateBenchmarks.scala | 3 ++- .../cats/collections/PredicateSpec.scala | 20 +++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/bench/src/main/scala/PredicateBenchmarks.scala b/bench/src/main/scala/PredicateBenchmarks.scala index 6685e39f..e7820631 100644 --- a/bench/src/main/scala/PredicateBenchmarks.scala +++ b/bench/src/main/scala/PredicateBenchmarks.scala @@ -14,7 +14,8 @@ class ChainedPredicateBench { @Setup def setup: Unit = { - pred = Iterator.iterate(Predicate.empty.negate)(_ - Predicate.empty).drop(n).next() + pred = Predicate(_ == 0) + pred = Iterator.iterate(pred.negate)(_ - pred).drop(n).next() } @Benchmark diff --git a/tests/src/test/scala/cats/collections/PredicateSpec.scala b/tests/src/test/scala/cats/collections/PredicateSpec.scala index e787709d..bee355b8 100644 --- a/tests/src/test/scala/cats/collections/PredicateSpec.scala +++ b/tests/src/test/scala/cats/collections/PredicateSpec.scala @@ -74,29 +74,29 @@ class PredicateSpec extends CatsSuite { }) { - def testStackSafety(name: String, deepSet: => Predicate[Any]) = + def testStackSafety(name: String, deepSet: => Predicate[Int]) = test(name) { noException should be thrownBy { deepSet.contains(0) } } val Depth = 200000 + val NonZero = Predicate[Int](_ != 0) testStackSafety("union is stack safe on the left hand side", - Iterator.fill(Depth)(Predicate.empty).reduceLeft(_ union _)) + Iterator.fill(Depth)(NonZero).reduceLeft(_ union _)) testStackSafety("union is stack safe on the right hand side", - Iterator.fill(Depth)(Predicate.empty).reduceRight(_ union _)) + Iterator.fill(Depth)(NonZero).reduceRight(_ union _)) testStackSafety("intersection is stack safe on the left hand side", - Iterator.fill(Depth)(!Predicate.empty).reduceLeft(_ intersection _)) + Iterator.fill(Depth)(!NonZero).reduceLeft(_ intersection _)) testStackSafety("intersection is stack safe on the right hand side", - Iterator.fill(Depth)(!Predicate.empty).reduceRight(_ intersection _)) + Iterator.fill(Depth)(!NonZero).reduceRight(_ intersection _)) testStackSafety("negation is stack safe", - Iterator.iterate(Predicate.empty)(_.negate).drop(Depth).next()) + Iterator.iterate(NonZero)(_.negate).drop(Depth).next()) testStackSafety("contramap() is stack safe", - Iterator.iterate(Predicate.empty)(_.contramap(identity _)).drop(Depth).next()) - val everything = Predicate.empty.negate + Iterator.iterate(NonZero)(_.contramap(identity _)).drop(Depth).next()) testStackSafety("diff is stack safe on the left hand side", - Iterator.fill(Depth)(everything).reduceLeft(_ diff _)) + Iterator.fill(Depth)(!NonZero).reduceLeft(_ diff _)) testStackSafety("diff is stack safe on the right hand side", - Iterator.fill(Depth)(everything).reduceRight(_ diff _)) + Iterator.fill(Depth)(!NonZero).reduceRight(_ diff _)) } } From f71a3bd5f0971c18e74b6359b89c755434a7ed35 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Thu, 30 Jul 2020 23:36:22 +0200 Subject: [PATCH 05/10] Use Predicate.empty where applicable --- core/src/main/scala/cats/collections/Set.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/scala/cats/collections/Set.scala b/core/src/main/scala/cats/collections/Set.scala index 52e84c78..35e4bd2d 100644 --- a/core/src/main/scala/cats/collections/Set.scala +++ b/core/src/main/scala/cats/collections/Set.scala @@ -409,6 +409,8 @@ object AvlSet extends AvlSetInstances { private[collections] case object BTNil extends AvlSet[Nothing] { override def isEmpty: Boolean = true + override def predicate(implicit order: cats.Order[Nothing]): Predicate[Nothing] = Predicate.empty + def apply[A](): AvlSet[A] = this.asInstanceOf[AvlSet[A]] def unapply[A](a: AvlSet[A]): Boolean = a.isEmpty From 47d57863568566dc7b0d4e44b73406d39a4ee915 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Thu, 30 Jul 2020 23:37:03 +0200 Subject: [PATCH 06/10] Improve ArbitraryPredicate --- .../collections/arbitrary/ArbitraryPredicate.scala | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/scalacheck/src/main/scala/cats/collections/arbitrary/ArbitraryPredicate.scala b/scalacheck/src/main/scala/cats/collections/arbitrary/ArbitraryPredicate.scala index 3fa756c0..673ae086 100644 --- a/scalacheck/src/main/scala/cats/collections/arbitrary/ArbitraryPredicate.scala +++ b/scalacheck/src/main/scala/cats/collections/arbitrary/ArbitraryPredicate.scala @@ -1,15 +1,13 @@ package cats.collections package arbitrary -import org.scalacheck.{Gen, Arbitrary} -import cats.Order +import org.scalacheck.{Gen, Cogen, Arbitrary} trait ArbitraryPredicate { - import set._ - def predicateGen[A: Arbitrary: Order]: Gen[Predicate[A]] = - setGen.map(_.predicate) + import Gen._ + def predicateGen[A: Arbitrary: Cogen]: Gen[Predicate[A]] = + oneOf(const(Predicate.empty), const(Predicate.everything), resultOf(Predicate(_: A => Boolean))) - implicit def predicateArbitrary[A: Arbitrary: Order]: Arbitrary[Predicate[A]] = + implicit def predicateArbitrary[A: Arbitrary: Cogen]: Arbitrary[Predicate[A]] = Arbitrary(predicateGen[A]) } - From d0df5ec901e3f8bf71707fbc9fe2d9b011a249b3 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Thu, 30 Jul 2020 23:37:25 +0200 Subject: [PATCH 07/10] Add Bool[Predicate[A]] --- core/src/main/scala/cats/collections/Predicate.scala | 10 ++++++++++ .../test/scala/cats/collections/PredicateSpec.scala | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/core/src/main/scala/cats/collections/Predicate.scala b/core/src/main/scala/cats/collections/Predicate.scala index 12991c47..77ad5a7a 100644 --- a/core/src/main/scala/cats/collections/Predicate.scala +++ b/core/src/main/scala/cats/collections/Predicate.scala @@ -1,5 +1,6 @@ package cats.collections +import algebra.lattice.Bool import cats._ import cats.data.Kleisli @@ -159,6 +160,15 @@ trait PredicateInstances { override def combine(l: Predicate[A], r: Predicate[A]): Predicate[A] = l union r } + implicit def predicateBool[A]: Bool[Predicate[A]] = new Bool[Predicate[A]] { + override def one: Predicate[A] = Predicate.everything + override def zero: Predicate[A] = Predicate.empty + override def complement(x: Predicate[A]): Predicate[A] = x.negate + override def and(l: Predicate[A], r: Predicate[A]): Predicate[A] = l intersection r + override def or(l: Predicate[A], r: Predicate[A]): Predicate[A] = l union r + + } + implicit val predicateMonoidK: MonoidK[Predicate] = new MonoidK[Predicate] { override def empty[A]: Predicate[A] = Predicate.empty override def combineK[A](l: Predicate[A], r: Predicate[A]): Predicate[A] = l union r diff --git a/tests/src/test/scala/cats/collections/PredicateSpec.scala b/tests/src/test/scala/cats/collections/PredicateSpec.scala index bee355b8..02956dc8 100644 --- a/tests/src/test/scala/cats/collections/PredicateSpec.scala +++ b/tests/src/test/scala/cats/collections/PredicateSpec.scala @@ -1,6 +1,8 @@ package cats.collections + package tests +import algebra.laws.LogicLaws import cats.collections.arbitrary.predicate._ import cats.laws.discipline.{ContravariantMonoidalTests, SerializableTests} import cats._ @@ -22,6 +24,16 @@ class PredicateSpec extends CatsSuite { checkAll("ContravariantMonoidal[Predicate]", ContravariantMonoidalTests[Predicate].contravariantMonoidal[Int, Int, Int]) } + { + implicit val eqForPredicateInt: Eq[Predicate[Int]] = new Eq[Predicate[Int]] { + val sample = -1 to 1 // need at least 2 elements to distinguish in-between values + override def eqv(x: Predicate[Int], y: Predicate[Int]): Boolean = + sample.forall(a => x(a) == y(a)) + } + + checkAll("Bool[Predicate[Int]]", LogicLaws[Predicate[Int]].bool) + } + test("intersection works")( forAll { (as: List[Int], bs: List[Int]) => From 2e93fcebf852593482b589e9b7885f342015a49c Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Fri, 31 Jul 2020 10:23:25 +0200 Subject: [PATCH 08/10] Predicate: use Eval static instances --- core/src/main/scala/cats/collections/Predicate.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/collections/Predicate.scala b/core/src/main/scala/cats/collections/Predicate.scala index 77ad5a7a..02e9e8a7 100644 --- a/core/src/main/scala/cats/collections/Predicate.scala +++ b/core/src/main/scala/cats/collections/Predicate.scala @@ -132,7 +132,7 @@ object Predicate extends PredicateInstances { * build a set from a membership function. */ def apply[A](p: A => Boolean): Predicate[A] = Lift { - Kleisli(a => Eval.now(p(a))) + Kleisli(a => if (p(a)) Eval.True else Eval.False) } /** From 77e0628f904098f5b12d339af62eabbd5cfdcf72 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Fri, 31 Jul 2020 10:24:16 +0200 Subject: [PATCH 09/10] PredicateInstances: fix naming of implicits --- core/src/main/scala/cats/collections/Predicate.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/cats/collections/Predicate.scala b/core/src/main/scala/cats/collections/Predicate.scala index 02e9e8a7..0762b7cb 100644 --- a/core/src/main/scala/cats/collections/Predicate.scala +++ b/core/src/main/scala/cats/collections/Predicate.scala @@ -147,7 +147,7 @@ object Predicate extends PredicateInstances { } trait PredicateInstances { - implicit def predicateContravariantMonoidal: ContravariantMonoidal[Predicate] = new ContravariantMonoidal[Predicate] { + implicit def catsCollectionsPredicateContravariantMonoidal: ContravariantMonoidal[Predicate] = new ContravariantMonoidal[Predicate] { override def contramap[A, B](fb: Predicate[A])(f: B => A): Predicate[B] = fb.contramap(f) override def product[A, B](fa: Predicate[A], fb: Predicate[B]): Predicate[(A, B)] = @@ -155,12 +155,12 @@ trait PredicateInstances { override def unit: Predicate[Unit] = Predicate.empty } - implicit def predicateMonoid[A]: Monoid[Predicate[A]] = new Monoid[Predicate[A]] { + implicit def catsCollectionsPredicateMonoid[A]: Monoid[Predicate[A]] = new Monoid[Predicate[A]] { override def empty: Predicate[A] = Predicate.empty override def combine(l: Predicate[A], r: Predicate[A]): Predicate[A] = l union r } - implicit def predicateBool[A]: Bool[Predicate[A]] = new Bool[Predicate[A]] { + implicit def catsCollectionsPredicateBool[A]: Bool[Predicate[A]] = new Bool[Predicate[A]] { override def one: Predicate[A] = Predicate.everything override def zero: Predicate[A] = Predicate.empty override def complement(x: Predicate[A]): Predicate[A] = x.negate @@ -169,7 +169,7 @@ trait PredicateInstances { } - implicit val predicateMonoidK: MonoidK[Predicate] = new MonoidK[Predicate] { + implicit val catsCollectionsPredicateMonoidK: MonoidK[Predicate] = new MonoidK[Predicate] { override def empty[A]: Predicate[A] = Predicate.empty override def combineK[A](l: Predicate[A], r: Predicate[A]): Predicate[A] = l union r } From 94604fb3f2d97c90041023f52b50f4355ed24b36 Mon Sep 17 00:00:00 2001 From: Louis Bettens Date: Fri, 31 Jul 2020 11:27:34 +0200 Subject: [PATCH 10/10] PredicateSpec: increase comparion sample size --- tests/src/test/scala/cats/collections/PredicateSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/test/scala/cats/collections/PredicateSpec.scala b/tests/src/test/scala/cats/collections/PredicateSpec.scala index 02956dc8..c3a9ac6a 100644 --- a/tests/src/test/scala/cats/collections/PredicateSpec.scala +++ b/tests/src/test/scala/cats/collections/PredicateSpec.scala @@ -26,7 +26,7 @@ class PredicateSpec extends CatsSuite { { implicit val eqForPredicateInt: Eq[Predicate[Int]] = new Eq[Predicate[Int]] { - val sample = -1 to 1 // need at least 2 elements to distinguish in-between values + val sample = Byte.MinValue to Byte.MaxValue override def eqv(x: Predicate[Int], y: Predicate[Int]): Boolean = sample.forall(a => x(a) == y(a)) }