Skip to content

Improve startup and runtime performance by removing XamlTypeInvoker dead code #9736

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ThomasGoulet73
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 commented Sep 7, 2024

Description

Removes XamlTypeInvoker dead code. This code is hit when creating an instance of a non-known type from XAML/BAML. Removing this code improves both startup and runtime performance.

Known types are types in an hard-coded list that makes them more optimized by WPF. This includes types from WPF and some BCL common types (Like int, bool, string, etc). Everything else is a non-known type so things like custom controls are non-known types.

The reason this is dead code is because DefaultCtorXamlActivator.EnsureConstructorDelegate always returns false because of this condition here:

if ((tConstInfo.IsSecurityCritical && !tConstInfo.IsSecuritySafeCritical) ||

Since the removal of CAS from .Net, ConstructorInfo.IsSecurityCritical always returns true and ConstructorInfo.IsSecuritySafeCritical always returns false so the the condition is always true which means that the whole DefaultCtorXamlActivator class doesn't do anything.

Here's the .Net code for ConstructorInfo.IsSecurityCritical and ConstructorInfo.IsSecuritySafeCritical which always return constants:
https://github.com/dotnet/runtime/blob/a349912e4f8610f39a6ac3cb73d8d4d0968ccec4/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs#L228-L230

Benchmarks

I benchmarked a new WPF from the Visual Studio template where I added an empty class like public class CustomLabel : Label { } and I added this control 20 times in the XAML of the main window. This PR shaves about 30-40 ms when starting the app.

There's a small cache per-type that skips some of the code so I also benchmarked a new WPF from the Visual Studio template where I added 20 empty classes like public class CustomLabel1 : Label { } and I added each control in the XAML of the main window. This PR shaves about 50-60 ms when starting the app.

I expect more complex apps with more non-known type in XAML to have even bigger performance gain. Projects that have a ton of non-known types are third-party control vendors (Like DevExpress, Telerik, etc) because they have a ton of custom controls.

Customer Impact

Better startup and runtime performance.

Regression

No.

Testing

Local testing using sample app that hits this specific code.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@ThomasGoulet73 ThomasGoulet73 requested review from a team as code owners September 7, 2024 06:44
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Sep 7, 2024
@dipeshmsft dipeshmsft self-assigned this Sep 9, 2024
@ThomasGoulet73 ThomasGoulet73 force-pushed the remove-XamlTypeInvoker-dead-code branch from 01057b8 to 2c17842 Compare December 17, 2024 23:48
@ThomasGoulet73
Copy link
Contributor Author

I rebased to fix the conflicts.

@ThomasGoulet73 ThomasGoulet73 force-pushed the remove-XamlTypeInvoker-dead-code branch from 2c17842 to e7877f9 Compare December 28, 2024 06:26
@ThomasGoulet73
Copy link
Contributor Author

I rebased to fix the conflicts.

@ThomasGoulet73
Copy link
Contributor Author

ThomasGoulet73 commented Jan 23, 2025

I rebased on main to fix the conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants