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

Blazor hasDebuggingEnabled set when "Start without debugging" in VS #24623

Closed
lewing opened this issue Aug 6, 2020 · 9 comments
Closed

Blazor hasDebuggingEnabled set when "Start without debugging" in VS #24623

lewing opened this issue Aug 6, 2020 · 9 comments
Assignees
Labels
affected-all This issue impacts all the customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-debugging This issue is related to debugging of Blazor WebAssembly apps feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly severity-nice-to-have This label is used by an internal tool
Milestone

Comments

@lewing
Copy link
Member

lewing commented Aug 6, 2020

When blazor is started from inside VS with Start without debugging hasDebuggingEnabled is true and load_runtime is passed non zero debug level. This disables most interpreter optimizations (because they would break debugging) leading to substantially slower execution speed (dotnet/runtime#40428). Is this the intended behavior? If not can it be fixed, and if so can it be documented?

https://github.com/dotnet/AspNetCore/blob/master/src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts#L367

@lewing lewing added feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-debugging This issue is related to debugging of Blazor WebAssembly apps labels Aug 6, 2020
@lewing lewing changed the title Blazor hasDebuggingEnabled set when "Run without debugging" Blazor hasDebuggingEnabled set when "Start without debugging" in VS Aug 6, 2020
@Pilchie Pilchie added the area-blazor Includes: Blazor, Razor Components label Aug 6, 2020
@captainsafia
Copy link
Member

Is this the intended behavior?

Nope.

If not can it be fixed,

We can try to fix it. We had briefly discussed this class of issues (passing configurations from the IDE down to the debugger, such as start without debugging) but hadn't talked about the specifics.

The JS debugger sets a noDebug flag in the config. We might be able to figure out a way to read that from the client and configure the MonoProxy correctly.

and if so can it be documented?

@guardrex Let's add a note in the debugging section to let users know that "Start without debugging" doesn't work with Blazor WASM on VS.

@mkArtakMSFT This issue would be a good candidate for RC1 as a perf improvement.

@captainsafia
Copy link
Member

captainsafia commented Aug 14, 2020

It looks like our best option here is to implement a middleware in the debug proxy that checks to see if there is a chrome process with the remote-debugging-port set. I landed on this solution after spiking into a couple of different approaches:

  • We can't rely on any client-side checks, including checking navigator.webdriver, because pretty soon that will not be set when debugging is enabled (ref).
  • You can figure out if a Chrome browser is running with debugging enabled by checking the command line arguments listed at chrome://version. Of course, there is no way to check this from the JavaScript on a webpage.
  • We can try sending a request to http://localhost:9222/json/version and seeing if we get a response. However, there's no way for us to know ahead of time what debugging port (e.g. 9222) is being used by Chrome/Edge, especially if Chrome is launched with any available port (e.g. remote-debugging-port=0).
  • We could set a flag from the IDE (VS/VS Code) when Start without debugging is used. However, this change is a little bit more high cost than the timeline we have here. Also, it makes the change less portable across different scenarios.

So, debug proxy + PID scan strikes the balance between being the most portable and requiring us to know the least ahead of time (e.g. what debugging port is used). This approach should also work on remote scenarios without requiring port forwarding/proxying.

Some things to be mindful of with this approach:

  • Any Chrome browser running with remote-debugging-enabled on the machine will cause us to enable debugging in the browser (e.g. false positives). This should be rare though since we can assume that most of the time the user is starting the debugging session from the IDE.

@captainsafia
Copy link
Member

Orrrrrrrrrrr....alternatively as discussed during today's Blazor engineering sync we can have the user explicitly set whether or not they want to debug via a query parameter.

  1. User starts debugging.
  2. User navigates tohttps://localhost:5001?debug=true.
  3. We parse the query params and set the flag accordingly.

VS Code and VS will need to update the launch URL of the application accordingly.

@ghost ghost added Done This issue has been fixed and removed Working labels Aug 18, 2020
NTaylorMullen added a commit to dotnet/vscode-csharp that referenced this issue Aug 18, 2020
- Updated changelog & testplan as well.

This includes:
  * Fully qualify component light bulb ([dotnet/aspnetcore-tooling#22309](https://github.com/dotnet/aspnetcore/issues/22309))
  * Add using for component light bulb ([dotnet/aspnetcore-tooling#22308](https://github.com/dotnet/aspnetcore/issues/22308))
  * Create component from tag light bulb ([dotnet/aspnetcore-tooling#22307](https://github.com/dotnet/aspnetcore/issues/22307))
  * Go to definition on Blazor components ([dotnet/aspnetcore-tooling#17044](https://github.com/dotnet/aspnetcore/issues/17044))
  * Rename Blazor components ([dotnet/aspnetcore-tooling#22312](https://github.com/dotnet/aspnetcore/issues/22312))
  * Prepare Blazor debugging to have better support for "Start without debugging" scenarios ([dotnet/aspnetcore-tooling#24623](dotnet/aspnetcore#24623))
NTaylorMullen added a commit to dotnet/vscode-csharp that referenced this issue Aug 18, 2020
- Updated changelog & testplan as well.

This includes:
  * Fully qualify component light bulb ([dotnet/aspnetcore-tooling#22309](https://github.com/dotnet/aspnetcore/issues/22309))
  * Add using for component light bulb ([dotnet/aspnetcore-tooling#22308](https://github.com/dotnet/aspnetcore/issues/22308))
  * Create component from tag light bulb ([dotnet/aspnetcore-tooling#22307](https://github.com/dotnet/aspnetcore/issues/22307))
  * Go to definition on Blazor components ([dotnet/aspnetcore-tooling#17044](https://github.com/dotnet/aspnetcore/issues/17044))
  * Rename Blazor components ([dotnet/aspnetcore-tooling#22312](https://github.com/dotnet/aspnetcore/issues/22312))
  * Prepare Blazor debugging to have better support for "Start without debugging" scenarios ([dotnet/aspnetcore-tooling#24623](dotnet/aspnetcore#24623))
JoeRobich pushed a commit to dotnet/vscode-csharp that referenced this issue Aug 18, 2020
- Updated changelog & testplan as well.

This includes:
  * Fully qualify component light bulb ([dotnet/aspnetcore-tooling#22309](https://github.com/dotnet/aspnetcore/issues/22309))
  * Add using for component light bulb ([dotnet/aspnetcore-tooling#22308](https://github.com/dotnet/aspnetcore/issues/22308))
  * Create component from tag light bulb ([dotnet/aspnetcore-tooling#22307](https://github.com/dotnet/aspnetcore/issues/22307))
  * Go to definition on Blazor components ([dotnet/aspnetcore-tooling#17044](https://github.com/dotnet/aspnetcore/issues/17044))
  * Rename Blazor components ([dotnet/aspnetcore-tooling#22312](https://github.com/dotnet/aspnetcore/issues/22312))
  * Prepare Blazor debugging to have better support for "Start without debugging" scenarios ([dotnet/aspnetcore-tooling#24623](dotnet/aspnetcore#24623))
@captainsafia
Copy link
Member

We reverted this change and will put in a more robust fix for 6.0 so re-opening.

@captainsafia captainsafia reopened this Sep 1, 2020
@captainsafia captainsafia removed the Done This issue has been fixed label Sep 1, 2020
@captainsafia captainsafia removed this from the 5.0.0-rc1 milestone Sep 1, 2020
@guardrex
Copy link
Contributor

guardrex commented Sep 1, 2020

@captainsafia ... Should I close the docs issue 👉 dotnet/AspNetCore.Docs#19545?

@captainsafia
Copy link
Member

@guardrex Yes.

@captainsafia captainsafia added this to the Next sprint planning milestone Sep 2, 2020
@ghost
Copy link

ghost commented Sep 2, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Sep 16, 2020
@SteveSandersonMS SteveSandersonMS added affected-all This issue impacts all the customers severity-nice-to-have This label is used by an internal tool labels Oct 7, 2020 — with ASP.NET Core Issue Ranking
@ghost
Copy link

ghost commented Oct 9, 2020

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@pranavkm
Copy link
Contributor

@lewing given this is a dev scenario and fixing it seems tedious at best, I'm going to mark this as won't fix. Let me know if you feel very strongly about fixing it.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-all This issue impacts all the customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-debugging This issue is related to debugging of Blazor WebAssembly apps feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

7 participants