Skip to content

Commit

Permalink
Patmat: Switch from ListSet to mutable.LinkedHashSet
Browse files Browse the repository at this point in the history
* experiment fix for SI-12499 (patmat perf)
* use java.util.LinkedHashSet
* switch j.u to s.c.m LinkedHashSet
* avoid convert to Set to keep deterministic iteration order
* fix compile error

Co-authored-by: Dale Wijnand <dale.wijnand@gmail.com>
  • Loading branch information
KisaragiEffective and dwijnand committed Mar 2, 2023
1 parent 91f0f66 commit 5d5d00f
Show file tree
Hide file tree
Showing 6 changed files with 619 additions and 23 deletions.
27 changes: 13 additions & 14 deletions src/compiler/scala/tools/nsc/transform/patmat/Logic.scala
Expand Up @@ -15,6 +15,7 @@ package tools.nsc.transform.patmat

import scala.collection.mutable
import scala.collection.immutable.ArraySeq
import scala.collection.IterableOps
import scala.reflect.internal.util.Collections._
import scala.reflect.internal.util.HashSet

Expand Down Expand Up @@ -113,24 +114,22 @@ trait Logic extends Debugging {
def implications: List[(Sym, List[Sym], List[Sym])]
}

// The error message of t7020 assumes the ops are ordered implicitly
// However, ListSet is slow (concatenate cost?), so use
// scala.collection.mutable.LinkedHashSet (which grantees "the order in which elements were inserted into the set")

// would be nice to statically check whether a prop is equational or pure,
// but that requires typing relations like And(x: Tx, y: Ty) : (if(Tx == PureProp && Ty == PureProp) PureProp else Prop)
final case class And(ops: Set[Prop]) extends Prop
final case class And(ops: mutable.LinkedHashSet[Prop]) extends Prop
object And {
def apply(ps: Prop*) = create(ps)
def create(ps: Iterable[Prop]) = ps match {
case ps: Set[Prop] => new And(ps)
case _ => new And(ps.to(scala.collection.immutable.ListSet))
}
def create(ps: Iterable[Prop]) = new And(ps.to(mutable.LinkedHashSet))
}

final case class Or(ops: Set[Prop]) extends Prop
final case class Or(ops: mutable.LinkedHashSet[Prop]) extends Prop
object Or {
def apply(ps: Prop*) = create(ps)
def create(ps: Iterable[Prop]) = ps match {
case ps: Set[Prop] => new Or(ps)
case _ => new Or(ps.to(scala.collection.immutable.ListSet))
}
def create(ps: Iterable[Prop]) = new Or(ps.to(mutable.LinkedHashSet))
}

final case class Not(a: Prop) extends Prop
Expand Down Expand Up @@ -197,7 +196,7 @@ trait Logic extends Debugging {
*/
def simplify(f: Prop): Prop = {

def hasImpureAtom(ops0: collection.Iterable[Prop]): Boolean = {
def hasImpureAtom(ops0: Iterable[Prop]): Boolean = {
// HOT method, imperative rewrite of:
// ops.combinations(2).exists {
// case Seq(a, Not(b)) if a == b => true
Expand Down Expand Up @@ -247,7 +246,7 @@ trait Logic extends Debugging {
}
}

def mapConserve[A <: AnyRef](s: Set[A])(f: A => A): Set[A] = {
def mapConserve[CC[X] <: IterableOps[X, CC, CC[X]], A <: AnyRef](s: CC[A])(f: A => A): CC[A] = {
var changed = false
val s1 = s.map {a =>
val a1 = f(a)
Expand Down Expand Up @@ -284,7 +283,7 @@ trait Logic extends Debugging {
| (_: AtMostOne) => p
}

def simplifyAnd(ps: Set[Prop]): Prop = {
def simplifyAnd(ps: Iterable[Prop]): Prop = {
// recurse for nested And (pulls all Ands up)
// build up Set in order to remove duplicates
val props = mutable.LinkedHashSet.empty[Prop]
Expand All @@ -300,7 +299,7 @@ trait Logic extends Debugging {
else /\(props)
}

def simplifyOr(ps: Set[Prop]): Prop = {
def simplifyOr(ps: Iterable[Prop]): Prop = {
// recurse for nested Or (pulls all Ors up)
// build up Set in order to remove duplicates
val props = mutable.LinkedHashSet.empty[Prop]
Expand Down
Expand Up @@ -56,7 +56,7 @@ trait MatchOptimization extends MatchTreeMaking with MatchApproximation {
val cond = test.prop

def simplify(c: Prop): Set[Prop] = c match {
case And(ops) => ops flatMap simplify
case And(ops) => ops.flatMap(simplify).toSet
case Or(ops) => Set(False) // TODO: make more precise
case Not(Eq(Var(_), NullConst)) => Set.empty // not worth remembering
case True => Set.empty // same
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/scala/tools/nsc/transform/patmat/Solving.scala
Expand Up @@ -166,7 +166,7 @@ trait Solving extends Logic {
}
}

def and(bv: Set[Lit]): Lit = {
def and(bv: collection.Set[Lit]): Lit = {
if (bv.isEmpty) {
// this case can actually happen because `removeVarEq` could add no constraints
constTrue
Expand All @@ -178,14 +178,14 @@ trait Solving extends Logic {
// op1 /\ op2 /\ ... /\ opx <==>
// (o -> op1) /\ (o -> op2) ... (o -> opx) /\ (!op1 \/ !op2 \/... \/ !opx \/ o)
// (!o \/ op1) /\ (!o \/ op2) ... (!o \/ opx) /\ (!op1 \/ !op2 \/... \/ !opx \/ o)
val new_bv = bv - constTrue // ignore `True`
val new_bv = bv.toSet - constTrue // ignore `True`
val o = newLiteral() // auxiliary Tseitin variable
new_bv.foreach(op => addClauseProcessed(clause(op, -o)))
o
}
}

def or(bv: Set[Lit]): Lit = {
def or(bv: collection.Set[Lit]): Lit = {
if (bv.isEmpty) {
constFalse
} else if (bv.size == 1) {
Expand All @@ -196,7 +196,7 @@ trait Solving extends Logic {
// op1 \/ op2 \/ ... \/ opx <==>
// (op1 -> o) /\ (op2 -> o) ... (opx -> o) /\ (op1 \/ op2 \/... \/ opx \/ !o)
// (!op1 \/ o) /\ (!op2 \/ o) ... (!opx \/ o) /\ (op1 \/ op2 \/... \/ opx \/ !o)
val new_bv = bv - constFalse // ignore `False`
val new_bv = bv.toSet - constFalse // ignore `False`
val o = newLiteral() // auxiliary Tseitin variable
addClauseProcessed(new_bv + (-o))
o
Expand Down
8 changes: 8 additions & 0 deletions test/files/neg/t12499.check
@@ -0,0 +1,8 @@
t12499.scala:455: warning: Cannot check match for unreachability.
The analysis required more space than allowed.
Please try with scalac -Ypatmat-exhaust-depth 60 or -Ypatmat-exhaust-depth off.
implicit val converter: Thing[Phantom.TypeA] => String = {
^
error: No warnings can be incurred under -Werror.
1 warning
1 error

0 comments on commit 5d5d00f

Please sign in to comment.