[Blazor] Use JSON source generator during WebAssembly startup #54956
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Text.Json; | ||
| using System.Text.Json.Serialization; | ||
| using System.Text.Json.Serialization.Metadata; | ||
|
|
||
| namespace Microsoft.AspNetCore.Components; | ||
|
|
||
| // For custom converters that don't rely on serializing an object graph, | ||
| // we can resolve the incoming type's JsonTypeInfo directly from the converter. | ||
| // This skips extra work to collect metadata for the type that won't be used. | ||
| internal sealed class JsonConverterFactoryTypeInfoResolver<T> : IJsonTypeInfoResolver | ||
| { | ||
| public static readonly JsonConverterFactoryTypeInfoResolver<T> Instance = new(); | ||
|
|
||
| private JsonConverterFactoryTypeInfoResolver() | ||
| { | ||
| } | ||
|
|
||
| public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) | ||
| { | ||
| if (type != typeof(T)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| foreach (var converter in options.Converters) | ||
| { | ||
| if (converter is not JsonConverterFactory factory || !factory.CanConvert(type)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (factory.CreateConverter(type, options) is not { } converterToUse) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| return JsonMetadataServices.CreateValueInfo<T>(options, converterToUse); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.ComponentModel; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using Microsoft.JSInterop; | ||
|
|
||
| namespace Microsoft.AspNetCore.Components.Web.Internal; | ||
|
|
||
| /// <summary> | ||
| /// For internal framework use only. | ||
| /// </summary> | ||
| [EditorBrowsable(EditorBrowsableState.Never)] | ||
| public interface IInternalWebJSInProcessRuntime | ||
| { | ||
| /// <summary> | ||
| /// For internal framework use only. | ||
| /// </summary> | ||
| string InvokeJS(string identifier, [StringSyntax(StringSyntaxAttribute.Json)] string? argsJson, JSCallResultType resultType, long targetInstanceId); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| #nullable enable | ||
| Microsoft.AspNetCore.Components.Web.Internal.IInternalWebJSInProcessRuntime | ||
| Microsoft.AspNetCore.Components.Web.Internal.IInternalWebJSInProcessRuntime.InvokeJS(string! identifier, string? argsJson, Microsoft.JSInterop.JSCallResultType resultType, long targetInstanceId) -> string! | ||
| Microsoft.AspNetCore.Components.Web.KeyboardEventArgs.IsComposing.get -> bool | ||
| Microsoft.AspNetCore.Components.Web.KeyboardEventArgs.IsComposing.set -> void |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,10 @@ | |
|
|
||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Text.Json; | ||
| using System.Text.Json.Serialization; | ||
| using Microsoft.AspNetCore.Components.Web; | ||
| using Microsoft.AspNetCore.Components.Web.Infrastructure; | ||
| using Microsoft.AspNetCore.Components.Web.Internal; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Logging; | ||
| using Microsoft.JSInterop; | ||
|
|
@@ -41,12 +43,7 @@ public WebRenderer( | |
| // Supply a DotNetObjectReference to JS that it can use to call us back for events etc. | ||
| jsComponentInterop.AttachToRenderer(this); | ||
| var jsRuntime = serviceProvider.GetRequiredService<IJSRuntime>(); | ||
| AttachWebRendererInterop(jsRuntime, jsonOptions, jsComponentInterop); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -103,6 +100,44 @@ protected override void Dispose(bool disposing) | |
| base.Dispose(disposing); | ||
| } | ||
|
|
||
| [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "<Pending>")] | ||
| private void AttachWebRendererInterop(IJSRuntime jsRuntime, JsonSerializerOptions jsonOptions, JSComponentInterop jsComponentInterop) | ||
| { | ||
| const string JSMethodIdentifier = "Blazor._internal.attachWebRendererInterop"; | ||
|
|
||
| // These arguments should be kept in sync with WebRendererSerializerContext | ||
| object[] args = [ | ||
| _rendererId, | ||
| _interopMethodsReference, | ||
| jsComponentInterop.Configuration.JSComponentParametersByIdentifier, | ||
| jsComponentInterop.Configuration.JSComponentIdentifiersByInitializer, | ||
| ]; | ||
|
|
||
| if (jsRuntime is IInternalWebJSInProcessRuntime inProcessRuntime) | ||
| { | ||
| // Fast path for WebAssembly: Rather than using the JSRuntime to serialize | ||
| // parameters, we utilize the source-generated WebRendererSerializerContext | ||
| // for a faster JsonTypeInfo resolution. | ||
|
|
||
| // We resolve a JsonTypeInfo for DotNetObjectReference<WebRendererInteropMethods> from | ||
| // the JS runtime's JsonConverters. This is because adding DotNetObjectReference<T> as | ||
| // a supported type in the JsonSerializerContext generates unnecessary code to produce | ||
| // JsonTypeInfo for all the types referenced by both DotNetObjectReference<T> and its | ||
| // generic type argument. | ||
|
|
||
| var newJsonOptions = new JsonSerializerOptions(jsonOptions); | ||
|
There was a problem hiding this comment. I had another commit that eliminated this extra |
||
| newJsonOptions.TypeInfoResolverChain.Clear(); | ||
| newJsonOptions.TypeInfoResolverChain.Add(WebRendererSerializerContext.Default); | ||
| newJsonOptions.TypeInfoResolverChain.Add(JsonConverterFactoryTypeInfoResolver<DotNetObjectReference<WebRendererInteropMethods>>.Instance); | ||
| var argsJson = JsonSerializer.Serialize(args, newJsonOptions); | ||
| inProcessRuntime.InvokeJS(JSMethodIdentifier, argsJson, JSCallResultType.JSVoidResult, 0); | ||
|
There was a problem hiding this comment. If we were to avoid using the SG for this call, how big is the performance penalty? This is a single call per app, and we are creating a bunch of interfaces to support it. Might not be worth the extra infrastructure just for this. Alternatively, the renderer could expose a virtual Either way it's not a big deal IMO. If there is not a big penalty for avoiding this call through regular means, I would do that. If we benefit significantly, I will keep it and probably switch to use the virtual method instead of the extra interface, but its not a big deal eitherway. There was a problem hiding this comment.
Assuming that the goal of this change is to improve startup costs, it shouldn't matter how many times a particular call is made. There was a problem hiding this comment. It's about the trade-off of having an extra interface and coupling vs saving a couple MS. There was a problem hiding this comment. Saving a couple of MS sounds like a desirable goal assuming that it's a cost incurred by default for every wasm app. There was a problem hiding this comment. That couple-of-ms happens client-side as part of a several-hundred-ms startup process. It doesn't impact server CPU usage and hence mostly matters only to the extent that humans can notice it. So realistically it wouldn't be top of the queue of things for us to optimize. There was a problem hiding this comment.
Locally, I just measured about an 80ms difference on published builds. Not sure exactly how much that difference changes on slower machines, but it seems significant. To put that number into perspective, total startup blocking time with that optimization is (locally) ~330ms.
We could, it would just mean that if we wanted to do a similar optimization in another area of the framework, we'd have to add additional specialized API (I count 9 other calls to Is it fine if we proceed with this for now and discuss it further in API review? I think it's an early enough preview where we could change the API later if needed, especially since it's annotated as There was a problem hiding this comment. Fair enough, I leave it up to you. |
||
| } | ||
| else | ||
| { | ||
| jsRuntime.InvokeVoidAsync(JSMethodIdentifier, args).Preserve(); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// A collection of JS invokable methods that the JS-side code can use when it needs to | ||
| /// make calls in the context of a particular renderer. This object is never exposed to | ||
|
|
@@ -145,3 +180,11 @@ public void RemoveRootComponent(int componentId) | |
| => _jsComponentInterop.RemoveRootComponent(componentId); | ||
| } | ||
| } | ||
|
|
||
| // This should be kept in sync with the argument types in the call to | ||
| // 'Blazor._internal.attachWebRendererInterop' | ||
| [JsonSerializable(typeof(object[]))] | ||
| [JsonSerializable(typeof(int))] | ||
| [JsonSerializable(typeof(Dictionary<string, JSComponentConfigurationStore.JSComponentParameter[]>))] | ||
| [JsonSerializable(typeof(Dictionary<string, List<string>>))] | ||
| internal sealed partial class WebRendererSerializerContext : JsonSerializerContext; | ||
|
There was a problem hiding this comment. I'm curious about how you think we should maintain this in the future. How would a developer know when/what to add to this list of types? Am I right to think that if they start serializing something different and fail to add it here, everything would still work but would just be slower? Or would some error occur so they know to change this code? There was a problem hiding this comment. Along the same lines, how did you even know to include these specific types? I'm hoping it's because if you don't, there's a clear error indicating they need to be included! There was a problem hiding this comment.
These types match the types of the arguments passed to the JS interop call. Failing to do this results in a runtime error. However, I've just pushed a change that makes it even clearer where these types come from, so hopefully that eliminates any confusion. If we end up reverting that change, I can add a comment explaining how these types should be specified. |
||
Uh oh!
There was an error while loading. Please reload this page.