Skip to content

Commit 3676690

Browse files
reid-spencerclaude
andcommitted
Optimize TreePass and Definition hashing for Scala.js performance
Replace HashMap with Stack in TreePass for O(n) tree building. Add cheap hashCode/equals to Definition trait to avoid O(subtree) traversal of Contents fields in all HashMap operations. Change UsageResolution from Seq to mutable.Set for O(1) association lookups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3dc6d1e commit 3676690

4 files changed

Lines changed: 49 additions & 27 deletions

File tree

language/shared/src/main/scala/com/ossuminc/riddl/language/AST.scala

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,40 @@ object AST:
812812
override def isDefinition: Boolean = true
813813
override def isParent: Boolean = false
814814
override def hasDefinitions: Boolean = false
815+
816+
/** Cheap hash based on id + loc to avoid O(subtree) traversal of contents fields.
817+
* Case class auto-generated hashCode traverses ALL constructor fields including
818+
* Contents (mutable.ArrayBuffer), making every HashMap operation O(subtree).
819+
* Note: overriding hashCode here suppresses case class auto-generated equals too
820+
* (Scala 3 spec), so we must also override equals to preserve structural comparison.
821+
*/
822+
override def hashCode: Int =
823+
val h = id.hashCode * 31 + loc.hashCode
824+
h * 31 + getClass.hashCode
825+
override def equals(that: Any): Boolean = that match
826+
case other: Definition =>
827+
(this eq other) || (
828+
getClass == other.getClass &&
829+
id == other.id &&
830+
loc == other.loc &&
831+
metadata == other.metadata &&
832+
productEquals(other)
833+
)
834+
case _ => false
835+
836+
/** Structural comparison of product elements, skipping Contents fields
837+
* to avoid O(subtree) traversal. For two Definitions at the same loc
838+
* with the same id, the non-contents fields determine equality.
839+
*/
840+
private def productEquals(other: Definition): Boolean =
841+
(this, other) match
842+
case (a: Product, b: Product) =>
843+
a.productArity == b.productArity &&
844+
(0 until a.productArity).forall: i =>
845+
(a.productElement(i), b.productElement(i)) match
846+
case (_: Contents[?], _: Contents[?]) => true // skip contents
847+
case (x, y) => x == y
848+
case _ => false
815849
end Definition
816850

817851
object Definition:

passes/shared/src/main/scala/com/ossuminc/riddl/passes/TreePass.scala

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ case class TreePass(
4444

4545
def name: String = TreePass.name
4646

47-
private val childrenMap: mutable.Map[Definition, mutable.ListBuffer[TreeNode]] =
48-
mutable.Map.empty
47+
private val stack: mutable.Stack[mutable.ListBuffer[TreeNode]] =
48+
mutable.Stack.empty
4949

5050
private val topLevel: mutable.ListBuffer[TreeNode] =
5151
mutable.ListBuffer.empty
@@ -54,7 +54,7 @@ case class TreePass(
5454
definition: Definition,
5555
parents: Parents
5656
): Unit = {
57-
childrenMap(definition) = mutable.ListBuffer.empty
57+
stack.push(mutable.ListBuffer.empty)
5858
}
5959

6060
protected def processLeaf(
@@ -70,11 +70,7 @@ case class TreePass(
7070
offset = definition.loc.offset,
7171
children = Seq.empty
7272
)
73-
if parents.nonEmpty then
74-
childrenMap.get(parents.head.asInstanceOf[Definition]) match {
75-
case Some(buf) => buf.append(leaf)
76-
case None => topLevel.append(leaf)
77-
}
73+
if stack.nonEmpty then stack.head.append(leaf)
7874
else topLevel.append(leaf)
7975
end if
8076
end if
@@ -89,8 +85,7 @@ case class TreePass(
8985
definition: Definition,
9086
parents: Parents
9187
): Unit = {
92-
val children = childrenMap.remove(definition)
93-
.map(_.toSeq).getOrElse(Seq.empty)
88+
val children = if stack.nonEmpty then stack.pop().toSeq else Seq.empty
9489
if definition.id.nonEmpty then
9590
val node = TreeNode(
9691
kind = definition.kind,
@@ -100,11 +95,7 @@ case class TreePass(
10095
offset = definition.loc.offset,
10196
children = children
10297
)
103-
if parents.nonEmpty then
104-
childrenMap.get(parents.head.asInstanceOf[Definition]) match {
105-
case Some(buf) => buf.append(node)
106-
case None => topLevel.append(node)
107-
}
98+
if stack.nonEmpty then stack.head.append(node)
10899
else topLevel.append(node)
109100
end if
110101
end if

passes/shared/src/main/scala/com/ossuminc/riddl/passes/resolve/UsageResolution.scala

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ import scala.reflect.ClassTag
1515

1616
trait UsageBase {
1717

18-
type UseMap = mutable.HashMap[Definition, Seq[Definition]]
19-
type UsedByMap = mutable.HashMap[Definition, Seq[Definition]]
18+
type UseMap = mutable.HashMap[Definition, mutable.Set[Definition]]
19+
type UsedByMap = mutable.HashMap[Definition, mutable.Set[Definition]]
2020

21-
protected val uses: UseMap = mutable.HashMap.empty[Definition, Seq[Definition]]
22-
protected val usedBy: UsedByMap = mutable.HashMap.empty[Definition, Seq[Definition]]
21+
protected val uses: UseMap = mutable.HashMap.empty[Definition, mutable.Set[Definition]]
22+
protected val usedBy: UsedByMap = mutable.HashMap.empty[Definition, mutable.Set[Definition]]
2323
}
2424

2525
/** Validation State for Uses/UsedBy Tracking. During parsing, when usage is detected, call associateUsage. After
@@ -60,11 +60,8 @@ trait UsageResolution(using io: PlatformContext) extends UsageBase {
6060
end associateUsage
6161

6262
def associateUsage(user: Definition, use: Definition): this.type =
63-
val used = uses.getOrElse(user, Seq.empty[Definition])
64-
if !used.contains(use) then uses.update(user, used :+ use)
65-
66-
val usages = usedBy.getOrElse(use, Seq.empty[Definition])
67-
if !usages.contains(user) then usedBy.update(use, usages :+ user)
63+
uses.getOrElseUpdate(user, mutable.Set.empty[Definition]).add(use)
64+
usedBy.getOrElseUpdate(use, mutable.Set.empty[Definition]).add(user)
6865
this
6966
end associateUsage
7067

passes/shared/src/main/scala/com/ossuminc/riddl/passes/resolve/Usages.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ case class Usages(
3131

3232
/** Determine if a definition is used or not */
3333
def isUsed(definition: Definition): Boolean = {
34-
uses.keys.exists(_ == definition)
34+
uses.contains(definition)
3535
}
3636

3737
/** Determine if one definition is used by another
@@ -72,7 +72,7 @@ case class Usages(
7272
* The [[scala.Seq]] of [[com.ossuminc.riddl.language.AST.Definition]] that are using `used`
7373
*/
7474
def getUsers(used: Definition): Seq[Definition] = {
75-
usedBy.getOrElse(used, Seq.empty)
75+
usedBy.getOrElse(used, mutable.Set.empty).toSeq
7676
}
7777

7878
/** Retrieve the uses of a given user
@@ -83,7 +83,7 @@ case class Usages(
8383
* The [[scala.Seq]] of [[com.ossuminc.riddl.language.AST.Definition]] that are used by `user`
8484
*/
8585
def getUses(user: Definition): Seq[Definition] = {
86-
uses.getOrElse(user, Seq.empty)
86+
uses.getOrElse(user, mutable.Set.empty).toSeq
8787
}
8888

8989
def usesAsString: String = {

0 commit comments

Comments
 (0)