Skip to content
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

AggressiveInlining for custom markup extensions #17622

Open
Mikolaytis opened this issue Nov 26, 2024 · 9 comments
Open

AggressiveInlining for custom markup extensions #17622

Mikolaytis opened this issue Nov 26, 2024 · 9 comments

Comments

@Mikolaytis
Copy link
Contributor

Mikolaytis commented Nov 26, 2024

Is your feature request related to a problem? Please describe.

We use around ~3k markup extensions in our app's markup - this is causing 10-100k redundant MarkupExtensions objects allocations on launch.
here is how x:Static works after xaml compilation for Data="{x:Static Icons.Bool_Union}”

 path.Data = Icons.Bool_Union;

here is how markup extension works forHeader="{Loc BoolOperations_Union}"

Loc loc = new Loc(LocKey.BoolOperations_Union);
  context.ProvideTargetProperty = HeaderedSelectingItemsControl.HeaderProperty;
  object obj6 = loc.ProvideValue((IServiceProvider)(object)context);
  context.ProvideTargetProperty = null;

Describe the solution you'd like

I see 2 solutions:

  1. We want an attribute for MarkupExtension like [XamlAggressiveInlining]
  2. some kind of custom x:Static extensions

Describe alternatives you've considered

usage of x:Static is an option, but adds a lot of boilerplate on a large scale.
Compare:
{Loc AppHeader} - with an 10-line markup extension code
{x:Static Loc.AppHeader} and large code-generated 10k lines Loc static class with all the possible Loc properties.

Additional context

No response

@stevemonaco
Copy link
Contributor

Pooling could be a simpler strategy for high-use extensions, such as localization. MarkupExtension should be short-lived: created, provide a value, and never used again.

@maxkatz6
Copy link
Member

It might be obsolete with .NET 9/10 optimizations, as these should eliminate short living objects.

@maxkatz6
Copy link
Member

this is causing 10-100k redundant MarkupExtensions objects allocations on launch

What about:

public class CustomMarkupExtension
{
  public static YoutReturn ProvideValue(IServiceProvider _, YourInput content)
  {
  }
}

Where it restricts markup extension, without allowing parameter options.

@Mikolaytis
Copy link
Contributor Author

It might be obsolete with .NET 9/10 optimizations, as these should eliminate short living objects.

maybe sometime it will be implemented and maybe it will work in some cases, hopefully case above, but let's be real :)

@Mikolaytis
Copy link
Contributor Author

maybe making a custom struct markup extension will work well, but still, the goal is not 4 lines of code, but 1, like x:Static.

@MrJul
Copy link
Member

MrJul commented Nov 27, 2024

What about:

public class CustomMarkupExtension
{
  public static YoutReturn ProvideValue(IServiceProvider _, YourInput content)
  {
  }
}

Where it restricts markup extension, without allowing parameter options.

Ah, that's the exact same idea I had in my (now rather long) todo list! StaticResource would perfectly match this pattern.

@kekekeks
Copy link
Member

kekekeks commented Dec 1, 2024

public static class FastMarkupExtension
{
    public static struct FastMarkupExtensionParameters
    {
        public IServiceProvider ServiceProvider; // optional, special naming
        public IProvideValueTarget ProvideValueTarget; // optional, special naming
        [ConstructorArgument]
        public string Arg1;
        public string Arg2;
    }
    public static string ProvideValue(ref FastMarkupExtensionParameters args);
}

@maxkatz6
Copy link
Member

maxkatz6 commented Dec 1, 2024

Out of interest, I tried a couple of options in sharplab, to see how generated IL code looks like.
Particularly using static extension + struct param, static extension + ref struct param and class extension:
https://sharplab.io/#gist:3277c818c93820dbdd51113423703c6a

For relatively simple ProvideValue implementation, there was virtually zero difference between ref or non-ref struct param, JIT was able to inline either of them, eliminating struct completely.

And class markup extension wasn't inlined at all, allocating an object... In .NET Framework that is, and older Core runtimes.
If you switch to "Default" or "x64", i.e. latest stable runtime, JIT is already clever enough to inline everything, even a class markup extension:

C.StaticStructProvideValueRef()
    L0000: mov eax, 9
    L0005: ret

C.StaticStructProvideValue()
    L0000: mov eax, 9
    L0005: ret

C.ClassProvideValue()
    L0000: mov eax, 9
    L0005: ret

I haven't tested, but complex ProvideValue code will likely prevent inlining, and likely return object allocation. But if ProvideValue kept simple enough by redirecting call to another static method, it might work good enough.

@maxkatz6
Copy link
Member

maxkatz6 commented Dec 1, 2024

Yep. Implemented inlining-friendly static resource extension, and it completely eliminated object allocation.
Which relies on new .NET 9 optimizations.

-       object? Old(IServiceProvider serviceProvider)
+       object? New(IServiceProvider serviceProvider)
        {
-           var ex = new StaticResourceExtension("SystemAccentColor");
+           var ex = new StaticResource2Extension("SystemAccentColor");
            return ex.ProvideValue(serviceProvider);
        }
-; Assembly listing for method AvaloniaApplication5.CustomTheme:Old(System.IServiceProvider):System.Object:this (FullOpts)
+; Assembly listing for method AvaloniaApplication5.CustomTheme:New(System.IServiceProvider):System.Object:this (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; rsp based frame
; fully interruptible
; No PGO data
-; 0 inlinees with PGO data; 3 single block inlinees; 0 inlinees without PGO data
+; 0 inlinees with PGO data; 5 single block inlinees; 0 inlinees without PGO data

G_M000_IG01:
-       push     rbx
-       sub      rsp, 32
-       mov      rbx, rdx
 
G_M000_IG02:
-       mov      rcx, 0xD1FFAB1E
-       call     CORINFO_HELP_NEWSFAST
-       mov      rcx, 0xD1FFAB1E
-       mov      gword ptr [rax+0x08], rcx
-       mov      rcx, rax
-       mov      rdx, rbx
+       mov      rcx, rdx
+       mov      rdx, 0xD1FFAB1E

G_M000_IG03:
-       add      rsp, 32
-       pop      rbx
-       tail.jmp [Avalonia.Markup.Xaml.MarkupExtensions.StaticResourceExtension:ProvideValue(System.IServiceProvider):System.Object:this]
+       tail.jmp [Avalonia.Markup.Xaml.MarkupExtensions.StaticResource2Extension:ProvideValue(System.IServiceProvider,System.Object):System.Object]

-; Total bytes of code 54
+; Total bytes of code 19

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

No branches or pull requests

5 participants