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

Exponential increase in generated code when there are multiple nested hiccup2.core/html calls #210

Open
luontola opened this issue Mar 11, 2024 · 7 comments

Comments

@luontola
Copy link
Contributor

luontola commented Mar 11, 2024

This is similar to #205 but less common. Consider the following code:

(ns example
  (:require [clojure.walk :as walk]
            [hiccup2.core :as h]))

(walk/macroexpand-all
 `(h/html
   [:p (identity "") "foo"]
   (identity (h/html
              [:p (identity "") "bar"]
              (identity (h/html
                         [:p (identity "") "gazonk"]

In the generated code, foo will appear 8 times, bar 64 times, and gazonk 512 times.

If you try to evaluate this code normally (i.e. without macroexpand-all), it will throw "IndexOutOfBoundsException: Method code too large!"

Each level of nesting multiplies the code by 8, because every h/html checks hiccup.util/*html-mode* and hiccup.util/*escape-strings?* again and generates the 8 code paths.

Is there a way for a Clojure macro to recognize that it's nested within itself? Then those 8 code paths could be generated at only the outermost h/html, and the inner macros could use the same html and escape mode as the outermost macro.

Hiccup version 2.0.0-RC3

Workaround

Extract the inner html macro call to a function, so that you won't have many nested html macros.

@weavejester
Copy link
Owner

Macros are evaluated from the outside in, so we could certainly look through the syntax tree and do something to reduce the nesting. There may also be a better solution to the problem, but keeping the API backward compatible ties our hands somewhat.

@luontola
Copy link
Contributor Author

luontola commented Mar 20, 2024

If html receives an options map with :mode and :escape-strings?, will it then generate only one code variant? Could then the outermost html insert matching options maps to the nested html forms?

Alternatively, create a new internal function similar to compile-html-with-bindings, but for generating only one code variant, and replace the nested html forms with that.

@habruening
Copy link

Wouldn't it be helpful to be able to optionally disable all the preprocessing at compile time and do everything at runtime? Then the evaluation happens inside-out, and everything becomes easier. If someone has code like in this issue, he may not want an optimisation at compile time.

@weavejester
Copy link
Owner

I think ideally I want to separate out compilation entirely, but that will require some consideration and probably a hiccup3 namespace.

@luontola
Copy link
Contributor Author

luontola commented Oct 7, 2024

I came up with an idea for solving this issue by decomplecting the Hiccup API: Instead of hiccup2.core/html which generates the 8 code paths, provide the following macros which take no option parameters:

  • hiccup3.core/html
  • hiccup3.core/xhtml
  • hiccup3.core/xml
  • hiccup3.core/sgml
  • hiccup3.core/unsafe-html
  • hiccup3.core/unsafe-xhtml
  • hiccup3.core/unsafe-xml
  • hiccup3.core/unsafe-sgml

In application code, typically each template will always be used to produce the same output format. So we don't need the ability to switch between different output formats. You might have some SVG XML inside an HTML component, but that's about it.

To support code completion, the variants which don't escape strings are prefix with "unsafe-"; this way they will not be shown in code completion after the user has typed the first character.

In library code it may make sense to support different output formats, so component libraries can continue using hiccup2.core/html.

For backward compatibility, hiccup2.core/html can do the 8-way switch and delegate to those 8 macros.

hiccup2.core/html binds hiccup.util/*html-mode* and hiccup.util/*escape-strings?*. Is this use of binding there to support using component libraries? For backward compatibility, the new macros should also bind them to their respective values.

@habruening
Copy link

I found an easy solution for that problem. Just move a let around your hiccup vector. Then Hiccup cannot do any compile-time optimisations.

@habruening
Copy link

In your case you don't even need the let. Just hiccup everything at the end.

(macroexpand-1 '(h/html
 (list [:p (identity "") "foo"]
       (list   [:p (identity "") "bar"]
               (identity (list [:p (identity "") "gazonk"]))))))

I don't know if this answers the problem. In all my cases where I had a big macro expansion I was able to fix it. In my opinion using list instead of h/html is even the better solution, because the whole idea of Hiccup is to use Clojure datastructures for HTML and not a special Hiccup vector structure, that needs special mechanisms with special behaviour to concat them. For Clojure data structures list is the natural way to concat them.

luontola added a commit to luontola/territory-bro that referenced this issue Oct 27, 2024
Why:
- Hiccup generates 8 code paths every time the hiccup2.core/html macro
  is used, one for each combination of the output formats (html, xhtml,
  xml, sgml) and string escape mode (true, false). In practice, an
  application uses only one or two of them, and the rest is dead code.
- This fixes the problem of exponential amount of generated code in some
  situations. See weavejester/hiccup#210
luontola added a commit to luontola/territory-bro that referenced this issue Oct 27, 2024
Why:
- Hiccup generates 8 code paths every time the hiccup2.core/html macro
  is used, one for each combination of the output formats (html, xhtml,
  xml, sgml) and string escape mode (true, false). In practice, an
  application uses only one or two of them, and the rest is dead code.
- This fixes the problem of exponential amount of generated code in some
  situations. See weavejester/hiccup#210
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

3 participants