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

Design Proposal: Reducing the reflection dependency #1

Open
matt-andrews opened this issue Nov 22, 2024 · 1 comment
Open

Design Proposal: Reducing the reflection dependency #1

matt-andrews opened this issue Nov 22, 2024 · 1 comment

Comments

@matt-andrews
Copy link

Hello stranger! I ran across your reddit post for this project, to be completely honest I don't have a lot of use for desktop applications these days, but I've identified a potential solution to your reflection problem and thought I'd share (if you want). This might be considered a micro-optimization, so feel free to tell me to go to hell.

Summary

Reflection is expensive. In an application such as this, efficient interaction between the framework and the users code is critical for widespread adoption. I will spend the following wall of text explaining how we can accomplish this.

Solution

Source Generators! tl;dr source generators are a roslyn analyzer feature that when implemented, generates code based on code the user enters. We can then use the generated code to interface with user code instead of relying on reflection.

The following rough examples are in the context of registering a component and invoking the methods of a component.

Imagine a new interface:

public interface IIpcComponent
{
    object? Invoke(string target, object?[]? args = null);
}

And imagine that your IpcContainer looks more like this:

public class IpcContainer
{
    private readonly Dictionary<string, IIpcComponent> _instanceCache = [];
    public IpcContainer Register<T>(string instanceName, T target)
        where T : notnull, IIpcComponent
    {
        //we could even get rid of the instanceName param and use typeof(T).GetName() instead
        //for an even better user experience!
        if(!_instanceCache.TryAdd(instanceName, target))
        {
            throw new InvalidOperationException("This instance name already exists in the container");
        }

        return this;
    }

    public IIpcComponent GetInstance(string instanceName)
    {
        if (!_instanceCache.TryGetValue(instanceName, out var instance))
        {
            throw new ArgumentException($"Instance '{instanceName}' not found.");
        }

        return instance;
    }
}

Now the changes above are already a huge performance boost in regards to registering a new component - since all were doing is managing a dictionary. So how do we make this work with the rest of the framework, you might ask? This is where source generation comes in. Imagine a simple component:

public partial class TestComponent
{
    [IpcExpose]
    public int Add(int a, int b)
    {
        return a + b;
    }
}

Using a simple source generator we can very easily generate the following, triggered by the attribute:

// <auto-generated/>
using System;

#nullable enable
public partial class TestComponent : IIpcComponent
{
    public object? Invoke(string target, object?[]? args = null)
    {
        switch(target)
        {
            case nameof(Add): 
            {
                //ArgsBuilder is a simple helper class to cast the positional args
                ArgsBuilder a = new(args); 
		return Add(a.MoveNext<Int32>(), a.MoveNext<Int32>());
            }
            default:
                throw new Exception($"Target {target} not found in available methods");
        }
    }
}

With the generated partial class, the TestComponent class is now of type IIpcComponent and can be registered with the IpcContainer, leading us to the ability to invoke methods dynamically without the cost of reflection.

Note

I'm not including the actual source generation code here because it would bloat this proposal even more. Suffice to say the above code is 100% generated lol

Benchmarks

These benchmarks probably aren't super accurate since I threw them together on short notice, but the numbers are interesting to consider:

Component Registration

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22621.1702/22H2/2022Update/SunValley2)
12th Gen Intel Core i5-12600K, 1 CPU, 16 logical and 10 physical cores
.NET SDK 8.0.400
[Host] : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
DefaultJob : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2

