Repository: adobe/elixir-styler
Pull request #234 (2025-06-12 19:27:32)

Revert "optimize zipper performance"
Running against a large elixir umbrella app, the optimization showed a 50% slow down :O

```
➜ # v1.4.2
➜ hyperfine 'mix format' -i --warmup 1 --runs 32 # v1.4.2
Benchmark 1: mix format
  Time (mean ± σ):      1.094 s ±  0.027 s    [User: 0.781 s, System: 0.818 s]
  Range (min … max):    1.048 s …  1.155 s    32 runs

➜ # styler main
➜ hyperfine 'mix format' -i --warmup 1 --runs 32 
Benchmark 1: mix format
  Time (mean ± σ):      1.534 s ±  0.037 s    [User: 1.009 s, System: 0.979 s]
  Range (min … max):    1.475 s …  1.655 s    32 runs
```

novaugust: cc @doorgan - PR'd this so you could see :O this reverts the tuple-instead-of-map optimization in styler's zipper. it seems that Zipper.traverse is a poor benchmark, but honestly the 50% slowdown means something very odd is happening in there. maybe there's a singular issue i could identify that'd speed things back up.

but honestly, `mix format` bottlenecks on IO so hard that running with-or-without styler makes no difference to user-perceived runtime so i might walk from optimizations for now ;)

```
➜  # running with no styler
➜  hyperfine 'mix format' -i --warmup 1 --runs 32 # no styler
Revert "optimize zipper performance"
Benchmark 1: mix format
  Time (mean ± σ):      1.095 s ±  0.041 s    [User: 0.788 s, System: 0.830 s]
  Range (min … max):    1.046 s …  1.255 s    32 runs
```

Patch:
diff --git a/lib/style/configs.ex b/lib/style/configs.ex
index a537989..d83b447 100644
--- a/lib/style/configs.ex
+++ b/lib/style/configs.ex
@@ -48,9 +48,8 @@ defmodule Styler.Style.Configs do
   end
 
   def run({{:config, cfm, [_, _ | _]} = config, zm}, %{mix_config?: true, comments: comments} = ctx) do
-    {l, p, r} = zm
     # all of these list are reversed due to the reduce
-    {configs, assignments, rest} = accumulate(r, [], [])
+    {configs, assignments, rest} = accumulate(zm.r, [], [])
     # @TODO
     # okay so comments between nodes that we moved.......
     # lets just push them out of the way (???). so
@@ -93,9 +92,9 @@ defmodule Styler.Style.Configs do
         {nodes, comments}
       end
 
-    [config | left_siblings] = Enum.reverse(nodes, l)
+    [config | left_siblings] = Enum.reverse(nodes, zm.l)
 
-    {:skip, {config, {left_siblings, p, rest}}, %{ctx | comments: comments}}
+    {:skip, {config, %{zm | l: left_siblings, r: rest}}, %{ctx | comments: comments}}
   end
 
   def run(zipper, %{config?: true} = ctx) do
diff --git a/lib/style/module_directives.ex b/lib/style/module_directives.ex
index 16a372e..b88f3c6 100644
--- a/lib/style/module_directives.ex
+++ b/lib/style/module_directives.ex
@@ -107,9 +107,9 @@ defmodule Styler.Style.ModuleDirectives do
   # puts `@derive` before `defstruct` etc, fixing compiler warnings
   def run({{:@, _, [{:derive, _, _}]}, _} = zipper, ctx) do
     case Style.ensure_block_parent(zipper) do
-      {:ok, {derive, {l, p, r}}} ->
+      {:ok, {derive, %{l: left_siblings} = z_meta}} ->
         previous_defstruct =
