Skip to content

[repo-assist] perf: HtmlDocument.ToString reuses one StringBuilder across all child elements #1787

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.

Sign up for GitHub

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

Draft
github-actions wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[repo-assist] perf: HtmlDocument.ToString reuses one StringBuilder across all child elements #1787

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions src/FSharp.Data.Html.Core/HtmlNode.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,25 +118,26 @@ type HtmlNode =
/// <param name="content">The actual content</param>
static member NewCData content = HtmlCData(content)

override x.ToString() =
let sb = StringBuilder()
/// Serialize this node into an existing StringBuilder.
/// Used by HtmlDocument.ToString() to avoid creating an intermediate string per element.
member internal x.SerializeTo(sb: StringBuilder, indentation: int, canAddNewLine: bool, insidePre: bool) =
let append (str: string) = sb.Append str |> ignore

let appendEndTag name =
append "</"
append name
append ">"

let newLine indentation plus =
let newLine ind plus =
sb.AppendLine() |> ignore
sb.Append(' ', indentation + plus) |> ignore
sb.Append(' ', ind + plus) |> ignore

// serialization uses an explicit work stack instead of call-stack recursion:
// the parser accepts arbitrarily deep documents (it uses its own stack), so
// ToString must not StackOverflow on them either
let work = System.Collections.Generic.Stack<unit -> unit>()

let rec serialize indentation canAddNewLine insidePre html =
let rec serialize ind canAdd inPre html =
match html with
| HtmlElement(name, attributes, elements) ->
let onlyText =
Expand All @@ -146,10 +147,10 @@ type HtmlNode =
| _ -> false)

let isPreTag = name = "pre"
let nowInsidePre = insidePre || isPreTag
let nowInsidePre = inPre || isPreTag

if canAddNewLine && not insidePre && not (onlyText || isPreTag) then
newLine indentation 0
if canAdd && not inPre && not (onlyText || isPreTag) then
newLine ind 0

append "<"
append name
Expand All @@ -169,13 +170,13 @@ type HtmlNode =
else
append ">"

if not insidePre && not (onlyText || isPreTag) then
newLine indentation 2
if not inPre && not (onlyText || isPreTag) then
newLine ind 2

// pushed first so it runs after all children are processed
work.Push(fun () ->
if not insidePre && not (onlyText || isPreTag) then
newLine indentation 0
if not inPre && not (onlyText || isPreTag) then
newLine ind 0

appendEndTag name)

Expand All @@ -186,7 +187,7 @@ type HtmlNode =
for i in elements.Length - 1 .. -1 .. 0 do
let element = elements.[i]
let canAddNewLine = i > 0
work.Push(fun () -> serialize (indentation + 2) canAddNewLine nowInsidePre element)
work.Push(fun () -> serialize (ind + 2) canAddNewLine nowInsidePre element)
| HtmlText str -> append str
| HtmlComment str ->
append "<!--"
Expand All @@ -197,11 +198,14 @@ type HtmlNode =
append str
append "]]>"

serialize 0 false false x
serialize indentation canAddNewLine insidePre x

while work.Count > 0 do
work.Pop () ()

override x.ToString() =
let sb = StringBuilder()
x.SerializeTo(sb, 0, false, false)
sb.ToString()

/// <exclude />
Expand Down Expand Up @@ -250,8 +254,9 @@ type HtmlDocument =
sb.Append(docType) |> ignore
sb.AppendLine(">") |> ignore

// Reuse the same StringBuilder for all elements to avoid intermediate strings.
for element in elements do
sb.Append(element.ToString()) |> ignore
element.SerializeTo(sb, 0, false, false)

sb.ToString()

Expand Down