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

Want to integrate this source generator into your project as a 'experimental feature'? #66

Open
JoaoVictorVP opened this issue Feb 18, 2024 · 14 comments
Labels
enhancement New feature or request

Comments

@JoaoVictorVP
Copy link

JoaoVictorVP commented Feb 18, 2024

A week ago, I developed this source generator in some hours for a experiment I thought was interesting, it uses the new .NET feature called 'Interceptors' to intercept the 'prototype-style queries' and turn them into the more 'boilerplate and fast' version as a method.

As an example, it will take this:

public class TestSystem : ISystem
{
    public static string SomeExternalSourceSearch(string searchFor)
        => searchFor + " found";

    [Optimize]
    public void Update(SystemProps props)
    {
        var (world, _) = props;

        var query = new QueryDescription()
            .WithAll<TestComponent, OtherComponent>();

        world.Query(query, (Entity entity, ref TestComponent test, ref OtherComponent other) =>
        {
            string searched = Optimizer.OutOfScope(() => TestSystem.SomeExternalSourceSearch("test")); // For any reason

            test.Value += 1;
        });
    }
    public void Draw(SystemProps props, SpriteBatch spriteBatch) { }
}
public struct TestComponent
{
    public float Value;
}
public struct OtherComponent;

(this ISystem is a part of a game I'm making with arch, it is a custom thing for my needs; the 'Optimize' attribute can be applied on any kind of function with as many queries as you would like)
And will turn into this:

public static class ProjectName_ComponentsUpdate_0_Interceptor
{
    [InterceptsLocation(@"C:\(...hiding my directories...)\Components\TestSystem.cs", 26, 15)]
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static void Intercept(this World world, in QueryDescription description, Arch.Core.ForEachWithEntity<ProjectName.Components.TestComponent, ProjectName.Components.OtherComponent> _)
    {
{
var searched=TestSystem.SomeExternalSourceSearch("test");
    foreach(var chunk in world.Query(description).GetChunkIterator())
    {
        var arr = chunk.GetFirst<ProjectName.Components.TestComponent, ProjectName.Components.OtherComponent>();
        foreach(var index in chunk)
        {
            ref var entity = ref chunk.Entity(index);
            ref var test = ref Unsafe.Add(ref arr.t0, index);
ref var other = ref Unsafe.Add(ref arr.t1, index);
        {

            test.Value += 1;
        }        }
    }
}
    }
}

(it's not being generated formatted currently, but it works and kinda the generated code is the same for all instances, so it should not produce any bugs)

So basically, what this does is take the closure AST and turn it into an actual boilerplate-like query, using the closure body as a simple C# block in the generated output. It also replaces any Optimzer.OutOfScope call with an actual out of scope variable, inlining the closure expression as the variable initializer (and the actual variable declaration as the, well, variable declaration).

I made some benchmarks, and it appear to have similar (or better) performance than the source generated queries, and in relation to the closure prototype non-optimized version they have marginal gains, and substantial gains when accessing objects out of the closure (I suppose because of the inlining).

These where some of my tests for it:

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22621.3007/22H2/2022Update/SunValley2)
Intel Core i5-10600KF CPU 4.10GHz, 1 CPU, 12 logical and 6 physical cores
.NET SDK 8.0.101
  [Host]     : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2


Method Mean Error StdDev Code Size
DealDamageQuery 12.141 ms 0.0529 ms 0.0494 ms 7,815 B
DealDamageQueryOptimized 9.261 ms 0.0355 ms 0.0296 ms 9,577 B
DealDamageManuallyOptimized 9.391 ms 0.1259 ms 0.1178 ms 9,180 B
DealDamageManuallyOptimizedSimpler 10.483 ms 0.0896 ms 0.0794 ms 8,957 B
DealDamageQueryFromArchExtended 10.946 ms 0.0801 ms 0.0749 ms 9,415 B

The 'Query' version is the normal prototype one, the optimized is the prototype one with code generation and my interceptors, the manually optimized one is the version I wrote manually in place, the simpler is not using unsafe and instead querying manually from the entity each iteration, and the last is your version. I think the benchmarks can be enhanced, but it sounds promising from my tests.

I still want to do some improvements here and there, and cover some edge cases (as well as adding compiler errors for non static queries and automatically infering the static method calls to the source type without needing to specify it manually, and add some testing as well), but I thought this would interest you and I'm curious if you would want to export this project if I publish it or is willing to put it inside the extended repository.

@genaray genaray added the enhancement New feature or request label Feb 20, 2024
@genaray genaray moved this to Todo in Arch Roadmap Feb 20, 2024
@genaray
Copy link
Owner

genaray commented Feb 20, 2024

It looks incredibly cool. Something I hadn't even thought about.

So it takes a standard World.Query and rewrites it into an optimized version? Does it replace the World.Query or does it just generate the optimized version and make it available? Or does it replace it? Does this only work for static methods or for everything in general?

This is very interesting! :)

