Skip to content
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.

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

Feature?: Only Interpret Required Files & Modules With Breakpoints #615

Closed
jeremyjh opened this issue Oct 29, 2021 · 5 comments · Fixed by #616
Closed

Feature?: Only Interpret Required Files & Modules With Breakpoints #615

jeremyjh opened this issue Oct 29, 2021 · 5 comments · Fixed by #616

Comments

@jeremyjh
Copy link

I assume this is a feature request, as I don't see a documented way to do it although that is a bit surprising to me. I do not want all my modules and especially all my dependencies to be interpreted modules in the debugger. Instead, I would like to interpret only modules/files I have specified in requiredFiles. Ideally any file with a breakpoint would also be automatically interpreted.

I'd like to use the debugger with VS Code in my project but I can't because it takes 5+ minutes to run a single test with everything interpreted. The same test will run in less than a second in a normal process.

If it is a feature, I'm willing to work on it unless there is some reason this is not a good idea or not practical.

axelson added a commit to axelson/elixir-ls that referenced this issue Oct 30, 2021
On larger projects, the overhead of interpreting all the modules in the
project (including all dependencies) causes an unbearable slowdown.

This commit makes it possible to disable the auto interpreting of
modules and instead rely on files that are in `requireFiles` (which are
interpreted)

Fixes elixir-lsp#615
@axelson
Copy link
Member

axelson commented Oct 30, 2021

@jeremyjh 👋 Thanks for the feature request!

Indeed that does make sense. Since modules that are found in requireFiles are already being interpreted I've implemented this feature as a new autoInterpretFiles configuration. If that that configuration is false (it defaults to true), then the ElixirLS debugger doesn't interpret all files, only those that are in requireFiles or have a breakpoint set on them. Can you please test it out?

#616

Here's an example launch configuration:
Screen Shot 2021-10-29 08-17-43

Note: Any modules that don't have a breakpoint and are not in requireFiles cannot be stepped into while debugging.

However I think with some more work and effort we could support stepping into arbitrary modules while debugging by analyzing what modules are referenced from the module that we are currently in.

@jeremyjh
Copy link
Author

@axelson Wow, thanks for the quick response!

I tried this out and the new flag definitely works in terms of not automatically interpreting everything; now debug tasks can run very fast.

However I was not successful in adding a module of my program to requireFiles; I think the difference is the module has already been compiled and loaded in the VM, this is the error output:

10:03:26.798 [error] Module 'Elixir.Project.ModuleName' must be purged before deleting

(Debugger) Initialization failed because an exception was raised:
    ** (MatchError) no match of right hand side value: false
        (elixir_ls_debugger 0.8.1) lib/debugger/server.ex:873: ElixirLS.Debugger.Server.save_and_reload/2
        (elixir_ls_debugger 0.8.1) lib/debugger/server.ex:868: anonymous fn/2 in ElixirLS.Debugger.Server.require_files/1
        (elixir 1.11.2) lib/enum.ex:2181: Enum."-reduce/3-lists^foldl/2-0-"/3
        (elixir_ls_debugger 0.8.1) lib/debugger/server.ex:863: ElixirLS.Debugger.Server.require_files/1
        (elixir_ls_debugger 0.8.1) lib/debugger/server.ex:722: ElixirLS.Debugger.Server.initialize/1

I'm just adding it like this:

"requireFiles": [
        "test/**/test_helper.exs",
        "lib/project/module_name.ex",
        "${relativeFile}"
      ]

@jeremyjh
Copy link
Author

However I think with some more work and effort we could support stepping into arbitrary modules while debugging by analyzing what modules are referenced from the module that we are currently in.

I think this would also need to be guarded by a flag as well as I think its possible that instrumenting every possible call graph from a module could lead to significant slow-downs. It would be really nice though if we could at least auto-instrument every module that has a breakpoint in it; for most of my uses that would be all I need.

@axelson
Copy link
Member

axelson commented Nov 1, 2021

However I was not successful in adding a module of my program to requireFiles; I think the difference is the module has already been compiled and loaded in the VM, this is the error output:

Ah, I see. Yes that is easy to reproduce. Thinking about this some more we probably shouldn't go through requireFiles for files that are already compiled. What I'm wrestling with now is that we need the module names in order to interpret the module. I was originally thinking that the modules should be provided based on a file path, but after thinking about it I think we should specify them by a regex on the module name. Does this interface work well for you?

So I have added a interpretModulesPatterns configuration. You'd set it in your launch.json with something like:

"interpretModulesPatterns": [
  "MyApp.*"
]

This would interpret all the modules in your application. Of course you use a more restrictive pattern if you only wanted to interpret some of the modules.

Can you pull the code and try again?

@jeremyjh
Copy link
Author

jeremyjh commented Nov 1, 2021

@axelson Brilliant! This is a much more flexible design, I really like it. It works great for me.

lukaszsamson pushed a commit that referenced this issue Dec 13, 2021
* Debugger: Allow disabling auto interpreting

On larger projects, the overhead of interpreting all the modules in the
project (including all dependencies) causes an unbearable slowdown.

This commit makes it possible to disable the auto interpreting of
modules and instead rely on files that are in `requireFiles` (which are
interpreted)

Fixes #615

* Add interpretModulesPatterns

* Fix formatting

* Update variable naming

* Add try catch when interpretting by pattern

Also improve some naming

* Fix error

* Fix formatting
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 a pull request may close this issue.

2 participants