-
Notifications
You must be signed in to change notification settings - Fork 177
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
"Method code too large!" when compiling the hiccup2.core/html macro #205
Comments
That sounds fine, thanks for your investigation of this. Happy to accept a PR to reduce the compiled code size. |
luontola
added a commit
to luontola/hiccup
that referenced
this issue
Dec 1, 2023
luontola
added a commit
to luontola/hiccup
that referenced
this issue
Dec 6, 2023
It was possible for the macros to generate so much bytecode that they would exceed Java's 64KB limit for method code size. In such situations the Clojure compiler would fail with the message "Method code too large!" See weavejester#205 This commit does the following optimizations: 1. Concatenate string literals at compile time, so that we can replace multiple StringBuilder.append() calls with just one. This reduces the generated code from O(n) to O(1). This also improves performance by 10-20%, since copying one long string is faster than many short strings. 2. When a runtime check is needed to determine whether a value is the element's attribute map or its first child element, avoid duplicating the code for the element's content. This reduces the generated code from O(n^2) to O(n). While improving the test coverage, some edge cases of generating bad HTML were detected. This commit doesn't change the existing behavior, but only documents it in the new tests. Fixing that behavior will be done in future commits.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
Below is an example of a pretty small function
layout-page
that can't be compiled on Hiccup 2.0.0-RC2.The code generated by the
hiccup2.core/html
macro exceeds Java's 64KB maximum size for a method (calculated in bytecode instructions). Trying to load this namespace will throwMethod code too large!
from ASM when the Clojure compiler tries to compile the method:Analysis
Note in the example, that I've commented out the empty attribute maps (
#_{}
) that precede function calls and other runtime values. If you change even one of them back to{}
, then the generated code will be slightly less than 64KB and the function can be compiled. But if all five of them are removed/commented out, as above, then the compiler crashes.I've created a gist which shows the
macroexpand-all
output and how much the generated code increases every time you remove one of the{}
empty attribute maps: https://gist.github.com/luontola/bb95eb7779c3d18b5486cdfe360f843cCause 1
This is primarily caused by
hiccup.compiler/compile-element ::literal-tag
which duplicates the whole element every time it can't know at compile time whether a value is an attribute map or a child element.For example, when the
(layout-navigation path navigation-tree)
call is preceded by[:div.l-docs {}
, then Hiccup generates this much code:But when the
(layout-navigation path navigation-tree)
call is preceded by[:div.l-docs
, without the{}
, then Hiccup nearly doubles the amount of generated code, even though the only difference between the variants is the starting tag:I'm working on a fix for this. I have it already working, but it needs some cleanup. I'll create a PR for it soon.
Cause 2
Big part of the generated output consists of appending small string literals to a StringBuilder:
It's possible to change
hiccup.compiler/build-string
to concatenate these string literals at compile time. We can also avoid the(if x x "")
for string literals.This will result in much less bytecode instructions, making it harder to get over the 64KB method code limit. The string literals themselves are stored in the class file's constant pool, so they don't count against the method code limit. It should also be faster to concatenate a few big strings than lots of small strings.
I have a fix for this ready and will create a PR soon, together with the fix for cause 1.
The text was updated successfully, but these errors were encountered: