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.

Sign up for GitHub

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

Jump to bottom

Don't recompute IR children #9915

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Don't recompute IR children #9915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/CallArgument.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ object CallArgument {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = name.toList :+ value
override lazy val children: List[IR] = name.toList :+ value
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to the change, but it should've been Vector, instead of a List

Copy link
Member

Choose a reason for hiding this comment

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

Once I heard @4e6 saying we shouldn't use List as it does an eager copy, but a lazy iterator. With List the preorder() implementation:

  • calls children on each one
  • the children implementation constructs a List
  • the preoder then concatenates the list to its own List representing the whole IR tree

Where can the slowdown be? Only in the construction of of children List, right? This PR tries to trade that CPU cost for increased memory consumption.

Wouldn't it be better to change the children to follow more efficient pattern and not allocate its own List?

Copy link
Contributor

Choose a reason for hiding this comment

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

List is a classic linked list https://github.com/scala/scala/blob/e858de50d51e13d225ac5b35015aac54cd71e23e/src/library/scala/collection/immutable/List.scala#L655-L660
It is suitable for implementing control flows (where you can efficiently pattern-match on head and tail), and maybe in prepending elements https://august.nagro.us/list-vs-vector.html. But it is not how we're using it.

Vector is a radix-balanced finger tree backed by arrays and should be more efficient in mixed use-cases (like ours append+traverse) both CPU and memory wise scala/scala#8534

Copy link
Member

Choose a reason for hiding this comment

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

elements https://august.nagro.us/list-vs-vector.html

Our usecase is "Construction with Append + Traversals" where Vector is faster. Possibly even better with children(addTo : Vector.Builder) : Unit.

Up to you guys, I don't want to be expert on Scala data structures.

Copy link
Member

Choose a reason for hiding this comment

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

I don't want to be expert on Scala data structures.

I continue to believe the best might be to have a lazy iterator like in this case of iterating all children of a directory - e.g. a queue where each nextElement() adds its children to the end of the queue.


/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/DefinitionArgument.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ object DefinitionArgument {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] =
override lazy val children: List[IR] =
name :: ascribedType.toList ++ defaultValue.toList

