Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors callback tracing initialization to avoid fetching/processing the module list multiple times by reusing a single “live modules + compiled paths” query for both trace-pattern setup and file-system monitoring.
Changes:
- Removed
Paths.compiled_modules_directories/0and its dedicated test; moved “live modules + compiled path” discovery intoCallbacks. - Added
Callbacks.all_live_modules_with_paths/0and list-basedCallbacks.all_callbacks/1to reuse module lists efficiently. - Updated tracing initialization to
setup_tracing_with_monitoring!/1, applying trace patterns and starting the FileSystem monitor in one pass; adjusted tests accordingly.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
test/services/callback_tracer/queries/paths_test.exs |
Removed tests for the deleted Paths query module. |
test/services/callback_tracer/queries/callbacks_test.exs |
Updated Mox stubs to include live_module?/1 due to new filtering path. |
test/services/callback_tracer/gen_servers/tracing_manager_test.exs |
Updated init/setup expectations to match new monitoring startup location. |
test/services/callback_tracer/actions/tracing_test.exs |
Renamed/updated setup tests and added coverage for multi-directory monitoring during setup. |
lib/live_debugger/services/callback_tracer/queries/paths.ex |
Deleted; functionality superseded by Callbacks.all_live_modules_with_paths/0. |
lib/live_debugger/services/callback_tracer/queries/callbacks.ex |
Added live-module/path query + overloads for callback building from a provided module list. |
lib/live_debugger/services/callback_tracer/gen_servers/tracing_manager.ex |
Switched to setup_tracing_with_monitoring!/1; removed monitoring call from init/1. |
lib/live_debugger/services/callback_tracer/actions/tracing.ex |
Combined tracing setup + file monitoring; trace-pattern application now accepts pre-fetched modules. |
.gitignore |
Ignored shell.nix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kraleppa
added a commit
that referenced
this pull request
Feb 26, 2026
* Moved recompilation monitoring setup after tracing setup * Add nix to gitignore * Fix tests * Enhanced performance of LiveDebugger initialization * Mix format * Don't duplicate monitor process
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Basically the problem was that we made some heavy file system operations on TracingManager GenServer init, which caused a significant lag. Apparently, we were doing some operations two times (once when setting up tracing, secondly when setting up recompilation monitoring).
It was fine for MacOS but on nixOS it started to cause some issues
I unified those two functions and moved it out from GenServer init, so now setup should not be that long