perf(compiler): Inliner optimization [LNG-322] #1047
Conversation
# Conflicts: # aqua-src/antithesis.aqua
# Conflicts: # aqua-src/antithesis.aqua
DieMyst
changed the title
# Conflicts: # aqua-src/antithesis.aqua
There was a problem hiding this comment.
Could you restore this file please?
| @@ -11,7 +11,7 @@ class ImportsSpec extends AnyFlatSpec with ScalaCheckPropertyChecks with Matcher | |||
|
|
|||
| implicit override val generatorDrivenConfig = | |||
| // Tests here are lightweight, so we can afford to run more of them | |||
| PropertyCheckConfiguration(minSuccessful = 500, sizeRange = 64) | |||
| PropertyCheckConfiguration(minSuccessful = 50, sizeRange = 32) | |||
There was a problem hiding this comment.
Why lower those values?
|
|
||
| def findNewName(introduce: String): State[S, String] = | ||
| findNewNames(Set(introduce)).map(_.getOrElse(introduce, introduce)) | ||
| def getForbiddenNames: State[S, ManglerState] |
There was a problem hiding this comment.
Is it used somewhere outside implementation and tests?
Maybe return actually set of forbidden names, rename the method to smth like getState or totally remove this method from the interface?
| getForbiddenNames.flatMap(forbidden => | ||
| val (newState, newNames) = forbidden.findNewNames(introduce) | ||
| State.modify[ManglerState](_ => newState).map(_ => newNames) | ||
| ) |
There was a problem hiding this comment.
| getForbiddenNames.flatMap(forbidden => | |
| val (newState, newNames) = forbidden.findNewNames(introduce) | |
| State.modify[ManglerState](_ => newState).map(_ => newNames) | |
| ) | |
| State.apply(_.findNewNames(introduce)) |
| val mangler = Mangler.Simple | ||
|
|
||
| val a = for { | ||
| _ <- mangler.forbid(Set("first", "second")) | ||
| fn <- mangler.getForbiddenNames | ||
| } yield fn |
| } | ||
| } | ||
|
|
||
| object Mangler { | ||
| def apply[S](implicit mangler: Mangler[S]): Mangler[S] = mangler | ||
|
|
||
| implicit object Simple extends Mangler[Set[String]] { | ||
| val getForbiddenNames: State[Set[String], Set[String]] = State.get | ||
| implicit object Simple extends Mangler[ManglerState] { |
There was a problem hiding this comment.
| implicit object Simple extends Mangler[ManglerState] { | |
| given Mangler[ManglerState] with { |
and then Mangler.Simple -> Mangler[ManglerState]
| def forbid(names: Set[String]): State[R, Unit] = | ||
| self.forbid(names).transformS(f, g) | ||
|
|
||
| def findAndForbidNames(introduce: Set[String]): State[R, Map[String, String]] = | ||
| self.findAndForbidNames(introduce).transformS(f, g) | ||
| } | ||
| } | ||
|
|
||
| object Mangler { | ||
| def apply[S](implicit mangler: Mangler[S]): Mangler[S] = mangler |
There was a problem hiding this comment.
| def apply[S](implicit mangler: Mangler[S]): Mangler[S] = mangler | |
| def apply[S](using mangler: Mangler[S]): Mangler[S] = mangler |
|
|
||
| val results = for { | ||
| res <- mangler.findAndForbidNames(Set("first", "second")) | ||
| fn <- mangler.getForbiddenNames |
| val mangler = Mangler.Simple | ||
|
|
||
| val results = for { | ||
| res1 <- mangler.findAndForbidNames(Set("first", "first")) |
There was a problem hiding this comment.
What is the meaning of Set("first", "first")?
| span. | ||
| focus(locationMap.value, ctx).map(FileSpan.Focus(name, locationMap, ctx, _)) |
There was a problem hiding this comment.
nit: I think this syntax change is uglier
| state <- get | ||
| result = state.forbidAndRename(name) | ||
| (newState, newName) = result | ||
| _ <- modify(_ => newState) |
There was a problem hiding this comment.
Maybe rewrite modify according to use case?
Smth like
private def apply[A](f: ManglerState => (ManglerState, A)): State[X, A] =
State.apply(lens.modifyF(f andThen (_.swap)) andThen (_.swap))
(have to swap because of functor instances)
and then
newName <- apply(_.forbidAndRename)
|
|
||
| // find unique names that have not yet been used | ||
| def findNewNames(introduce: Set[String]): (ManglerState, Map[String, String]) = { | ||
| introduce.foldLeft(this, Map.empty[String, String]) { |
There was a problem hiding this comment.
nit: I think this class may implement just findNewName(name: String) and renaming for a set could be done in Mangler trait just by traverse. I see no gain in optimization or code clarity from implementing this method for set directly.
Description
Inliner works slow on big projects.
Proposed Changes
Mangler works unefficiently, rewrite it with effective alghorithm.
Implementation Details
Also, air builder was rewritten on StringBuilder for effectiveness.
Checklist
Reviewer Checklist