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

Roslyn Source Generator for targets #1101

Open
galvesribeiro opened this issue Dec 30, 2024 · 1 comment
Open

Roslyn Source Generator for targets #1101

galvesribeiro opened this issue Dec 30, 2024 · 1 comment

Comments

@galvesribeiro
Copy link

Hello!

We are using this package for a recent project and we were looking for supporting AoT. However, we quickly noticed the heavy usage of reflection in particular in the proxies and the dynamic assembly, regardless if the target uses or not an interface, or if we hardcode the method using AddLocalRpcMethod(). The only benefit of keep using reflection is because I see is that we can dynamically add new targets at runtime but even that use case, according to the docs, is not encouraged after the listener starts, hence why it is guarded by that exception/flag.

It would be nice to have a Roslyn source generator as an external package which allow people which choose to use the reflection approach without breaking anything, and optionally generate the proxy types at build time and embed in the output assembly. We do that a lot in Microsoft Orleans for all the serializer types and invokers.

Would a PR be accepted to add such package? If so, what should I look in the current codebase for the desired generated types?

The approach I would take would be:

  1. Create a source generator package/project StreamJsonRpc.CodeGen as a Roslyn incremental source generator
  2. Introduce a new attribute [JsonRpcTarget] which can be used in classes.
  3. At build time, scan the classes which have that attribute and follow the same discovery process done Today by the reflection code, like look for the [JsonRpcMethod], then other public methods without it, ignore private, etc.
  4. With the discovered types, generate the code as partial and append to the assembly
  5. When the target is added, the generated code would be there so no reflection needed

This would allow people to AoT their applications. If the user decide to use the dynamic target registration along with the source generated, then, at runtime, we would have the bootstrapping benefits on the source generated classes but at the same time add targets dynamically. If the dynamic target doesn't have generated code, it fallback to the reflection emit approach, which obviously wouldn't work in AoT scenarios.

Please let me know if this is something you guys would accept as a PR and if so, where I should look for the desired generated output so I can quickly get into it and submit the PR.

Thanks!

@AArnott
Copy link
Member

AArnott commented Jan 11, 2025

Thanks for the feedback, and especially for offering to contribute to the codebase!
There are two aspects to StreamJsonRpc that are relevant here: use of reflection and use of Ref.Emit. Reflection can work with AOT with caveats, but Ref.Emit cannot work with AOT.
We use Ref.Emit for the proxies, but proxies are optional so you can avoid those.
We use reflection a lot, including for closing generic types which is not generally safe in a trimmed application such as AOT.

The overall goal you have is one we're only recently considering as a goal for the library. It'll be a mammoth task (far more than one PR). But you can see we're already exploring it with #1100, which adds a formatter that is AOT-safe. Of course the formatter itself is just one part of the overall StreamJsonRpc library and getting the rest of it to be AOT ready will be a lot more work. And you're right, that source generation will certainly be a part of the solution.

We're not committed to delivering AOT support at this point, but I'm hopeful that we can at least move in that direction. As we don't yet have a design for it, I suggest you not spend your time (yet) contributing to that effort. But if we commit to a design, we'll open an issue and document it. At that point, I expect there will be areas of it that we'd welcome pull requests from yourself and others who are interested in contributing.

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