Method Count Mean Error StdDev Ratio RatioSD Gen0 Gen1 Gen2 Allocated Alloc Ratio
Reflection 10 14,013.8 ns 127.22 ns 119.00 ns 1.00 0.01 1.7395 0.0610 - 17.97 KB 1.00
SourceGen 10 619.0 ns 7.67 ns 7.17 ns 0.04 0.00 0.7248 0.0191 - 7.41 KB 0.41
Reflection 100 162,327.8 ns 1,865.01 ns 1,744.53 ns 1.00 0.01 17.0898 4.3945 - 178 KB 1.00
SourceGen 100 6,827.7 ns 136.42 ns 204.19 ns 0.04 0.00 7.2479 1.5793 - 74.05 KB 0.42
Reflection 1000 4,472,908.4 ns 44,627.46 ns 41,744.56 ns 1.00 0.01 171.8750 109.3750 - 1796.05 KB 1.00
SourceGen 1000 94,303.8 ns 1,048.52 ns 980.78 ns 0.02 0.00 74.5850 44.9219 - 762.35 KB 0.42
Reflection 10000 349,939,866.7 ns 5,731,331.73 ns 5,361,091.26 ns 1.00 0.02 1000.0000 - - 18140.77 KB 1.00
SourceGen 10000 3,894,223.1 ns 51,385.16 ns 40,118.15 ns 0.01 0.00 710.9375 671.8750 312.5000 7629.44 KB 0.42

These metrics are not surprising at all, since the V2 version of the IpcContainer is basically just a wrapper over a dictionary - but it shows what kind of penalty we're paying with the original implementation.

Component Invocation

Method Count Mean Error StdDev Ratio Gen0 Allocated Alloc Ratio
Reflection 10 794.8 ns 6.47 ns 6.05 ns 1.00 0.1984 2.03 KB 1.00
SourceGen 10 288.9 ns 1.99 ns 1.77 ns 0.36 0.1683 1.72 KB 0.85
Reflection 100 7,382.9 ns 68.46 ns 60.69 ns 1.00 1.9836 20.31 KB 1.00
SourceGen 100 2,893.0 ns 13.37 ns 11.85 ns 0.39 1.6823 17.19 KB 0.85
Reflection 1000 78,986.5 ns 592.94 ns 554.63 ns 1.00 21.9727 225 KB 1.00
SourceGen 1000 35,216.4 ns 502.42 ns 469.96 ns 0.45 18.9209 193.75 KB 0.86
Reflection 10000 1,156,346.8 ns 11,557.47 ns 10,810.86 ns 1.00 228.5156 2334.38 KB 1.00
SourceGen 10000 457,363.7 ns 8,492.86 ns 7,944.23 ns 0.40 197.7539 2021.88 KB 0.87

These numbers are a little more interesting, this is calculated by getting a component from the container and invoking it with _container.Invoke("Add", [10, 5]); and compared to a slimmed down version of the original. As the metrics show, we can save a significant amount of time on the invocation by using source generators instead of reflection.

Cons

