[repo-assist] perf: HtmlParser reuses CurrentTag and Content StringBuilders across tokens - #1789
Draft
github-actions[bot] wants to merge 1 commit into
Conversation
…tokens
Instead of allocating a new { Contents = StringBuilder() } on every
EmitTag, EmitSelfClosingTag, EmitToAttributeValue, and Emit call,
reuse the existing CharList by calling .Clear(). This eliminates two
StringBuilder allocations per HTML element (one for CurrentTag, one
for Content) - for a typical HTML document with hundreds of elements
this removes hundreds of short-lived heap objects.
The CharList.Clear() method already existed for this purpose.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
The HTML parser's
HtmlStatetype previously allocated a new{ Contents = StringBuilder() }(a newCharListrecord and a newStringBuilder) on every call to:EmitSelfClosingTag()— resetCurrentTagEmitTag()— resetCurrentTagEmitToAttributeValue()— resetContentEmit()— resetContentFor a typical HTML document with hundreds of elements, this means hundreds of short-lived
StringBuilderobjects, all discarded immediately after.Clear()would have sufficed.Fix
Replace
x.CurrentTag <- { Contents = StringBuilder() }andx.Content <- { Contents = StringBuilder() }withx.CurrentTag.Clear()andx.Content.Clear()respectively.The
CharList.Clear()method already existed for exactly this purpose (line 90).Benefit
CharListrecord + oneStringBuilder) for bothCurrentTagandContentresets — roughly 4 allocations per opening/closing tag pair.Trade-offs
None — this is a pure internal refactor. The
CharListstruct already reuses itsStringBuildersafely via.Clear().Test Status
All 2284 HTML-related tests pass on Linux/net8.0 (FSharp.Data.Core.Tests, HTML filter).
Closes #none (standalone performance improvement)
Add this agentic workflow to your repo
To install this agentic workflow, run