Skip to content

Enhancement: Extend GarbageCollector #766

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Enhancement: Extend GarbageCollector #766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0660e78
Add to_remove flag for garbage collecting state
hhubert6 Sep 12, 2025
8ad33d7
Replace DebuggerTerminated event with Process.monitor
hhubert6 Sep 30, 2025
6f25388
Refactor GC remove flag mechanism
hhubert6 Sep 30, 2025
94f0865
Update specs and refactor
hhubert6 Sep 30, 2025
15d0d1a
Add saving state mount and handle_params calls
hhubert6 Sep 30, 2025
165c0c1
Update tests
hhubert6 Oct 1, 2025
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
8 changes: 0 additions & 8 deletions lib/live_debugger/app/debugger/web/debugger_live.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ defmodule LiveDebugger.App.Debugger.Web.DebuggerLive do
alias LiveDebugger.Bus

alias LiveDebugger.App.Debugger.Events.NodeIdParamChanged
alias LiveDebugger.App.Events.DebuggerTerminated

@impl true
def mount(%{"pid" => string_pid}, _session, socket) do
Expand Down Expand Up @@ -104,13 +103,6 @@ defmodule LiveDebugger.App.Debugger.Web.DebuggerLive do
@impl true
def handle_info(_, socket), do: {:noreply, socket}

@impl true
def terminate(_, _) do
Bus.broadcast_event!(%DebuggerTerminated{
debugger_pid: self()
})
end

defp init_debugger(socket, pid) when is_pid(pid) do
socket
|> Hooks.AsyncLvProcess.init(pid)
Expand Down
1 change: 0 additions & 1 deletion lib/live_debugger/app/events.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ defmodule LiveDebugger.App.Events do
defevent(UserRefreshedTrace)

defevent(DebuggerMounted, debugged_pid: pid(), debugger_pid: pid())
defevent(DebuggerTerminated, debugger_pid: pid())

defevent(FindSuccessor, lv_process: LvProcess.t())
end
12 changes: 12 additions & 0 deletions lib/live_debugger/services/callback_tracer/actions/state.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ defmodule LiveDebugger.Services.CallbackTracer.Actions.State do
alias LiveDebugger.API.StatesStorage
alias LiveDebugger.API.LiveViewDebug
alias LiveDebugger.Structs.Trace
alias LiveDebugger.Structs.LvState

alias LiveDebugger.Bus
alias LiveDebugger.Services.CallbackTracer.Events.StateChanged
Expand All @@ -18,6 +19,11 @@ defmodule LiveDebugger.Services.CallbackTracer.Actions.State do
do_save_state!(pid)
end

def maybe_save_state!(%Trace{pid: pid, function: function, args: [_, _, socket]})
when function in [:mount, :handle_params] do
do_save_initial_state!(pid, socket)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that this variant is used for LiveViews that dies immediately and we cannot fetch the state as usual via LiveViewDebug.liveview_state/1 because it may be already dead, right? The problem with this approach is that you don't have a list of LiveComponents in the state storage which theoretically may affect LiveViews that do not die immediately.

What will happen if someone triggers handle_params/3 that does not change any assigns? handle_params/3 in this case will be the last callback triggered in given LiveView (render/1 will not be triggered because assigns did not change). In such case components inspection may break

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite follow. If LiveView process dies so quickly that we can't fetch any information from it (e.g. LiveComponents) then we can't do much. But we can save socket from arguments of callbacks and show some information about this LiveView.

The problem with this approach is that you don't have a list of LiveComponents in the state storage which theoretically may affect LiveViews that do not die immediately.

In what way?

If the LiveView does not crash to the first connected render then most likely we will save it's state (full information about LiveComponents). There is still some chance that it will crash right after this render before processing our state fetch request but again we can't do much then.

end
Comment on lines +22 to +25
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's made as a preparation for better debugging dead LiveViews


def maybe_save_state!(%Trace{pid: pid, function: :delete_component, type: :call}) do
do_save_state!(pid)
end
Expand All @@ -31,4 +37,10 @@ defmodule LiveDebugger.Services.CallbackTracer.Actions.State do
:ok
end
end

defp do_save_initial_state!(pid, socket) do
StatesStorage.save!(%LvState{pid: pid, socket: socket})
Bus.broadcast_state!(%StateChanged{pid: pid}, pid)
:ok
end
end
61 changes: 43 additions & 18 deletions lib/live_debugger/services/garbage_collector/actions/garbage_collecting.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ defmodule LiveDebugger.Services.GarbageCollector.Actions.GarbageCollecting do

alias LiveDebugger.API.StatesStorage
alias LiveDebugger.API.TracesStorage
alias LiveDebugger.Services.GarbageCollector.GenServers.GarbageCollector