@LilithSilver
Copy link

This is super cool.

A few questions:

  1. What happens if the lambda captures variables? Is there a way to grab those and put them in the interceptor?
  2. Can the OutOfScope calling requirement be replaced by an automatic parameter-variable generator?
  3. Can the Optimize attribute be removed, to automatically optimize the whole codebase?

@JoaoVictorVP
Copy link
Author

@genaray
It is pretty interesting! I was thinking of using this to optimize LINQ (like Rust does with their iterators for example) for a long time now, since I've heard of it. So I thought to myself "well, I'm already using arch, why not try this here?". I would prefer if microsoft allowed source generators to just modify the source code (or having scoped modifications like Rust macros), but this is a "middle ground" for now.

As for your questions:

So it takes a standard World.Query and rewrites it into an optimized version?

Precisely, it just finds the calls to World.Query, extract the AST of the lambda you are passing to it to iterate the components and then "paste" it in the interceptor, rewritting what is needed to make it work (like adding the for loop and replacing Optimizer.OutOfScope calls to the raw version outside the loop).

Does it replace the World.Query or does it just generate the optimized version and make it available? Or does it replace it?

It does not directly "replace" the World.Query, interceptors are more like "compiler will redirect calls from World.Query into your interceptor at compile time", so it's more "replacing the call" than the method itself (this allows a per-call optimization instead of something more general).

Does this only work for static methods or for everything in general?

I don't think I understood your question, are you asking wheter the interceptors can only be static methods, or if you can intercept anything?
For the first: at least of what I know you can only make them as static methods
And for the second: you can intercept any method call, static or not (for example, the World.Query interception is an instance-based one)

I want to work on it more to improve the codegen and make it more reliable this weekend as well.

@LilithSilver

What happens if the lambda captures variables? Is there a way to grab those and put them in the interceptor?

Yeah, this is like one of the most annoying problems about it currently... interceptors can only accept 'this' parameters and the parameters the function you are intercepting declares, so you cannot use lambda captures inside the code-generated interceptor (unless I find any way to do so, which is unlikely because C# does not allow you to modify the source code by source generators). I think this can be alleviated if you make the method itself accepting a generic parameter arbitrarily to pass around to the lambda, or maybe by using an extension method (I just thought about it now; it might work)?
But you really cannot capture the variables, so I think I will enforce something like "only static lambdas here" and explore creating an extension method maybe for allowing users to pass arbitrary variables into the World.Query, it looks like a good idea and I will try to make the source generator optimize specifically to these calls.

Can the OutOfScope calling requirement be replaced by an automatic parameter-variable generator?

Hmmm, I don't think I get it, can you explain to me what it is? OutOfScope currently just replaces the lambda inside with a variable outside of the loop scope, like this:
var x = Optimizer.OutOfScope(() => SomeHeavyMethod());
will be replaced by:
var x = SomeHeavyMethod();
for(...) // the iteration look with the rest of the lambda code
This was made to kind of help with the variable capture problem, so users can like define something statically and then use this to reference it inside of the lambda in an optimized manner.

Can the Optimize attribute be removed, to automatically optimize the whole codebase?

Oh, certainly! I'm using this solution because it:

  • Makes things easier and faster (as you only need to check methods marked by the attributes)
  • Gives more fine grained control to the developer (you can benchmark for example, or control for when you would like to rather have captured variables than the optimized version)
    But I could totally make for example an assembly-level attribute that you could use (so both versions could coexist) and allow for "whole codebase optimizations".

@genaray
Copy link
Owner

genaray commented Feb 23, 2024

So all in all this looks very very promising and I would be happy to either integrate it into Arch.System.SourceGenerator or offer it as a new package. Depending on what makes sense :)

Maybe you could even use similar techniques in some places to get even more out of Arch. We'll have to see about that.

Would you be willing to provide the project and add it to Arch.Extended? :)

@JoaoVictorVP
Copy link
Author

So all in all this looks very very promising and I would be happy to either integrate it into Arch.System.SourceGenerator or offer it as a new package. Depending on what makes sense :)
Would you be willing to provide the project and add it to Arch.Extended? :)

Amazing! Yeah, I'm willing to provide it. I've published it to a repository, which I'll link here below for you. This way you can decide which approach you find the best: Integrating with the Arch.System.SourceGenerator or creating it as a new package
Repository

