Skip to content

Defer visible/invisible HTML rendering until actually requested#31

Open
bigio wants to merge 1 commit into
apache:trunkfrom
bigio:speed_html_rendered_lazy
Open

Defer visible/invisible HTML rendering until actually requested#31
bigio wants to merge 1 commit into
apache:trunkfrom
bigio:speed_html_rendered_lazy

Conversation

@bigio

@bigio bigio commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

rendered() unconditionally computed get_rendered_text() three times per HTML part (full, visible-only, invisible-only), each doing a full mask-and-whitespace-cleanup pass over the text. Keep the HTML parser object around instead and only compute the visible/invisible variants inside visible_rendered()/invisible_rendered() when something actually calls them, since not every ruleset uses invisible-text rules.

rendered() unconditionally computed get_rendered_text() three times
per HTML part (full, visible-only, invisible-only), each doing a full
mask-and-whitespace-cleanup pass over the text. Keep the HTML parser
object around instead and only compute the visible/invisible variants
inside visible_rendered()/invisible_rendered() when something actually
calls them, since not every ruleset uses invisible-text rules.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@fkoyer

fkoyer commented Jul 21, 2026

Copy link
Copy Markdown

I like the idea of doing things lazily to improve performance. I also like the idea of moving towards making Node.pm type-agnostic. In other words, I was thinking about moving all HTML handling into the HTML handler at some point in the near future. Currently that would mean moving lines 814–892 of Node::rendered() into Handler::HTML. This change makes that slightly more complicated because both visible_rendered and invisible_rendered need to know about html_obj. Your change works because, right now, text/html is the only type that emits invisible text. I like to keep things pure in case someday there's another part type that emits invisible text (I don't know what that would be). But I don't dispute that this change has a slight performance advantage, if there are no invisible text rules.

One more thing to note, bayes_token_sources defaults to "header visible invisible uri" so if Bayes is enabled, both visible_rendered() and invisible_rendered() fire on every HTML part and both deferred passes run anyway.

@bigio

bigio commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

We could revisit this once you moved the code into the HTML handler, I will keep the PR open as a reminder.

@fkoyer

fkoyer commented Jul 22, 2026

Copy link
Copy Markdown

One possible idea: check invisible_rendered first, if it returns nothing then set visible_rendered = full rendered. That would save one pass if there's no invisible text.

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.

2 participants