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

[release/7.0-preview2][mono][hot reload] Fix sequence point logic for compiler-generated methods in updates #65867

Merged
merged 11 commits into from
Mar 2, 2022

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Feb 24, 2022

Backport of #65865 and #65973 to release/7.0-preview2

Summary:

This is about fixing the new hot reload capabilities in .NET 7 Preview 1 on platforms that use Mono (Blazor WebAssembly, mobile).

It includes:

  • If an update adds a compiler-generated method that doesn't have any sequence points, don't fall back to looking for sequence points in the baseline file - consider 0 sequence points as a success.
  • Also modify the hot reload tests to pass MONO_DEBUG=gen-seq-points to the runtime (via remote executor or build properties) so that we exercise this code even when we don't have a debugger attached.
  • In cases where we tell the interpreter to generate sequence points, but where the hot reload deltas don't include PDBs (basically dotnet watch) treat added methods as having zero sequence points. This is a follow-up to [mono][hot reload] Fix sequence point logic for compiler-generated methods in updates #65865 which actually makes it possible to add static lambdas.
  • Allow custom attribute deletions. In the case of nullability attributes, we get deletions (modifications that set the Parent to row 0) even if we don't declare a ChangeCustomAttribute capability. We intend to support custom attribute deletions in .NET 7, so this is fine.
  • Fix an off by one error where the last modified method in a delta was considered a method addition.

Related to #65808 and #51126

Testing:

new CI tests; manually using the new runtime together with dotnet watch from .NET 7 Preview 1

Risks:

Low. This is only for mono workloads; it does not impact existing hot reload capabilities (method body replacement), just the new capabilities to add lambdas and methods that lit up in .NET 7 Preview 1. In case of instability, customers using the next preview can go back to using dotnet watch to restart the blazor app instead of reloading in place.

-1 means there was some kind of problem

0 means 0 sequence points

In particular for mono_ppdb_get_seq_points_enc a compiler-generated method
might have 0 sequence points in the PDB delta, but we should consider that as
ok, and not fall back to looking in the baseline image
For platforms where we use the remote executor, set the environment variable
when starting the remote process.

For WASM, pass --setenv when building the project.
@ghost ghost assigned lambdageek Feb 24, 2022
@lambdageek lambdageek added area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc and removed area-VM-meta-mono labels Feb 24, 2022
@marek-safar marek-safar added this to the 7.0.0 milestone Feb 25, 2022
@lambdageek
Copy link
Member Author

I don't have permission to merge to the preview2 branch. Can someone with the right permissions click the squash button.

/cc @steveisok

@lambdageek

This comment was marked as outdated.

@lambdageek
Copy link
Member Author

CoreCLR win arm64 failure is #65818

@lambdageek lambdageek added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 25, 2022
@lambdageek
Copy link
Member Author

Needs more work. Something else is going wrong with a real blazor app

@lambdageek
Copy link
Member Author

lambdageek commented Feb 28, 2022

Updated the PR to also include all of #65973.

I'm not sure if there's still a window to land this in Preview 2 if I go to tactics. /cc @marek-safar

When hot reload is used with dotnet watch, the baseline PDBs are available, and
sequence points are enabled, but there are no PDB updates.
when counting added and modified rows
…tomAttributes capability

Roslyn seems to delete nullability annotations sometimes
@lambdageek lambdageek removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 28, 2022
@carlossanlop carlossanlop added the Servicing-consider Issue for next servicing release review label Mar 1, 2022
@marek-safar marek-safar added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 1, 2022
@lambdageek

This comment was marked as outdated.

@mmitche mmitche merged commit e24f66d into dotnet:release/7.0-preview2 Mar 2, 2022
@lambdageek lambdageek deleted the fix-gh-65808-preview2 branch March 19, 2022 16:45
@ghost ghost locked as resolved and limited conversation to collaborators Apr 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-EnC-mono Hot Reload for WebAssembly, iOS/Android, etc Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants