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.

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

Performance improvement: single parser instance #254

Merged
merged 12 commits into from Aug 27, 2021
Merged

Conversation

DieMyst
Copy link
Member

@DieMyst DieMyst commented Aug 27, 2021

Use one parser instance for performance reasons.

Performance was increased both for jvm and js builds.
JVM: from 5-7 seconds to 3 seconds
JS: from >30 seconds to 7 seconds

fixes #250

@DieMyst DieMyst requested a review from alari August 27, 2021 08:16
@DieMyst DieMyst self-assigned this Aug 27, 2021
Copy link
Member

@alari alari left a comment

Choose a reason for hiding this comment

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

Awesome!

build.sbt Outdated
"org.typelevel" %%% "cats-free" % catsV
"org.typelevel" %%% "cats-parse" % catsParseV,
"org.typelevel" %%% "cats-free" % catsV,
"org.typelevel" %%% "shapeless3-deriving" % "3.0.1"
Copy link
Member

Choose a reason for hiding this comment

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

It seems unused

case Right(value) => value
case Left(e) => Validated.invalidNec(LexerError[S](e.wrapErr))
}
def fromString[S[_]: Comonad](parser: String => ValidatedNec[ParserError[S], Ast[S]], script: String): ValidatedNec[ParserError[S], Ast[S]] =
Copy link
Member

Choose a reason for hiding this comment

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

It seems useless now

}
}

def natParser[S[_] : LiftParser : Comonad, K[_] : Comonad]
Copy link
Member

Choose a reason for hiding this comment

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

Code formatting, braces seem strange

extends Expr[F](AbilityIdExpr, ability)
extends Expr[F](AbilityIdExpr, ability) {

def mapK[K[_]: Comonad](fk: F ~> K): Expr[K] =
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better return AbilityIdExpr[K]?

@DieMyst DieMyst changed the title 250 single parser instance Performance improvement: single parser instance Aug 27, 2021
@DieMyst DieMyst merged commit dae234d into main Aug 27, 2021
@DieMyst DieMyst deleted the 250-single-parser branch August 27, 2021 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance: create parser only once on the compilation
2 participants