/** @inheritdoc */
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Empty.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ sealed case class Empty(
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def message(source: (IdentifiedLocation => String)): String =
Expand Down
4 changes: 2 additions & 2 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Expression.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ object Expression {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = expressions :+ returnValue
override lazy val children: List[IR] = expressions :+ returnValue

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down Expand Up @@ -268,7 +268,7 @@ object Expression {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(name, expression)
override lazy val children: List[IR] = List(name, expression)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down
4 changes: 2 additions & 2 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Function.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ object Function {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = arguments :+ body
override lazy val children: List[IR] = arguments :+ body

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down Expand Up @@ -308,7 +308,7 @@ object Function {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = (name :: arguments) :+ body
override lazy val children: List[IR] = (name :: arguments) :+ body

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down
4 changes: 2 additions & 2 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ object Literal {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String = if (this.base.isDefined) {
Expand Down Expand Up @@ -230,7 +230,7 @@ object Literal {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String = s""""$text""""
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ final case class Module(
}

/** @inheritdoc */
override def children: List[IR] = imports ++ exports ++ bindings
override lazy val children: List[IR] = imports ++ exports ++ bindings

/** @inheritdoc */
override def toString: String =
Expand Down
16 changes: 8 additions & 8 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Name.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ object Name {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] =
override lazy val children: List[IR] =
typePointer.map(_ :: methodName :: Nil).getOrElse(methodName :: Nil)

/** @inheritdoc */
Expand Down Expand Up @@ -353,7 +353,7 @@ object Name {
|""".stripMargin

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String = "_"
Expand Down Expand Up @@ -423,7 +423,7 @@ object Name {
|)
|""".stripMargin

override def children: List[IR] = List()
override def children: List[IR] = Nil

override def showCode(indent: Int): String = name
}
Expand Down Expand Up @@ -526,7 +526,7 @@ object Name {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String = name
Expand Down Expand Up @@ -630,7 +630,7 @@ object Name {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String = s"@$name"
Expand Down Expand Up @@ -719,7 +719,7 @@ object Name {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(expression)
override lazy val children: List[IR] = List(expression)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down Expand Up @@ -799,7 +799,7 @@ object Name {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved

/** @inheritdoc */
override def showCode(indent: Int): String = name
Expand Down Expand Up @@ -875,7 +875,7 @@ object Name {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String = name
Expand Down
8 changes: 4 additions & 4 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Pattern.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ object Pattern {
copy(location = location)

/** @inheritdoc */
override def children: List[IR] = List(name)
override lazy val children: List[IR] = List(name)

/** @inheritdoc */
override def showCode(indent: Int): String = name.showCode(indent)
Expand Down Expand Up @@ -264,7 +264,7 @@ object Pattern {
): Constructor = copy(location = location)

/** @inheritdoc */
override def children: List[IR] = constructor :: fields
override lazy val children: List[IR] = constructor :: fields

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down Expand Up @@ -359,7 +359,7 @@ object Pattern {
copy(location = location)

/** @inheritdoc */
override def children: List[IR] = List(literal)
override lazy val children: List[IR] = List(literal)

/** @inheritdoc */
override def showCode(indent: Int): String = literal.showCode(indent)
Expand Down Expand Up @@ -463,7 +463,7 @@ object Pattern {
copy(location = location)

/** @inheritdoc */
override def children: List[IR] = List(name, tpe)
override lazy val children: List[IR] = List(name, tpe)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down
8 changes: 4 additions & 4 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Type.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ object Type {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = args :+ result
override lazy val children: List[IR] = args :+ result

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down Expand Up @@ -218,7 +218,7 @@ object Type {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(typed, signature)
override lazy val children: List[IR] = List(typed, signature)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down Expand Up @@ -323,7 +323,7 @@ object Type {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(typed, context)
override lazy val children: List[IR] = List(typed, context)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down Expand Up @@ -426,7 +426,7 @@ object Type {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(typed, error)
override lazy val children: List[IR] = List(typed, error)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down
8 changes: 4 additions & 4 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/Application.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ object Application {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = function :: arguments
override lazy val children: List[IR] = function :: arguments

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down Expand Up @@ -216,7 +216,7 @@ object Application {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(target)
override lazy val children: List[IR] = List(target)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down Expand Up @@ -328,8 +328,8 @@ object Application {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] =
expression.map(List(_)).getOrElse(List())
override lazy val children: List[IR] =
expression.map(List(_)).getOrElse(Nil)

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down
4 changes: 2 additions & 2 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/Case.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ object Case {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = scrutinee :: branches.toList
override lazy val children: List[IR] = scrutinee :: branches.toList

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down Expand Up @@ -298,7 +298,7 @@ object Case {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(pattern, expression)
override lazy val children: List[IR] = List(pattern, expression)

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/Comment.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ object Comment {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/Error.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ object Error {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(ir)
override lazy val children: List[IR] = List(ir)

/** @inheritdoc */
override def message(source: (IdentifiedLocation => String)): String =
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/Foreign.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ object Foreign {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List()
override def children: List[IR] = Nil

/** @inheritdoc */
override def showCode(indent: Int): String = "FOREIGN DEF"
Expand Down
2 changes: 1 addition & 1 deletion engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/Operator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ object Operator {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(left, operator, right)
override lazy val children: List[IR] = List(left, operator, right)

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down
6 changes: 3 additions & 3 deletions engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/Section.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ object Section {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(arg, operator)
override lazy val children: List[IR] = List(arg, operator)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down Expand Up @@ -216,7 +216,7 @@ object Section {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(operator)
override lazy val children: List[IR] = List(operator)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down Expand Up @@ -322,7 +322,7 @@ object Section {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(operator, arg)
override lazy val children: List[IR] = List(operator, arg)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down
2 changes: 1 addition & 1 deletion ...untime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/Conversion.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ sealed case class Conversion(
}

/** @inheritdoc */
override def children: List[IR] = List(storedIr)
override lazy val children: List[IR] = List(storedIr)

/** @inheritdoc */
override def showCode(indent: Int): String =
Expand Down
2 changes: 1 addition & 1 deletion ...time-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/ImportExport.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ sealed case class ImportExport(
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = List(ir)
override lazy val children: List[IR] = List(ir)

/** @inheritdoc */
override def message(source: (IdentifiedLocation => String)): String =
Expand Down
2 changes: 1 addition & 1 deletion ...e/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/Pattern.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ sealed case class Pattern(
override val location: Option[IdentifiedLocation] =
originalPattern.location

override def children: List[IR] = List(originalPattern)
override lazy val children: List[IR] = List(originalPattern)

override def showCode(indent: Int): String =
originalPattern.showCode(indent)
Expand Down
Loading
Loading