Skip to content

Fix perf regression in testing for RPC marshalable objects #1329

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.

Sign up for GitHub

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
AArnott merged 2 commits into from
Oct 21, 2025
Merged

Fix perf regression in testing for RPC marshalable objects #1329

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8455a3f
Cache the result of queries for `RpcMarshalableAttribute`
AArnott Oct 21, 2025
9d304c2
Correct regression in early out logic
AArnott Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions src/StreamJsonRpc/Reflection/MessageFormatterRpcMarshaledContextTracker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ internal partial class MessageFormatterRpcMarshaledContextTracker
private static readonly ConcurrentDictionary<Type, (JsonRpcProxyOptions? ProxyOptions, JsonRpcTargetOptions TargetOptions, RpcMarshalableAttribute Attribute)> MarshaledTypes = new();
private static readonly JsonRpcTargetOptions RpcMarshalableInterfaceDefaultTargetOptions = new() { NotifyClientOfEvents = false, DisposeOnDisconnect = true };
private static readonly MethodInfo ReleaseMarshaledObjectMethodInfo = typeof(MessageFormatterRpcMarshaledContextTracker).GetMethod(nameof(ReleaseMarshaledObject), BindingFlags.NonPublic | BindingFlags.Instance)!;
private static readonly ConcurrentDictionary<Type, RpcMarshalableOptionalInterfaceAttribute[]> MarshalableOptionalInterfaces = new ConcurrentDictionary<Type, RpcMarshalableOptionalInterfaceAttribute[]>();
private static readonly ConcurrentDictionary<Type, RpcMarshalableOptionalInterfaceAttribute[]> MarshalableOptionalInterfaces = new();
private static readonly ConcurrentDictionary<Type, RpcMarshalableAttribute?> RpcMarshalableAttributeCache = new();

private readonly Dictionary<long, (RpcMarshaledContext Context, IDisposable Revert)> marshaledObjects = new Dictionary<long, (RpcMarshaledContext Context, IDisposable Revert)>();
private readonly JsonRpc jsonRpc;
Expand Down Expand Up @@ -106,12 +107,13 @@ internal static bool TryGetMarshalOptionsForType<T>(
[NotNullWhen(true)] out JsonRpcTargetOptions? targetOptions,
[NotNullWhen(true)] out RpcMarshalableAttribute? rpcMarshalableAttribute)
{
if (TryGetMarshalOptionsForTypeHelper(typeShape.Type, defaultProxyOptions, out proxyOptions, out targetOptions, out rpcMarshalableAttribute))
bool? helperResult = TryGetMarshalOptionsForTypeHelper(typeShape.Type, defaultProxyOptions, out proxyOptions, out targetOptions, out rpcMarshalableAttribute);
if (helperResult is not null)
{
return true;
return helperResult.Value;
}

if (typeShape.Type.GetCustomAttribute<RpcMarshalableAttribute>() is RpcMarshalableAttribute marshalableAttribute)
if (TryGetRpcMarshalableAttribute(typeShape.Type, out RpcMarshalableAttribute? marshalableAttribute))
{
// Validation requires more trim annotations than our NativeAOT callers can provide.
// And besides, analyzers should have called out any issues at compile-time.
Expand All @@ -137,19 +139,23 @@ internal static bool TryGetMarshalOptionsForType(
[NotNullWhen(true)] out JsonRpcTargetOptions? targetOptions,
[NotNullWhen(true)] out RpcMarshalableAttribute? rpcMarshalableAttribute)
{
if (TryGetMarshalOptionsForTypeHelper(type, defaultProxyOptions, out proxyOptions, out targetOptions, out rpcMarshalableAttribute))
bool? helperResult = TryGetMarshalOptionsForTypeHelper(type, defaultProxyOptions, out proxyOptions, out targetOptions, out rpcMarshalableAttribute);
if (helperResult is not null)
{
// Because events are not checked by the Nerdbank.MessagePack formatter,
// Remove this check when issues related to https://github.com/eiriktsarpalis/PolyType/issues/226 are resolved in this file.
if (type.GetEvents().Length > 0)
if (helperResult is true)
Comment thread
AArnott marked this conversation as resolved.
{
throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, Resources.MarshalableInterfaceHasEvents, type.FullName));
// Because events are not checked by the Nerdbank.MessagePack formatter,
// Remove this check when issues related to https://github.com/eiriktsarpalis/PolyType/issues/226 are resolved in this file.
if (type.GetEvents().Length > 0)
{
throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, Resources.MarshalableInterfaceHasEvents, type.FullName));
}
}

return true;
return helperResult.Value;
}

if (type.GetCustomAttribute<RpcMarshalableAttribute>() is RpcMarshalableAttribute marshalableAttribute)
if (TryGetRpcMarshalableAttribute(type, out RpcMarshalableAttribute? marshalableAttribute))
{
// Validation requires more trim annotations than our NativeAOT callers can provide.
// And besides, analyzers should have called out any issues at compile-time.
Expand Down Expand Up @@ -199,7 +205,7 @@ internal static RpcMarshalableOptionalInterfaceAttribute[] GetMarshalableOptiona

foreach (RpcMarshalableOptionalInterfaceAttribute attribute in attributes)
{
if (attribute.OptionalInterface.GetCustomAttribute<RpcMarshalableAttribute>() is null)
if (!TryGetRpcMarshalableAttribute(attribute.OptionalInterface, out _))
{
throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, Resources.RpcMarshalableOptionalInterfaceMustBeMarshalable, attribute.OptionalInterface.FullName));
}
Expand Down Expand Up @@ -441,7 +447,7 @@ internal MarshalToken GetToken(
}
}

private static bool TryGetMarshalOptionsForTypeHelper(
private static bool? TryGetMarshalOptionsForTypeHelper(
Type type,
JsonRpcProxyOptions defaultProxyOptions,
[NotNullWhen(true)] out JsonRpcProxyOptions? proxyOptions,
Expand All @@ -453,6 +459,7 @@ private static bool TryGetMarshalOptionsForTypeHelper(
rpcMarshalableAttribute = null;
if (type.IsInterface is false)
{
// Definitely not.
return false;
}

Expand All @@ -479,7 +486,8 @@ private static bool TryGetMarshalOptionsForTypeHelper(
}
}

return false;
// Unknown. Caller may want to do more work.
return null;
}

/// <summary>
Expand Down Expand Up @@ -545,6 +553,12 @@ private static void ValidateMarshalableInterface(
}
#endif

private static bool TryGetRpcMarshalableAttribute(Type type, [NotNullWhen(true)] out RpcMarshalableAttribute? attribute)
{
attribute = RpcMarshalableAttributeCache.GetOrAdd(type, static type => type.GetCustomAttribute<RpcMarshalableAttribute>());
return attribute is not null;
}

/// <summary>
/// Releases memory associated with marshaled objects.
/// </summary>
Expand Down