-          l
+          left_siblings
           |> Stream.with_index()
           |> Enum.find_value(fn
             {{struct_def, meta, _}, index} when struct_def in @defstruct -> {meta[:line], index}
@@ -119,8 +119,8 @@ defmodule Styler.Style.ModuleDirectives do
         if previous_defstruct do
           {defstruct_line, defstruct_index} = previous_defstruct
           derive = Style.set_line(derive, defstruct_line - 1)
-          left_siblings = List.insert_at(l, defstruct_index + 1, derive)
-          {:skip, Zipper.remove({derive, {left_siblings, p, r}}), ctx}
+          left_siblings = List.insert_at(left_siblings, defstruct_index + 1, derive)
+          {:skip, Zipper.remove({derive, %{z_meta | l: left_siblings}}), ctx}
         else
           {:cont, zipper, ctx}
         end
diff --git a/lib/zipper.ex b/lib/zipper.ex
index c7a4487..9b43d1e 100644
--- a/lib/zipper.ex
+++ b/lib/zipper.ex
@@ -26,8 +26,14 @@ defmodule Styler.Zipper do
   import Kernel, except: [node: 1]
 
   @type tree :: Macro.t()
+
+  @opaque path :: %{
+            l: [tree],
+            ptree: zipper,
+            r: [tree]
+          }
+
   @type zipper :: {tree, path | nil}
-  @type path :: {left :: [tree], parent :: zipper, right :: [tree]}
   @type t :: zipper
   @type command :: :cont | :skip | :halt
 
@@ -85,7 +91,7 @@ defmodule Styler.Zipper do
   def down(zipper) do
     case children(zipper) do
       [] -> nil
-      [first | rest] -> {first, {[], zipper, rest}}
+      [first | rest] -> {first, %{ptree: zipper, l: [], r: rest}}
     end
   end
 
@@ -96,8 +102,9 @@ defmodule Styler.Zipper do
   @spec up(zipper) :: zipper | nil
   def up({_, nil}), do: nil
 
-  def up({tree, {l, {parent, parent_meta}, r}}) do
-    children = Enum.reverse(l, [tree | r])
+  def up({tree, meta}) do
+    children = Enum.reverse(meta.l, [tree | meta.r])
+    {parent, parent_meta} = meta.ptree
     {do_replace_children(parent, children), parent_meta}
   end
 
@@ -105,16 +112,16 @@ defmodule Styler.Zipper do
   Returns the zipper of the left sibling of the node at this zipper, or nil.
   """
   @spec left(zipper) :: zipper | nil
-  def left({tree, {[ltree | l], p, r}}), do: {ltree, {l, p, [tree | r]}}
+  def left({tree, %{l: [ltree | l], r: r} = meta}), do: {ltree, %{meta | l: l, r: [tree | r]}}
   def left(_), do: nil
 
   @doc """
   Returns the leftmost sibling of the node at this zipper, or itself.
   """
   @spec leftmost(zipper) :: zipper
-  def leftmost({tree, {[_ | _] = l, p, r}}) do
-    [leftmost | r] = Enum.reverse(l, [tree | r])
-    {leftmost, {[], p, r}}
+  def leftmost({tree, %{l: [_ | _] = l} = meta}) do
+    [leftmost | r] = Enum.reverse(l, [tree | meta.r])
+    {leftmost, %{meta | l: [], r: r}}
   end
 
   def leftmost({_, _} = zipper), do: zipper
@@ -123,16 +130,16 @@ defmodule Styler.Zipper do
   Returns the zipper of the right sibling of the node at this zipper, or nil.
   """
   @spec right(zipper) :: zipper | nil
-  def right({tree, {l, p, [rtree | r]}}), do: {rtree, {[tree | l], p, r}}
+  def right({tree, %{r: [rtree | r]} = meta}), do: {rtree, %{meta | r: r, l: [tree | meta.l]}}
   def right(_), do: nil
 
   @doc """
   Returns the rightmost sibling of the node at this zipper, or itself.
   """
   @spec rightmost(zipper) :: zipper
-  def rightmost({tree, {l, p, [_ | _] = r}}) do
-    [rightmost | l] = Enum.reverse(r, [tree | l])
-    {rightmost, {l, p, []}}
+  def rightmost({tree, %{r: [_ | _] = r} = meta}) do
+    [rightmost | l] = Enum.reverse(r, [tree | meta.l])
+    {rightmost, %{meta | l: l, r: []}}
   end
 
   def rightmost({_, _} = zipper), do: zipper
@@ -156,8 +163,8 @@ defmodule Styler.Zipper do
   """
   @spec remove(zipper) :: zipper
   def remove({_, nil}), do: raise(ArgumentError, message: "Cannot remove the top level node.")
-  def remove({_, {[left | rest], p, r}}), do: prev_down({left, {rest, p, r}})
-  def remove({_, {_, {parent, parent_meta}, children}}), do: {do_replace_children(parent, children), parent_meta}
+  def remove({_, %{l: [left | rest]} = meta}), do: prev_down({left, %{meta | l: rest}})
+  def remove({_, %{ptree: {parent, parent_meta}, r: children}}), do: {do_replace_children(parent, children), parent_meta}
 
   @doc """
   Inserts the item as the left sibling of the node at this zipper, without
@@ -177,7 +184,7 @@ defmodule Styler.Zipper do
   """
   @spec prepend_siblings(zipper, [tree]) :: zipper
   def prepend_siblings({node, nil}, siblings), do: {:__block__, [], siblings ++ [node]} |> zip() |> down() |> rightmost()
-  def prepend_siblings({tree, {l, p, r}}, siblings), do: {tree, {Enum.reverse(siblings, l), p , r}}
+  def prepend_siblings({tree, meta}, siblings), do: {tree, %{meta | l: Enum.reverse(siblings, meta.l)}}
 
   @doc """
   Inserts the item as the right sibling of the node at this zipper, without
@@ -197,7 +204,7 @@ defmodule Styler.Zipper do
   """
   @spec insert_siblings(zipper, [tree]) :: zipper
   def insert_siblings({node, nil}, siblings), do: {:__block__, [], [node | siblings]} |> zip() |> down()
-  def insert_siblings({tree, {l, p, r}}, siblings), do: {tree, {l, p, siblings ++ r}}
+  def insert_siblings({tree, meta}, siblings), do: {tree, %{meta | r: siblings ++ meta.r}}
 
   @doc """
   Inserts the item as the leftmost child of the node at this zipper,


Test patch:
diff --git a/test/zipper_test.exs b/test/zipper_test.exs
index 81dcf6e..2359d48 100644
--- a/test/zipper_test.exs
+++ b/test/zipper_test.exs
@@ -63,14 +63,14 @@ defmodule StylerTest.ZipperTest do
 
   describe "down/1" do
     test "rips and tears the parent node" do
-      assert [1, 2] |> Zipper.zip() |> Zipper.down() == {1, {[], {[1, 2], nil}, [2]}}
-      assert {1, 2} |> Zipper.zip() |> Zipper.down() == {1, {[], {{1, 2}, nil}, [2]}}
+      assert [1, 2] |> Zipper.zip() |> Zipper.down() == {1, %{l: [], r: [2], ptree: {[1, 2], nil}}}
+      assert {1, 2} |> Zipper.zip() |> Zipper.down() == {1, %{l: [], r: [2], ptree: {{1, 2}, nil}}}
 
       assert {:foo, [], [1, 2]} |> Zipper.zip() |> Zipper.down() ==
-               {1, {[], {{:foo, [], [1, 2]}, nil}, [2]}}
+               {1, %{l: [], r: [2], ptree: {{:foo, [], [1, 2]}, nil}}}
 
       assert {{:., [], [:a, :b]}, [], [1, 2]} |> Zipper.zip() |> Zipper.down() ==
-               {{:., [], [:a, :b]}, {[],{{{:., [], [:a, :b]}, [], [1, 2]}, nil}, [1, 2]}}
+               {{:., [], [:a, :b]}, %{l: [], r: [1, 2], ptree: {{{:., [], [:a, :b]}, [], [1, 2]}, nil}}}
     end
   end
 
@@ -471,8 +471,8 @@ defmodule StylerTest.ZipperTest do
     end
 
     test "builds a new root node made of a block" do
-      assert {42, {[:nope], {{:__block__, _, _}, nil}, []}} = 42 |> Zipper.zip() |> Zipper.insert_left(:nope)
-      assert {42, {[], {{:__block__, _, _}, nil}, [:nope]}} = 42 |> Zipper.zip() |> Zipper.insert_right(:nope)
+      assert {42, %{l: [:nope], ptree: {{:__block__, _, _}, nil}}} = 42 |> Zipper.zip() |> Zipper.insert_left(:nope)
+      assert {42, %{r: [:nope], ptree: {{:__block__, _, _}, nil}}} = 42 |> Zipper.zip() |> Zipper.insert_right(:nope)
     end
   end