I was also working on some changes like this:
image

I also made a csproj analyzer to check for the interceptor configuration on the csproj, but it was kinda frustrating because apparently you cannot do code fixes or appropriate infos/warnings/errors in a .csproj, so I later removed it.

Maybe you could even use similar techniques in some places to get even more out of Arch. We'll have to see about that.

Do you have any suggestions about these other places? I'm interested in exploring the possibilities you may have in mind.

@genaray
Copy link
Owner

genaray commented Feb 27, 2024

I had a quick look at your code and it looks great so far, good work :) I think it would make the most sense as a new package in Arch.Extended.

The only thing I would suggest for such an integration would be to split the code a bit more to make it more readable. You can use Arch.System.SourceGenerator as a guide. A model is created there which is processed further to generate code from it. Makes it a bit more readable (in my opinion). Or what do you think? :)

@JoaoVictorVP
Copy link
Author

Hi!

Thanks for reviewing the code! And also apologies for the delayed response, my job made me particularly busy this last week.

I agree with your suggestion on enhancing the code. While I also value the readability of having what I need in one sequential place, I think this code as it is will benefit by being more split and organizing some things.

I'm commited to make these changes and align the style of the code to be more like Arch.System.SourceGenerator. I'll start making these adjustments this weekend when I can dedicate focused time on it.

@genaray
Copy link
Owner

genaray commented Mar 6, 2024

Hi!

Thanks for reviewing the code! And also apologies for the delayed response, my job made me particularly busy this last week.

I agree with your suggestion on enhancing the code. While I also value the readability of having what I need in one sequential place, I think this code as it is will benefit by being more split and organizing some things.

I'm commited to make these changes and align the style of the code to be more like Arch.System.SourceGenerator. I'll start making these adjustments this weekend when I can dedicate focused time on it.

No problem, we're all busy sometimes! :) That sounds great so far, I can hardly wait. Let me know as soon as you make progress ^^

@genaray
Copy link
Owner

genaray commented Mar 13, 2024

Hi!

Thanks for reviewing the code! And also apologies for the delayed response, my job made me particularly busy this last week.

I agree with your suggestion on enhancing the code. While I also value the readability of having what I need in one sequential place, I think this code as it is will benefit by being more split and organizing some things.

I'm commited to make these changes and align the style of the code to be more like Arch.System.SourceGenerator. I'll start making these adjustments this weekend when I can dedicate focused time on it.

Had time yet? :)

@JoaoVictorVP
Copy link
Author

Hi! Yeah, last week not much hahaha, but I worked a bit on it, and today I finished. I used your project as a reference point and at the same time tried to keep it in line with how it was structured originally. I think it is more readable and easy to follow now! Please, take a look when you have some time and tell me what you think.

@genaray
Copy link
Owner

genaray commented Mar 26, 2024

I apologize for the late reply, somehow I didn't get a notification. I'll take a look tomorrow and give you some feedback, I'm curious! :)

@JoaoVictorVP
Copy link
Author

Hi, I'm back! No worries about the delay.
By the way, in this meantime I also envisioned an interesting thing I think that can get some use in future if I get to develop on it, it would be like a 'layer' on top of the C# compiler that will let developers use roslyn to actually modify code instead of just adding new code, I think it could be good for many things like eliminating the function calls interceptors still need and forcing inlining in some code (akin to what Kotlin does with the inline keyword), or maybe even to make deeper optimizations in the code (theoretically, even macros like in Rust would be possible, although simpler because they would need to have the shape of a method call, because IDE support for custom syntax is another beast). I have some ideas, but I'm still thinking on how it would be structured, but in case it advances I'll experiment with some arch projects to see how much performance it can extract (even if it runs only in release builds, it would be interesting I guess).

But anyway, let me know when you have the feedback of the code, I'm also curious!

@genaray
Copy link
Owner

genaray commented Apr 8, 2024

I had a look at the code now, all in all it looks good! I would like to integrate it directly, but unfortunately I have a lot on my plate at the moment. But as soon as I have some free time again, I'll take advantage of it. Your other plans also sound very interesting ^^

@JoaoVictorVP
Copy link
Author

No problem, thank you very much for the feedback!
I also developed this other concept a bit, like testing with static loop unrooling, and got in some conditions like 3x benefits on performance. I'm still conceptualizing how it would be working in a project-level and the best way to approach it for extensions support. But as you, I'm also being very busy lately, so when I have more free time I plan to proceed the development and test some more interesting concepts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

3 participants