So we've talked about all the pros for source generators, what are some of the cons?

  • Increased code complexity: Source generators make the underlying code much more complicated to maintain and understand - sometimes it feels like magic (not the good kind!). Any experienced dev can look at GetType().GetMethods() and know that reflection is being used to get a types methods; but magic attributes that make a type suddenly have an interface and a method that gets other methods (???)
  • This would be a breaking change: Since the added requirement that classes that have the IpcExposeAttribute or have methods that use IpcExposeAttribute must be partial, this would be a breaking change and require a major version bump :(
  • Not all reflection can be (or should be) replaced with source generators. Obvious hot paths such as registration/invocation, but I haven't looked hard enough to determine if anywhere else should be.
  • Its a lot of work to perform this kind of migration, so you must ask yourself "is it worth it?"

Conclusion

Thinking back I'd probably use an abstract class instead of an interface for IIpcComponent - it kind of feels more natural, and frees us up to pass in cool things during the component lifecycle if we wanted in the future.

I don't really have much else to add tbh, I couldn't sleep last night so this is what I did instead lol. Anyways, this was a fun thought experiment since I've never actually created a source generator before today.

If I don't hear from you again - good luck!

@UltraWelfare
Copy link
Owner

Hey @matt-andrews, thanks for your interest and taking the time to write the proposal !

This might be considered a micro-optimization

It really depends, most apps will do I/O with the database which really is the biggest culprit of performance instead of reflection.
But in cases where something more "real-time" is needed (streaming huge amount of data through IPC) then reflection is indeed a really bad idea.
This is also the reason I'm mentioning it in the Limitations section of README, so people with projects that need lightning-fast performance shouldn't expect much from this library.

The source generator is an interesting idea that I'm willing to explore to improve things up but as mentioned in your Cons section, it will greatly improve code complexity of the library. I'm also personally unfamiliar with writing source generators themselves. It's definitely an idea for the future.

I'm also interested in how that would work in the next iteration of the IPC. I'd like to share some progress I've made behind the scenes.

I've reworked the IPC, such that it allows more creative freedom. The current solution involves only method invokation without any level of depth. For example I can't do ipc.instanceOfClass.propertyOfClass.anotherPropertyOfClass.InvokeMethod(...); you're only limited to ipc.{instance}.{method}(...);

This is especially problem when it comes to getting something from a property. Since you can't get properties you'd have to write a method to get that property which introduces more boilerplate.

I've devised a new way of interacting with IPC that allows you to do this. Technically how it works is by creating this JSON:

{
  "uuid": "...",
  "instance": "calculator",
  "path": "instanceOfClass.propertyOfClass.anotherPropertyOfClass.InvokeMethod($1);",
  "args": {
    "$1": [{ name: "Data" }]
  }
}

(I'm not really entirely sure why it's highlighting part of it in red)
Or really any kind of path a.b.c().d.e().f (yes you can even index lists). This is a bit more "heavy" when it comes to reflection because we have to iteratively get every property or call a method. The code looks like :

private static async Task<object?> Execute(object instance, string path,
    Dictionary<string, List<JsonElement>>? args)
{
    var parts = path.Split('.');
    var currentObject = instance;

    foreach (var part in parts)
    {
        currentObject = part.Contains('(')
            ? await HandleMethodInvocationAsync(currentObject, part, args)
            : HandlePropertyAccess(currentObject, part);
    }

    return currentObject;
}

// I'm only including the method invocation part. I've also removed error checking, to reduce the code for brevity reasons
private static async Task<object?> HandleMethodInvocationAsync(object? currentObject, string part,
    Dictionary<string, List<JsonElement>>? args)
{
    var openParenIndex = part.IndexOf('(');
    var closedParenIndex = part.IndexOf(')');

    var methodName = part[..openParenIndex];
    var type = currentObject.GetType();
    var method = type.GetMethod(methodName);

    if (!IsIpcExposed(method) && !IsIpcExposed(currentObject.GetType()))
        throw new InvalidOperationException($"Method '{methodName}' is not exposed to IPC.");

    var parameters = method.GetParameters();
    var argKey = part[(openParenIndex + 1)..closedParenIndex];
    var methodArgs = args != null && args.TryGetValue(argKey, out var jsonElements)
        ? ResolveArgumentsFromJson(parameters, jsonElements)
        : null;
    // InvokeMethodAsync, is a helper that will either invoke sync or invoke and await async functions
    return await InvokeMethodAsync(currentObject, method, methodArgs);
}

I wonder in that case if the generated code would work... In a first glance it seems like it would work perfectly, but I thought of sharing my WIP anyways since it's different than what we have now in the library. But I think it's better in terms of IPC code since now you can do something like:

public class Playlist
{
    public List<Song> Songs { get; set; } = [];
    
    public async Task AddSongToDatabase(Song song)
    {
        // simulate database call
        await Task.Delay(1000);
        
        Songs.Add(song);
        
        Console.WriteLine($"Adding {song.Name} to database");
    }
}
public class Song(string name)
{
    public string Name { get; set; } = name;
    
    public void Play()
    {
        Console.WriteLine($"Playing {Name}");
    }
}

// inside the container:
Register("playlist", new Playlist());

And when it comes to javascript, it allows you to do:

await ipc().playlist.AddSongToDatabase({ Name: "s1" }).send();
await ipc().playlist.Songs[0].Play().send();

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

No branches or pull requests

2 participants