Skip to content

Remove Blazor internal profiling infrastructure #24468

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

Merged
merged 3 commits into from
Jul 31, 2020

Conversation

SteveSandersonMS
Copy link
Member

This removes the internal profiling infrastructure previously added in #23510, because:

  1. We now have a better way of capturing profiling data (modify the interpreter to capture the instruction counts at method entry/exit points)
  2. It was causing problems ([5.0.100-preview.8.20373.19] OpenMu plugins not displayed due to System.Reflection.ReflectionTypeLoadException #24339)

This PR also removes the JS-side profiling infrastructure, even though we don't have a similar replacement for it. I'm proposing to remove it because it's less useful (the JS-side code is already very tight, and the scenarios we're trying to optimize are now much heavier on the .NET side), it's not too hard to temporarily wire in a bit of timings capture code on an ad-hoc basis if you do want to measure something in JS, and it was quite a bit of extra JS code to be deploying into everyone's app which they couldn't even use.

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Jul 31, 2020
@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner July 31, 2020 12:38
@pranavkm
Copy link
Contributor

Could you do a quick check to see this fixes #24339? I was able to get this to throw by writing :Console.WriteLine(typeof(ComponentBase).Assembly.DefinedTypes) in one of our test projects.

@SteveSandersonMS
Copy link
Member Author

@pranavkm Yes, it does appear to fix it.

@SteveSandersonMS SteveSandersonMS merged commit 6f7a3df into master Jul 31, 2020
@SteveSandersonMS SteveSandersonMS deleted the stevesa/remove-profiling branch July 31, 2020 21:04
pranavkm pushed a commit that referenced this pull request Aug 5, 2020
* Put back InternalCalls

* Removing .NET profiling calls

* Remove JS side profiling
mkArtakMSFT pushed a commit that referenced this pull request Aug 5, 2020
* Put back InternalCalls

* Removing .NET profiling calls

* Remove JS side profiling

Co-authored-by: Steve Sanderson <[email protected]>
HaoK pushed a commit that referenced this pull request Aug 7, 2020
* Put back InternalCalls

* Removing .NET profiling calls

* Remove JS side profiling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[5.0.100-preview.8.20373.19] OpenMu plugins not displayed due to System.Reflection.ReflectionTypeLoadException
2 participants