Skip to content

Commit 8f6a5ce

Browse files
precizSteffenDE
andauthored
Optimize nested assign change tracking (#4225)
* Optimize nested assign change tracking This avoids redundant nested assign change checks when a parent assign is already tracked. Overall render speedup depends on how much time a template spends in those checks. Benchmarked template: ```heex <nav class="actions"> <button disabled={!(@current_user && @current_user.permissions.projects.create)}> New project </button> <button disabled={!(@current_user && @current_user.permissions.projects.archive)}> Archive </button> <button disabled={!(@current_user && @current_user.permissions.billing.manage)}> Billing </button> <button disabled={!(@current_user && @current_user.profile.account.plan.features.exports)}> Export </button> <button disabled={!(@current_user && @current_user.profile.account.settings.security.sso)}> SSO </button> </nav> ``` Benchmark measured the generated dynamic change-check path with assigns.__changed__ = %{} by repeatedly calling dynamic.(true). | Run | Before | After | Speedup | | --- | ---: | ---: | ---: | | 1 | 24,913/s | 69,647/s | 2.80x | | 2 | 22,138/s | 62,867/s | 2.84x | | 3 | 19,470/s | 52,347/s | 2.69x | * add test --------- Co-authored-by: Steffen Deusch <steffen@deusch.me>
1 parent 52a2e79 commit 8f6a5ce

2 files changed

Lines changed: 42 additions & 13 deletions

File tree

lib/phoenix_live_view/engine.ex

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,8 @@ defmodule Phoenix.LiveView.Engine do
692692

693693
defp changed_assigns(assigns) do
694694
checks =
695-
for {{changed_var, key}, _} <- assigns, not nested_and_parent_is_checked?(key, assigns) do
695+
for {{changed_var, key}, _} <- assigns,
696+
not nested_and_parent_is_checked?(changed_var, key, assigns) do
696697
changed = Macro.var(changed_var, __MODULE__)
697698

698699
case key do
@@ -736,19 +737,29 @@ defmodule Phoenix.LiveView.Engine do
736737
# @foo.bar or @foo, we don't need to check for @foo.bar.baz.
737738

738739
# If there is no nesting, then we are not nesting.
739-
defp nested_and_parent_is_checked?([_], _assigns),
740+
defp nested_and_parent_is_checked?(_changed_var, [_], _assigns),
740741
do: false
741742

742743
# Otherwise, we convert @foo.bar.baz into [:baz, :bar, :foo], discard :baz,
743744
# and then check if [:foo, :bar] and then [:foo] is in it.
744-
defp nested_and_parent_is_checked?(keys, assigns),
745-
do: parent_is_checked?(tl(Enum.reverse(keys)), assigns)
745+
defp nested_and_parent_is_checked?(changed_var, keys, assigns),
746+
do: parent_is_checked?(changed_var, tl(Enum.reverse(keys)), assigns)
746747

747-
defp parent_is_checked?([], _assigns),
748+
defp parent_is_checked?(_changed_var, [], _assigns),
748749
do: false
749750

750-
defp parent_is_checked?(rest, assigns),
751-
do: Map.has_key?(assigns, Enum.reverse(rest)) or parent_is_checked?(tl(rest), assigns)
751+
defp parent_is_checked?(changed_var, rest, assigns),
752+
do:
753+
Map.has_key?(assigns, {changed_var, Enum.reverse(rest)}) or
754+
parent_is_checked?(changed_var, tl(rest), assigns)
755+
756+
defp put_changed_assign(assigns, changed_var, key) do
757+
if nested_and_parent_is_checked?(changed_var, key, assigns) do
758+
assigns
759+
else
760+
Map.put(assigns, {changed_var, key}, true)
761+
end
762+
end
752763

753764
## Component keys change tracking
754765

@@ -1025,7 +1036,7 @@ defmodule Phoenix.LiveView.Engine do
10251036
)
10261037
when is_atom(name) and is_atom(context) and is_map_key(map, name) and type != :tainted do
10271038
if map[name] == :change_track do
1028-
{expr, vars, Map.put(assigns, {:vars_changed, [name | nest]}, true)}
1039+
{expr, vars, put_changed_assign(assigns, :vars_changed, [name | nest])}
10291040
else
10301041
analyze(expr, vars, assigns, caller)
10311042
end
@@ -1035,7 +1046,7 @@ defmodule Phoenix.LiveView.Engine do
10351046
defp analyze_assign({:@, meta, [{name, _, context}]}, vars, assigns, _caller, nest)
10361047
when is_atom(name) and is_atom(context) do
10371048
expr = {{:., meta, [@assigns_var, name]}, [no_parens: true] ++ meta, []}
1038-
{expr, vars, Map.put(assigns, {:changed, [name | nest]}, true)}
1049+
{expr, vars, put_changed_assign(assigns, :changed, [name | nest])}
10391050
end
10401051

10411052
# assigns.name
@@ -1047,7 +1058,7 @@ defmodule Phoenix.LiveView.Engine do
10471058
nest
10481059
)
10491060
when is_atom(name) and args in [[], nil] do
1050-
{expr, vars, Map.put(assigns, {:changed, [name | nest]}, true)}
1061+
{expr, vars, put_changed_assign(assigns, :changed, [name | nest])}
10511062
end
10521063

10531064
# assigns[:name]
@@ -1059,7 +1070,7 @@ defmodule Phoenix.LiveView.Engine do
10591070
nest
10601071
)
10611072
when is_atom(name) and is_access(access) do
1062-
{expr, vars, Map.put(assigns, {:changed, [name | nest]}, true)}
1073+
{expr, vars, put_changed_assign(assigns, :changed, [name | nest])}
10631074
end
10641075

10651076
# Maybe: assigns.foo[:bar]
@@ -1138,7 +1149,7 @@ defmodule Phoenix.LiveView.Engine do
11381149
{expr, {:tainted, map}, assigns}
11391150

11401151
%{^name => :change_track} ->
1141-
{expr, vars, Map.put(assigns, {:vars_changed, [name]}, true)}
1152+
{expr, vars, put_changed_assign(assigns, :vars_changed, [name])}
11421153

11431154
_ ->
11441155
{expr, {:restricted, map}, assigns}
@@ -1149,7 +1160,7 @@ defmodule Phoenix.LiveView.Engine do
11491160
when is_atom(name) do
11501161
cond do
11511162
Map.get(map, name) == :change_track ->
1152-
{expr, {type, map}, Map.put(assigns, {:vars_changed, [name]}, true)}
1163+
{expr, {type, map}, put_changed_assign(assigns, :vars_changed, [name])}
11531164

11541165
Keyword.get(meta, :change_track) ->
11551166
# this is a variable inside the left-hand side of a keyed for expression;

test/phoenix_live_view/engine_test.exs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,24 @@ defmodule Phoenix.LiveView.EngineTest do
459459
assert changed(template, %{foo: "a"}, %{}) == [["a", "a", "a"]]
460460
assert changed(template, %{foo: "a"}, %{foo: true}) == [["a", "a", "a"]]
461461
end
462+
463+
test "short-circuits checks when parent is already checked" do
464+
template = "<%= @foo && @foo.bar.baz.qux %>"
465+
quoted = EEx.compile_string(template, file: __ENV__.file, engine: Engine)
466+
code = Macro.to_string(quoted)
467+
468+
refute code =~ "nested_changed_assign",
469+
"expected short-circuiting of @foo checks, got: #{code}"
470+
end
471+
472+
test "short-circuits checks when parent is already checked (vars_changed)" do
473+
template = "<%= for foo <- @bar do %><%= foo && foo.bar.baz.qux %><% end %>"
474+
quoted = EEx.compile_string(template, file: __ENV__.file, engine: Engine)
475+
code = Macro.to_string(quoted)
476+
477+
refute code =~ "nested_changed_assign",
478+
"expected short-circuiting of foo checks, got: #{code}"
479+
end
462480
end
463481

464482
describe "if" do

0 commit comments

Comments
 (0)