Skip to content

Only root infrastructure and dependencies for IDynamicInterfaceCastable when IDIC is implemented #116660

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

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

jkoritzinsky
Copy link
Member

Implement a mechanism to conditionally root [RuntimeExport] methods based on the type corresponding to their feature being used. Use this mechanism to only root IDynamicInterfaceCastable logic when someone implements IDynamicInterfaceCastable and provide "default" implementations that should never be called in the bootstrapper when IDIC is unused.

Additionally, enhance the ScannedDevirtualizationManager to make two observations that we can optimize around:

  • Note when IDynamicInterfaceCastable has any implementors
  • If a [DynamicInterfaceCastableImplementation] type is rooted but IDynamicInterfaceCastable is not, the interfaces implemented by the attributed type still can have a known set of implementors.

Finally, adjust the "what type handle to use for casting" logic to only root a MaximallyConstructableType symbol when the target type is not IDynamicInterfaceCastable and when IDynamicInterfaceCastable is implemented in the compilation (based on DevirtualizationManager).

Comment on lines 324 to 327
else if (NodeFactory.DevirtualizationManager.CanHaveDynamicInterfaceImplementations())
{
return NodeFactory.MaximallyConstructableType(type);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

If we get a "metadata" type representation, we could limit this to "metadata" types.

Additionally, depending on the failure mode for "always will fail" types, we could go even further:

If a given interface type has no [DynamicInterfaceCastableImplementation] implementations, then we could return a NecessaryTypeSymbolIfPossible because there's no way the user's implementation can successfully return an implementation type. This will take further investigation though, so I didn't want to do it in this PR.

@jkoritzinsky
Copy link
Member Author

I've adjusted the InterfaceDispatchCellNode to do the same optimization as the cast target symbol optimization and that has allowed much of this work to cleanly work.

With the current changes, we have 1.7kB from IDynamicCastableSupport (and 32 B from IDynamicInterfaceCastable) preserved even when IDynamicInterfaceCastable is not used. I'll see if I can adjust the body substitutions logic to allow us to stub that out when IDynamicInterfaceCastable is unused.

@jkoritzinsky
Copy link
Member Author

I've added a commit to explicitly substitute away the entry-point to the logic that I couldn't get trimmed away so we can get most of the size savings now.

{
Dictionary<MethodDesc, BodySubstitution> bodySubstitutions = [];

TypeDesc iDynamicInterfaceCastableType = _factory.TypeSystemContext.SystemModule.GetType("System.Runtime.InteropServices", "IDynamicInterfaceCastable");
Copy link
Member

Choose a reason for hiding this comment

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

GetType has throwIfNotFound arg. You can use it to enable this path only when the type is present in CoreLib, and avoid omit IDynamicInterfaceCastable from Test.CoreLib. I assume that the type exists in Test.Corelib just to make this check happy.

Copy link
Member Author

@jkoritzinsky jkoritzinsky Jun 18, 2025

Choose a reason for hiding this comment

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

IDynamicInterfaceCastable exists in Test.CoreLib because TypeCast and CachedInterfaceDispatch call into the interface directly now instead of going through a "support" static class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants