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

Question: why is there exception handling in visitTag? (it slows down compile times considerably) #204

Open
cies opened this issue Dec 27, 2022 · 9 comments
Assignees

Comments

@cies
Copy link

cies commented Dec 27, 2022

We've migrated a significant number of HTML+Groovy templates over the Kotlinx.html.

The DSL in fantastic! We're super happy developers using it... until we compile.

As mentioned in this issue the DSL compiles very, VERY, slow. We have no way of knowing exactly how much time is spend by the compiler on each file (if anyone knows please let me know), but every time we ported a bunch of templates to Kotlinx.html, we saw a stark increase in compile times.

If I would have know this up front, I'd have chosen a different solution. And hence I believe this project's README should warn (potential) users for this caveat.

@cies
Copy link
Author

cies commented Jan 10, 2023

On YouTrack (see the link in the issue) we read the following comment from Dmitry Petrov:

As a workaround, I'd recommend extracting some deeply nested subtrees into separate methods. Each tree of depth N adds O(N^2) edges to the method control flow graph due to nested try-catch-finally blocks in inlined kotlinx.html.visitTag. This slows down everything, including class files generation.

This refers to: https://github.com/Kotlin/kotlinx.html/blob/f55bf940745e6c6166bd39d4e0c213bb44035c3b/src/commonMain/kotlin/visit.kt

I wonder what would happen without allowing tags to override onTagError() and instead by simply letting these errors bubble up. Maybe they can be caught (and "nicely" handled to some extend) at the call site of a template.

@cies
Copy link
Author

cies commented Jan 11, 2023

So I made a small change to kotlin/visit.kt:

inline fun <T : Tag> T.visitTag(block: T.() -> Unit) {
    consumer.onTagStart(this)
    this.block()
    // removed the exception handling, hoping to speed up compilation of our HTML DSL code
    consumer.onTagEnd(this)
}

And the from-clean compile time of our entire project (containing a lot of non-Kotlin-related tasks) went down from 5min3sec to 2min46sec.

I tried to run the some HTML DSL templating code with a throw RuntimeException() deeply nested into it, and it presents me the error and stacktrace just fine. I'm obviously really curious why the exception handling code I removed was there to begin with. Anyone?

@cies cies changed the title README needs a "slows down compilation considerably" warning Question: why is there exception handling in visitTag? (it slows down compile times considerably) Jan 11, 2023
@cies
Copy link
Author

cies commented Jan 11, 2023

I change the title, but I still believe that in it's current shape this library should warn for slow compile times (somewhere in the README). It is slow to a point that I would not recommend someone to use this library for a project with lots of templates (like the project we have with 150+ templates (pages) at a combined 30kLOC).

Otherwise I much prefer this eDSL over what I get with template engines!

@cies
Copy link
Author

cies commented Jan 12, 2023

@cies
Copy link
Author

cies commented Jan 12, 2023

We're considering to fork the project for the time being, having only one difference (the hack described above), only publishing one package (the kotlinx-html-jvm package).

EDIT: We did, see the very minimal change here: Stager-Software@a719b02

The next comment links to the benchmark results of this change among others.

@cies
Copy link
Author

cies commented Mar 17, 2023

I've used the minimal example from @spand to create a little benchmark tool in order to get to the bottom of what slows down/ speeds up kotlinx-html compilation. Find it here: https://github.com/cies/SlowKotlinxHtml

The results are interesting, showing various improvements tweaking different parameters.

We've also published a hacked version of kotlinx-html-jvm with the exception handling removed. As seen in the benchmarks this improves compile speeds by about 38%. Reducing the nesting of the eDSL to max 4 levels deep improved it by about 6%. Using K2 and adding some compiler flags improved it by 20%. Obviously this is on an unrealistically simple example code base, so this may not translate to your project.

@spand
Copy link

spand commented Mar 21, 2023

Thats pretty nice @cies . Hopefully we can get someone to look at the issue and get a faster 0.9 out of the door

@cies
Copy link
Author

cies commented Mar 22, 2023

Thanks for chiming in @spand , and thanks for your benchmarks code I thankfully made use of. I think a large part of the compilation slowness (that's left after tuning the parameters and hacking out the exception handling) comes from the fact that we use "deeply nested Kotlin eDSL syntax", and the eDSL syntax being slow to compile even when not deeply nested. But that's a hunch at this point.

This would mean the problem is less with kotlinx.html and more with the Kotlin compiler.

@cies
Copy link
Author

cies commented Aug 9, 2023

This commit: f1ca2c6

Seems to has implemented the changes I proposed! Thanks a lot @spand , @hhariri and ultimately @e5l. Feel free to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants