Skip to content

Blazor WebAssembly internal profiling infrastructure #23510

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 13 commits into from
Jul 1, 2020

Conversation

SteveSandersonMS
Copy link
Member

@SteveSandersonMS SteveSandersonMS commented Jun 30, 2020

Goals:

  • Ability to capture timings for both the .NET and JS sides of execution, all in a single timeline
  • Ability to vary how finely-grained is the timing info we capture
    • By default, with this PR we capture at a coarse-grained level. However it's possible to get far more detailed timings for rendertreebuilder and rendertreediffbuilder by passing extra compiler flags. Developers working on the sources in this repo can easily add more detailed timings for any other part of the code.
  • Minimal impact on perf
    • When not capturing at all, should be really close to zero even in the most intensive rendering scenarios. This is achieved by (1) skipping fine-grained capture calls via [Conditional], and (2) no-opping the remaining calls quickly based on a static bool flag indicating whether capturing is currently in progress.
    • When capturing, the per-capture overhead needs to be as minimal as possible to avoid distorting the timings. However it's impossible to measure without having some impact. This is achieved by using the most low-level form of JS interop and avoiding even decoding the string parameters during timings capture. We absolutely can't record actual timing data on the .NET side, because the act of reading the clock is very expensive relative to executing the code we're measuring.
  • No new public API right now, because in the short term it's more important to make progress on perf optimization than to get hung up designing the APIs and planning for what kinds of support is needed in the future. We can upgrade aspects of this to be public APIs in the future if we want.
    • This is achieved by inlining the capturing code directly into Microsoft.AspNetCore.Components, even though it violates the normal layering. I consider this an acceptable layering violation because it's completely nonpublic and won't have any impact on any other code.
    • I made ComponentsProfiling abstract because, even though there is a runtime cost to that, it's actually too small to be problematic even when capturing very detailed traces, and is completely undetectable when not capturing or when capturing coarsely-grained traces.
  • Ability to upgrade to a public API in the future.
    • This would be achieved by making ComponentsProfiling public and then moving WebAssemblyComponentsProfiling (still internal) into the WebAssembly project. We'd need some CircuitComponentsProfiling equivalent, though presumably it wouldn't do any JS interop and would instead be forwarding the calls to .NET's EventSource or similar.
  • Ability to capture at least basic coarse-grained traces from any Blazor WebAssembly app running in production, so we can't compile it all out entirely from the shipping packages
  • Integration with a widely-available perf trace inspection GUI.
    • The generated traces can be loaded into Chromium's dev tools for inspection.

@SteveSandersonMS SteveSandersonMS added area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly labels Jun 30, 2020
@SteveSandersonMS SteveSandersonMS requested a review from a team June 30, 2020 14:23
if (IsCapturing)
{
InternalCalls.InvokeJSUnmarshalled<string, object, object, object>(
out _, "_blazorProfileStart", name!, null!, null!);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we do null! here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nullability hints on InvokeJSUnmarshalled say that the parameters have to be nonnull, but that's incorrect - they can be null (as they are here).

Thanks for pointing this out. I've (hopefully) fixed the underlying hints now and changed these to regular null.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@@ -25,14 +28,17 @@ enum DiffAction { Match, Insert, Delete }
ArrayRange<RenderTreeFrame> oldTree,
ArrayRange<RenderTreeFrame> newTree)
{
ComponentsProfiling.Instance.Start();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use ProfilingStart/ProfilingEnd here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I wanted to include this method in the coarse-grained timings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha! OK -- I see how this call is different now.

@SteveSandersonMS SteveSandersonMS requested a review from a team June 30, 2020 15:22
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I like the approach of setting the isCapturing flag from the host's JavaScript. Should be easy to strip out once we support perf tracing in both hosting models.

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 feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants