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

Use standard library functions where possible #215

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jeroenvandijk
Copy link

@jeroenvandijk jeroenvandijk commented Oct 14, 2024

This commit removes some unnecessary interop and make it easier to use hiccup from other environments (like Sci)

  • .substring to subs
  • .indexOf to clojure.string/index-of (with backwards compatibility for older clojure versions <1.8)
  • Replace unnecessary String/valueOf with str (reasoning here)

It is a first step towards resolving issues #184 and #127

I've left the StringBuilder interop for now, as it might have performance benefits and a (good) patch would be more involved.

jeroenvandijk referenced this pull request Oct 14, 2024
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 #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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant