Skip to content

Optimize detect_duplicate_ids on parse_document and parse_fragment - #3962

Merged
SteffenDE merged 1 commit into
phoenixframework:mainfrom
ypconstante:optimize-duplicate-id
Sep 4, 2025
Merged

Optimize detect_duplicate_ids on parse_document and parse_fragment#3962
SteffenDE merged 1 commit into
phoenixframework:mainfrom
ypconstante:optimize-duplicate-id

Conversation

@ypconstante

@ypconstante ypconstante commented Aug 24, 2025

Copy link
Copy Markdown
Contributor

This PR replaces the duplicated id check on parse_document and parse_fragment to use lazydoc instead of tree.
This makes the check around 2.5 times faster than the current approach that traverses the tree collecting ids.

Benchmark using the benchmark HTML files from floki.

##### With input big #####
Name                                ips        average  deviation         median         99th %
lazy_query                      2128.92        0.47 ms   ±119.25%        0.32 ms        3.24 ms
current                          772.40        1.29 ms    ±61.16%        0.89 ms        3.55 ms
postreduce                       580.44        1.72 ms    ±48.63%        1.28 ms        4.23 ms
html_to_lazy + lazy_query        206.70        4.84 ms    ±22.23%        4.57 ms       10.57 ms
tree_to_lazy + lazy_query         67.84       14.74 ms     ±7.85%       14.52 ms       20.16 ms

Comparison:
lazy_query                      2128.92
current                          772.40 - 2.76x slower +0.82 ms
postreduce                       580.44 - 3.67x slower +1.25 ms
html_to_lazy + lazy_query        206.70 - 10.30x slower +4.37 ms
tree_to_lazy + lazy_query         67.84 - 31.38x slower +14.27 ms

##### With input medium #####
Name                                ips        average  deviation         median         99th %
lazy_query                       8.27 K      120.93 μs    ±78.56%      106.13 μs      773.70 μs
current                          2.89 K      346.24 μs    ±61.87%      282.85 μs     1511.35 μs
postreduce                       2.13 K      468.50 μs    ±51.16%      399.90 μs     1779.62 μs
html_to_lazy + lazy_query        0.65 K     1541.39 μs    ±36.66%     1334.93 μs     4330.52 μs
tree_to_lazy + lazy_query        0.21 K     4745.89 μs    ±14.12%     4514.47 μs     8165.55 μs

Comparison:
lazy_query                       8.27 K
current                          2.89 K - 2.86x slower +225.31 μs
postreduce                       2.13 K - 3.87x slower +347.58 μs
html_to_lazy + lazy_query        0.65 K - 12.75x slower +1420.47 μs
tree_to_lazy + lazy_query        0.21 K - 39.25x slower +4624.97 μs

##### With input small #####
Name                                ips        average  deviation         median         99th %
lazy_query                      31.75 K       31.50 μs    ±17.07%       30.82 μs       44.71 μs
current                         13.91 K       71.89 μs    ±11.79%       69.76 μs       84.21 μs
postreduce                      10.52 K       95.03 μs     ±5.97%       94.98 μs       99.71 μs
html_to_lazy + lazy_query        3.28 K      304.98 μs     ±8.94%      298.63 μs      398.71 μs
tree_to_lazy + lazy_query        1.08 K      921.87 μs     ±7.26%      910.83 μs     1081.67 μs

Comparison:
lazy_query                      31.75 K
current                         13.91 K - 2.28x slower +40.39 μs
postreduce                      10.52 K - 3.02x slower +63.53 μs
html_to_lazy + lazy_query        3.28 K - 9.68x slower +273.48 μs
tree_to_lazy + lazy_query        1.08 K - 29.27x slower +890.37 μs
read_file = fn name ->
  lazy =
    __ENV__.file
    |> Path.dirname()
    |> Path.join(name)
    |> File.read!()
    |> LazyHTML.from_document()

  tree = LazyHTML.to_tree(lazy)
  html = LazyHTML.Tree.to_html(tree)
  %{lazy: lazy, tree: tree, html: html}
end

inputs = %{
  "big" => read_file.("big.html"),
  "medium" => read_file.("medium.html"),
  "small" => read_file.("small.html")
}

defmodule Bench do
  def current(%{tree: tree}) do
    current(tree, MapSet.new())
  end

  defp current([node | rest], ids) do
    ids = current(node, ids)
    current(rest, ids)
  end

  defp current([], ids) do
    ids
  end

  defp current({_tag, _attr, children} = node, ids) do
    case attribute(node, "id") do
      id when not is_nil(id) ->
        current(children, MapSet.put(ids, id))

      _ ->
        current(children, ids)
    end
  end

  defp current(_node, acc) do
    acc
  end

  def attribute(node, key) do
    with {tag, attrs, _children} when is_binary(tag) <- node,
         {_, value} <- List.keyfind(attrs, key, 0) do
      value
    else
      _ -> nil
    end
  end

  def lazy_query(%{lazy: lazy}) do
    lazy
    |> LazyHTML.query("[id]")
    |> LazyHTML.attribute("id")
    |> MapSet.new()
  end

  def tree_to_lazy_and_lazy_query(%{tree: tree}) do
    tree
    |> LazyHTML.from_tree()
    |> LazyHTML.query("[id]")
    |> LazyHTML.attribute("id")
    |> MapSet.new()
  end

  def html_to_lazy_and_lazy_query(%{html: html}) do
    html
    |> LazyHTML.from_document()
    |> LazyHTML.query("[id]")
    |> LazyHTML.attribute("id")
    |> MapSet.new()
  end

  def postreduce(%{tree: tree}) do
    LazyHTML.Tree.postreduce(tree, MapSet.new(), fn node, ids ->
      if id = attribute(node, "id") do
        MapSet.put(ids, id)
      else
        ids
      end
    end)
  end
end

Benchee.run(
  %{
    "current" => &Bench.current/1,
    "lazy_query" => &Bench.lazy_query/1,
    "tree_to_lazy + lazy_query" => &Bench.tree_to_lazy_and_lazy_query/1,
    "html_to_lazy + lazy_query" => &Bench.html_to_lazy_and_lazy_query/1,
    "postreduce" => &Bench.postreduce/1
  },
  inputs: inputs,
  pre_check: :all_same,
  time: 10
)

Comment thread lib/phoenix_live_view/test/dom.ex Outdated
@SteffenDE

Copy link
Copy Markdown
Member

I think it's good if we move all the duplicate detection logic to Phoenix.LiveViewTest.DOM instead. Please also don't reference the %LazyHTML{} struct as such, as that leads to issues when LazyHTML is not available. Instead, do is_struct(var, LazyHTML).

@ypconstante
ypconstante force-pushed the optimize-duplicate-id branch from f9f0954 to 501e1d8 Compare September 2, 2025 22:02
@ypconstante

Copy link
Copy Markdown
Contributor Author

@SteffenDE moved to Phoenix.LiveViewTest.DOM and removed all changes that I had made to Phoenix.LiveViewTest.TreeDOM. New detect_duplicate_ids is a private function, so no need to check the struct type.

@SteffenDE
SteffenDE merged commit c10752e into phoenixframework:main Sep 4, 2025
7 checks passed
@SteffenDE

Copy link
Copy Markdown
Member

🙌🏻

SteffenDE pushed a commit that referenced this pull request Sep 4, 2025
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