alias LiveDebugger.Bus
alias LiveDebugger.Services.GarbageCollector.Events.TableTrimmed
Expand All @@ -14,34 +15,51 @@ defmodule LiveDebugger.Services.GarbageCollector.Actions.GarbageCollecting do
@watched_table_size 50 * @megabyte_unit
@non_watched_table_size 5 * @megabyte_unit

@spec garbage_collect_traces!(MapSet.t(pid()), MapSet.t(pid())) :: boolean()
def garbage_collect_traces!(watched_pids, alive_pids) do
@doc """
Performs garbage collection on traces based on `to_remove`, `watched_pids`, and `alive_pids` sets.
Returns a set of PIDs marked for removal in next cycle.
"""
@spec garbage_collect_traces!(GarbageCollector.state(), MapSet.t(pid()), MapSet.t(pid())) ::
to_remove :: MapSet.t(pid())
def garbage_collect_traces!(%{to_remove: to_remove}, watched_pids, alive_pids) do
TracesStorage.get_all_tables()
|> Enum.reduce(false, fn {pid, table}, acc ->
|> Enum.map(fn {pid, table} ->
result =
cond do
MapSet.member?(watched_pids, pid) -> maybe_trim_traces_table!(table, :watched)
MapSet.member?(alive_pids, pid) -> maybe_trim_traces_table!(table, :non_watched)
true -> delete_traces_table!(table)
MapSet.member?(to_remove, pid) -> delete_traces_table!(table)
true -> :to_remove
end

acc or result
{pid, result}
end)
|> aggregate_results()
end

@spec garbage_collect_states!(MapSet.t(pid()), MapSet.t(pid())) :: boolean()
def garbage_collect_states!(watched_pids, alive_pids) do
@doc """
Performs garbage collection on states based on `to_remove`, `watched_pids`, and `alive_pids` sets.
Returns a set of PIDs marked for removal in next cycle.
"""
@spec garbage_collect_states!(GarbageCollector.state(), MapSet.t(pid()), MapSet.t(pid())) ::
to_remove :: MapSet.t(pid())
def garbage_collect_states!(%{to_remove: to_remove}, watched_pids, alive_pids) do
StatesStorage.get_all_states()
|> Enum.reduce(false, fn {pid, _}, acc ->
|> Enum.map(fn {pid, _} ->
result =
if MapSet.member?(watched_pids, pid) or MapSet.member?(alive_pids, pid) do
false
else
delete_state!(pid)
cond do
watched_or_alive?(pid, watched_pids, alive_pids) -> :keep
MapSet.member?(to_remove, pid) -> delete_state!(pid)
true -> :to_remove
end

acc or result
{pid, result}
end)
|> aggregate_results()
end

defp watched_or_alive?(pid, watched_pids, alive_pids) do
MapSet.member?(watched_pids, pid) or MapSet.member?(alive_pids, pid)
end

defp maybe_trim_traces_table!(table, type) when type in [:watched, :non_watched] do
Expand All @@ -51,22 +69,29 @@ defmodule LiveDebugger.Services.GarbageCollector.Actions.GarbageCollecting do
if size > max_size do
TracesStorage.trim_table!(table, max_size)
Bus.broadcast_event!(%TableTrimmed{})
true
else
false
end

:keep
end

defp delete_traces_table!(table) do
TracesStorage.delete_table!(table)
Bus.broadcast_event!(%TableDeleted{})
true
:removed
end

defp delete_state!(pid) do
StatesStorage.delete!(pid)
Bus.broadcast_event!(%TableTrimmed{})
true
:removed
end

defp aggregate_results(gc_result) do
gc_result
|> Enum.reduce(MapSet.new(), fn
{pid, :to_remove}, acc -> MapSet.put(acc, pid)
_, acc -> acc
end)
end

defp max_table_size(:watched), do: @watched_table_size
Expand Down
1 change: 0 additions & 1 deletion lib/live_debugger/services/garbage_collector/events.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ defmodule LiveDebugger.Services.GarbageCollector.Events do

use LiveDebugger.Event

defevent(GarbageCollected)
defevent(TableDeleted)
defevent(TableTrimmed)
end
27 changes: 11 additions & 16 deletions lib/live_debugger/services/garbage_collector/gen_servers/garbage_collector.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,15 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.GarbageCollector do
as: GarbageCollectingActions

alias LiveDebugger.Bus
alias LiveDebugger.Services.GarbageCollector.Events.GarbageCollected
alias LiveDebugger.App.Events.UserChangedSettings

@garbage_collect_interval 2000

@type state :: %{
garbage_collection_enabled?: boolean(),
to_remove: MapSet.t(pid())
}

def start_link(opts \\ []) do
GenServer.start_link(__MODULE__, opts, name: __MODULE__)
end
Expand All @@ -28,7 +32,8 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.GarbageCollector do

{:ok,
%{
garbage_collection_enabled?: SettingsStorage.get(:garbage_collection)
garbage_collection_enabled?: SettingsStorage.get(:garbage_collection),
to_remove: MapSet.new()
}}
end

Expand All @@ -37,39 +42,29 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.GarbageCollector do
watched_pids = TableWatcher.watched_pids()
alive_pids = TableWatcher.alive_pids()

traces_collected = GarbageCollectingActions.garbage_collect_traces!(watched_pids, alive_pids)
states_collected = GarbageCollectingActions.garbage_collect_states!(watched_pids, alive_pids)

if traces_collected or states_collected do
Bus.broadcast_event!(%GarbageCollected{})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we get rid of this event?

Copy link
Contributor Author

@hhubert6 hhubert6 Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't used anywhere and so when I refactored this part of code I didn't try to keep it if it's not even needed. We can always add it when there will be such a need.

end
to_remove1 = GarbageCollectingActions.garbage_collect_traces!(state, watched_pids, alive_pids)
to_remove2 = GarbageCollectingActions.garbage_collect_states!(state, watched_pids, alive_pids)

loop_garbage_collection()

{:noreply, state}
{:noreply, %{state | to_remove: MapSet.union(to_remove1, to_remove2)}}
end

# Handle messages related to ETS table transfers from TracesStorage
@impl true
def handle_info({:"ETS-TRANSFER", _ref, _from, _}, state) do
{:noreply, state}
end

@impl true
def handle_info(%UserChangedSettings{key: :garbage_collection, value: true}, state) do
resume_garbage_collection()
{:noreply, Map.put(state, :garbage_collection_enabled?, true)}
end

@impl true
def handle_info(%UserChangedSettings{key: :garbage_collection, value: false}, state) do
{:noreply, Map.put(state, :garbage_collection_enabled?, false)}
end

@impl true
def handle_info(_, state) do
{:noreply, state}
end
def handle_info(_, state), do: {:noreply, state}

defp loop_garbage_collection() do
Process.send_after(
Expand Down
20 changes: 5 additions & 15 deletions lib/live_debugger/services/garbage_collector/gen_servers/table_watcher.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.TableWatcher do

alias LiveDebugger.Bus
alias LiveDebugger.App.Events.DebuggerMounted
alias LiveDebugger.App.Events.DebuggerTerminated
alias LiveDebugger.Services.ProcessMonitor.Events.LiveViewBorn
alias LiveDebugger.Services.ProcessMonitor.Events.LiveViewDied

Expand Down Expand Up @@ -60,7 +59,6 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.TableWatcher do
{:reply, pids, state}
end

@impl true
def handle_call(:watched_pids, _, state) do
pids =
state
Expand All @@ -78,22 +76,21 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.TableWatcher do
|> noreply()
end

@impl true
def handle_info(%LiveViewDied{pid: pid}, state) when is_map_key(state, pid) do
state
|> update_live_view_died(pid)
|> noreply()
end

@impl true
def handle_info(%DebuggerMounted{debugged_pid: debugged_pid, debugger_pid: debugger_pid}, state) do
Process.monitor(debugger_pid)

state
|> add_watcher(debugged_pid, debugger_pid)
|> noreply()
end

@impl true
def handle_info(%DebuggerTerminated{debugger_pid: debugger_pid}, state) do
def handle_info({:DOWN, _ref, :process, debugger_pid, _reason}, state) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we exactly change it? Using event is more flexible because you have to monitor in a single place only

Copy link
Contributor Author

@hhubert6 hhubert6 Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DebuggerTerminated was sent in terminate callback but turns out for some reason it's not triggered every time. For example when returning from debugger to active LiveViews it sometimes wasn't called so the event wasn't sent and the state of table_watcher was wrong. Using Process.monitor and listening for :DOWN is more reliable in this case.

state
|> Enum.find(fn {_, %ProcessInfo{watchers: watchers}} ->
MapSet.member?(watchers, debugger_pid)
Expand All @@ -109,10 +106,7 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.TableWatcher do
end
end

@impl true
def handle_info(_, state) do
{:noreply, state}
end
def handle_info(_, state), do: {:noreply, state}

@spec update_live_view_died(state(), pid()) :: state()
defp update_live_view_died(state, pid) do
Expand All @@ -134,11 +128,7 @@ defmodule LiveDebugger.Services.GarbageCollector.GenServers.TableWatcher do
end

defp add_watcher(state, pid, watcher) do
if Process.alive?(pid) do
Map.put(state, pid, %ProcessInfo{watchers: MapSet.new([watcher])})
else
state
end
Map.put(state, pid, %ProcessInfo{alive?: Process.alive?(pid), watchers: MapSet.new([watcher])})
end

@spec remove_watcher(state(), pid(), pid()) :: state()
Expand Down
Loading