refit icon indicating copy to clipboard operation
refit copied to clipboard

feature: Refit should be linker-friendly and support trimming

Open Sergio0694 opened this issue 1 year ago β€’ 38 comments

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

Yes. The issue is that despite Refit uses source generators, the generated code is just a thin proxy back to the reflection-powered functionality in the main library. This means that using Refit with trimming enabled completely breaks it. As it is, there's not much value in having source generators. They should be updated so that all the reflection-powered logic is executed at compile time, and the generated stubs will just have all the logic that is today done at runtime by the method builder APIs. This library should effectively require no runtime reflection at all (or, virtually none, but especially none that can break trimming).

Describe the solution you'd like

As mentioned, the source generator should be updated so that instead of just calling back into the method builder and invoking that reflection-based callback to perform the request, it'd instead emit all the same logic that's today done at runtime. That is, all the various marshalling of parameters, request building etc. should just be done in a statically-analyzeable way and generated at compile time. The library should just expose helper APIs for the generator to use to construct the individual bits of a request/response.

All the information that the method builder is using at runtime is there at compile time too anyway. That logic should just essentially be inverted, and the generator APIs should be used to achieve static reflection, instead of runtime reflection like today.

If there's any reflection left, additionally, all those APIs should be properly annotated.

Refit as a whole should have the trimming analyzers enabled when on the .NET 6+ target.

Describe alternatives you've considered

Runtime directives can be used to patch the problem. This is extremely cumbersome, hard to do especially for beginners, and causes an unnecessary binary size increased, given you'd be forced to preserve way more stuff than strictly needed, to be safe.

Describe suggestions on how to achieve the feature

  • Investigate all the individual bits that method builder uses to construct the request.
  • Expose the necessary helper APIs for them from that type.
  • Update the generator to do all that introspection at compile time.
  • Update the generator to perform the needed calls to those helpers to construct and execute the request right there.

Sergio0694 avatar Aug 01 '22 11:08 Sergio0694

Hey @clairernovotny! πŸ‘‹ Can we coordinate with you to discuss, plan and eventually implement this feature, or would you be able to connect us with the right folks to talk about this? @Kant2002 has kindly offered to help with this (he has experience doing very similar work, and has contributed to several source generator and trimming related work on other projects in the past), and he was wondering how to go about this, given that the scope of the work involved for this really needs some discussion first πŸ™‚

I was thinking we could do pretty much what we did back in #836 and following PRs, where we added System.Text.Json support and eventually published a new major release that split Newtonsoft.Json out in a separate package so consumers could avoid taking that extra dependency. The amount of changes here would also make this not be backwards compatible, so I feel we could do the same and do this in a new major release.

NOTE: this would only be a binary breaking change. Consumers bumping the NuGet reference and rebuilding from source should not actually encounter breaking changes, because the generators would just generate all updated code on the spot.

Let us know how we can proceed with this! We really think adding proper trimming and reflection-free support for Refit would be a huge win for a ton of consumers. Especially now that starting from .NET 7 (see here), NativeAOT builds will try to trim everything by default, which means people publishing and referencing an assembly using Refit will start to see more linker warnings now, because the compiler will attempt to trim everything. And unless they changed that back, they'd now also have runtime issues, because Refit just breaks down with trimming enabled.

Thank you! πŸ™‚

Sergio0694 avatar Aug 09 '22 17:08 Sergio0694

This feature would be great when used in conjunction with a source generator based DI system like https://github.com/YairHalberstadt/stronginject. It would make Refit usable in embedded applications which benefit heavily from NativeAOT compilation.

GitHub
compile time dependency injection for .NET. Contribute to YairHalberstadt/stronginject development by creating an account on GitHub.

crozone avatar Nov 11 '22 00:11 crozone

Microsoft: "Please rewrite your entire library and spend hours of your limited free developer time, because we broke it. Again. Users will receive absolutely no new feature or benefit as a result of this work, they will simply have exactly what they did before. Oh, and we also plan to include breaking changes in your library."

I recommend users disable NativeAOT.

anaisbetts avatar Nov 13 '22 10:11 anaisbetts

I think there's some misunderstanding here and some incorrect statements, and also some frustration towards Microsoft that I'm not really sure is warranted in this specific case. Let me try to clarify things here.

"Microsoft:"

This was just me proposing an improvement. I wasn't speaking for Microsoft, and I don't think I ever said that that was the case here. This was just something I wanted to start a conversation about, as I'm personally a fan of refit πŸ™‚

"Please rewrite your entire library and spend hours of your limited free developer time"

Nobody said anyone from the refit team should rewrite the entire library. In fact, @kant2002 already said he'd like to help work on this, and I also said I'd be happy to provide him with assistance.

"because we broke it"

Nobody broke anything. Refit still works just fine, it's just very inefficient and doesn't support trimming, but that has always been the case.

"Users will receive absolutely no new feature or benefit as a result of this work, they will simply have exactly what they did before"

This is just clearly false and also very easy to disprove. I already outlined the improvements for end users that these changes would being. The library as a whole would be faster, it would give them the ability to manually inspect the code being used and improve their debugging experience, it would make the whole library trimmable and it would also make it much easier to use it on AOT scenarios. Those are valid benefits for end users.

"Oh, and we also plan to include breaking changes in your library"

I already talked with @clairernovotny internally and she said this would be fine, which is the reason why @kant2002 had already started some preliminary work. The breaking changes would go into a new major version, so customers could also choose to just remain on the previous one until they're ready to migrate. This is exactly what we did when I added the System.Text.Json support to refit and we later decided to make it the default and pull Newtonsof.Json out and in a separate package.

"I recommend users disable NativeAOT."

This is just poor advice, and it's unwarranted.


Let me clarify things some more as well. I really like refit, and as I've mentioned I've also already contributed to it several time in the past. The reason why I'm proposing this is precisely because I like this project, and I'd love to see it improve, address the architectural issues it currently has, and become the best HTTP REST library it can possibly be. I've opened this proposal because I wanted to start a conversation about this and also allow @kant2002 to get in touch with the maintainers and get assurances that if he had done work it wouldn't have been blocked after the fact (which is why we reached out to Claire). This post wasn't meant as a critique on anyone nor as a demand on anyone from the team to do work for free πŸ™‚

Sergio0694 avatar Nov 13 '22 13:11 Sergio0694

As person who want this feature. Is this would be accepted if I implement this?

kant2002 avatar Nov 13 '22 15:11 kant2002

The library as a whole would be faster

Do you have any data to prove that, in real-world scenarios involving actual network requests, that this makes any significant difference?

much easier to use it on AOT scenarios.

This is Curious because Refit actually uses source generators because of Xamarin AOT compilation requirements on iOS, and there it has worked for nearly a decade without issues. Why is this different?

I think it is more appropriate to take a wait-and-see approach on this one, rather than radically rewriting how Refit works (for the third time!), in order to chase the latest Microsoft feature. If this is still causing users grief in a year, I think it's worth converting it.

anaisbetts avatar Nov 13 '22 15:11 anaisbetts

"Do you have any data to prove that, in real-world scenarios involving actual network requests, that this makes any significant difference?"

Yes, I'd you're doing few very long requests the relative impact would of course be minimal, but as I mentioned that wouldn't be the main point (though for apps doing tons of smaller requests it could still be noticeable). The main benefit would be supporting trimming, improving debugging, proper AOT support, and less impact on the IDE performance (this last one is a separate point, but it would be another benefit of a better source generation architecture).

"This is Curious because Refit actually uses source generators because of Xamarin AOT compilation requirements on iOS, and there it has worked for nearly a decade without issues. Why is this different?"

Using source generators alone is not a magic wand that automatically makes code trimmer-safe or AOT-friendly. Refit uses source generators, yes, but the architecture is pretty suboptimal: the generated code is just a few very thin stubs that just forward all the logic back into the library, which then uses some admittedly pretty extreme reflection (and comments in the code do say as much) to make things work. As in, refit is not using source generators in a way that a truly source generator first architecture would. As a result, the code can work in some AOT scenarios, but it's still fundamentally inefficient, not AOT-friendly and doesn't support trimming at all (that's both due to the architecture, and the fact it's not even annotated). For instance, if you use refit on AOT and enable trimming, it will not work at all and it will just crash at runtime.

"in order to chase the latest Microsoft feature"

To clarify: this is not something new per se. As I mentioned, refit has been broken with trimming on UWP for years already. The idea is just that now that better source generator APIs are available, trimming is more widespread, trimming annotations are available, NativeAOT is officially supported, it seems like a good time to start looking into whether this would be doable πŸ™‚

(Which it is, it just requires some work).

Sergio0694 avatar Nov 13 '22 16:11 Sergio0694

Refit uses source generators, yes, but the architecture is pretty suboptimal: the generated code is just a few very thin stubs that just forward all the logic back into the library, which then uses some admittedly pretty extreme reflection

....you know that I wrote this library right? Why are you explaining my own code to me

anaisbetts avatar Nov 13 '22 17:11 anaisbetts

"This is Curious because Refit actually uses source generators because of Xamarin AOT compilation requirements on iOS, and there it has worked for nearly a decade without issues. Why is this different?

I... Just answered your question πŸ₯²

I'm aware you wrote most of this library, but I'm not sure how you expected me to be able to answer that without having to underline some bits of how the codebase is structured versus what is being proposed here, which is what I did. If you meant that you already knew all of this too, well then I'm afraid I don't understand why you even asked that question in the first place.

Sergio0694 avatar Nov 13 '22 17:11 Sergio0694

My motivation to work on this issue is following. I would like to have better ecosystem which cares about NativeAOT + reflection-free mode. Reflection-free mode is experimental mode for NativeAOT, so you may think about it as niche within niche. Even if that's niche, whether possible I try to contribute back to existing libraries. Based on my observations, most code works just fine even in this mode, if augment things here and there with source generators (which I use like Reflection at compile time). Usually this results in the all dynamic code to be generated by sourcegen and visible to developers. That helps trimming technology within dotnet reason about things and end-user (developer) do not have to think about how to massage things to please trimming. I would say this also allows other developers to better reason about library, but that's maybe my opinion.

Who cares about this subniche. I would say game developers, hot-heads like me, embedded developers and quite possible cloud developers. Game developers and embedded developers obviously like squeezing water from stone, so this case is definitely good for them. Cloud developers would have faster startup time so for some workflows that's allow them save money.

I do not expect huge adoption of NativeAOT (default mode) in near future so if you drive this library purely based on demand, then I understand why you would like to wait and see. My experience is that unrestrained usage of reflection may cause issue for end-developers and complicate their lives. I do want that working with NativeAOT would be as pleasant as with CoreCLR, so that's why I'm going to libraries and try to fix things here and there.

If you think that's not important for you, I can understand that. No skin of my back. Then I have to create something similar but without reflection myself. Either way, you may see that I do not super speedy in writing code for this issue. If there chance that I may persuade you extend support of your for these niches I prefer that way.

kant2002 avatar Nov 13 '22 18:11 kant2002

Microsoft: "Please rewrite your entire library and spend hours of your limited free developer time, because we broke it. Again. Users will receive absolutely no new feature or benefit as a result of this work, they will simply have exactly what they did before. Oh, and we also plan to include breaking changes in your library."

I recommend users disable NativeAOT.

Well, this is a surprisingly hostile response to a feature request...

NativeAOT is an opt-in technology, but a valuable one in certain contexts where a JIT is just too slow or completely unavailable. For the moment, if an application requires NativeAOT, it simply cannot use Refit, which sucks once you're used to how nice Refit makes working with APIs. Going back to HttpClient boilerplate is a pill.

The reason this issue exists at all is because we like Refit and we want to use it in even more scenarios. It's a request to collaborate, not to ask you to do all the work, but to focus community effort towards this feature because it is an open source project and it makes all of our development experiences better. I really hope you consider re-opening this issue because true reflection-less Refit would be a massive boon to many.

crozone avatar Nov 14 '22 00:11 crozone

In isolation, I would not be so hostile to this request - it is Annoying, but Reasonable. However, having authored many C# libraries and having things like this happen over and over and over, it is frustrating, both as a user of C# who Wants It To Be Good, and as a maintainer.

.NET decides to break the world, and then eagerly rushes to convince everyone to get on board. The ecosystem gets left in a fractured mess over and over, as maintainers throw out all of their working, production-ready code and start again. Why is it okay to ship NativeAOT with such massive breaking changes? Xamarin shipped AOT almost a decade earlier and absolutely bent over backwards to still support existing code. Why? Because not breaking the ecosystem matters. And in fact, Refit works just fine with Xamarin AOT on iOS, arguably the most hostile AOT environment that exists!

Even the suggestion of "Just major version it bro" like it magically waives away all compatibility issues is really frustrating to me - now anyone who has a shared library using Refit, who tries to integrate the latest version to their app, is completely stuck. This is not an edge case, this happens all the time in corporate environments. But instead, we are now going to break all of those people. That seems Bad!

As a developer and maintainer, I am very much in the Raymond Chen camp of software design - Don't. Break. Software, unless it's really really really worth it. This is a part of why people, after all these years, still use things like Refit, and ReactiveUI, and Squirrel.Windows - because they don't constantly break people.

anaisbetts avatar Nov 14 '22 13:11 anaisbetts

Thanks for re-opening the issue.

Just briefly on reflection not working in NativeAOT vs working Xamarin AOT (iOS), I believe that this isn't a new issue, rather it comes down to how trimming is configured in Xamarin. In my experiences with Xamarin, aggressive member-level trimming can be enabled in the build settings, but is often avoided because it completely breaks most applications which rely on reflection, and even some that don't (it certainly broke every app I ever put through it). Assembly level trimming is far more common, but produces very large applications. The average untrimmed Xamarin app is pushing 100MB and up in size, and while it's not broken, it's still pretty non-ideal.

NativeAOT is aiming for small lightweight binaries and requires trimming integrally. Disabling trimming support for all of Refit won't necessarily work because Refit and reflection-based serializers have the additional complication of reflecting over user-provided types. This means that the user would also need to manually mark all of their types with trimming annotations and the general user experience wouldn't be amazing. This is already the case in Xamarin too.

As a developer and maintainer, I am very much in the Raymond Chen camp of software design - Don't. Break. Software, unless it's really really really worth it.

The irony in that article from 2004 is that it was wrong about many things. "And yet, people aren’t really using .NET much.", "WinForms is completely stillborn"... Except now they are, VB 6 is dead, WinForms ruled the world, and there are plenty more people that know the .NET BCL in and out than Win32. If anything it shows that we don't really know which things are worth breaking without being able to see into the future. I have a hunch that static/templated code generation in is going to be very useful for both performance as well as security, because it already is in other languages, but I digress...

Regardless, I absolutely agree with not breaking Refit, and fundamentally I don't see any glaring reasons why Refit would have to actually break its API surface in order to accommodate full static source generation, ideally the usage would remain as straightforward and streamlined as it is today. I could be wrong on that though.

crozone avatar Nov 15 '22 07:11 crozone

Hello everyone! I wanted to inquire about the current status of this matter. Previously, there seemed to be a significant community interest, with several individuals expressing their willingness to contribute by submitting pull requests. However, it has come to my attention that the pull request from @kant2002 has been closed. Does this indicate a lack of interest from the library owners?

Personally, I strongly believe that incorporating this feature and marking library with <IsTrimmable>true</IsTrimmable> would be highly beneficial as I utilize Blazor WASM. In this environment, trimming is crucial due to the importance of minimizing the libraries size and saving every byte.

ScarletKuro avatar May 23 '23 16:05 ScarletKuro

My use case is getting Refit into industrial .NET applications which are often targeted towards low power ARM SoCs, usually single core and around 1GHz in speed. .NET is surprisingly performant in steady state, but JIT pauses and deployment size are troublesome. ReadyToRun is an alright stopgap solution but NativeAOT performs excellently and is really the end goal.

I'm also curious as to why https://github.com/reactiveui/refit/pull/1414 was closed. If implementing full source generation is considered too disruptive to the project goals, would it be more feasible to fork/branch off into a separate package which is a ground up source generator first implementation?

crozone avatar Jun 16 '23 06:06 crozone

I'd also find AOT support extremely helpful. I'm deploying several web apis to Azure which use Refit to communicate with each other and other external apis. Since this is a private project and I need to keep my Azure bill low I've set the web apps to shut off when they're inactive. AOT would really help with the cold start experience when calling the apis the first time when they're inactive and need to start up as fast as possible. Also AOT (if I remember correctly) greatly reduces the file sizes of the published apps so that deploying to Azure would be faster and my Github action quotas for build artifact storage would not be exceeded too often.

It would be really awesome if Refit as such an integral tool for so many .Net projects would support this new feature.

damidhagor avatar Jul 09 '23 20:07 damidhagor

I'd also find AOT support extremely helpful. I'm deploying several web apis to Azure which use Refit to communicate with each other and other external apis. Since this is a private project and I need to keep my Azure bill low I've set the web apps to shut off when they're inactive. AOT would really help with the cold start experience when calling the apis the first time when they're inactive and need to start up as fast as possible. Also AOT (if I remember correctly) greatly reduces the file sizes of the published apps so that deploying to Azure would be faster and my Github action quotas for build artifact storage would not be exceeded too often.

It would be really awesome if Refit as such an integral tool for so many .Net projects would support this new feature.

I have develop new Refit that support AOT. Nuget package name is HttpClientFiller

https://www.nuget.org/packages/HttpClientFiller

HttpClient Filler Code Generate in C#

ghostnguyen avatar Jan 22 '24 09:01 ghostnguyen

@ghostnguyen Where's the source code?

crozone avatar Jan 29 '24 01:01 crozone

@ghostnguyen Where's the source code?

The source code/dll need to run your project are:

  • The generated code in your project.
  • HttpFillerAttribute and HttpFillerCore (https://github.com/ghostnguyen/HttpClientFillerIssue/)
GitHub
Contribute to ghostnguyen/HttpClientFillerIssue development by creating an account on GitHub.

ghostnguyen avatar Jan 29 '24 01:01 ghostnguyen

@ghostnguyen Where's the source code?

Seems like the source code of the code generator is not available.

bugproof avatar Feb 14 '24 05:02 bugproof

@ghostnguyen Where's the source code?

Seems like the source code of the code generator is not available.

This will not be required to build and to run your project.

ghostnguyen avatar Feb 14 '24 07:02 ghostnguyen

@ghostnguyen Where's the source code?

Seems like the source code of the code generator is not available.

This will not be required to build and to run your project.

It's not about if it will be required or not. It's about trust and safety. Some people might think it's closed source because it contains some malicious code inside/backdoor/whatever. I think that is the concern here. I mean sure you can still add malicious code with open source libs but it adds a bit of trust when there's a complete src available.

bugproof avatar Feb 14 '24 09:02 bugproof

@ghostnguyen Where's the source code?

Seems like the source code of the code generator is not available.

This will not be required to build and to run your project.

It's not about if it will be required or not. It's about trust and safety. Some people might think it's closed source because it contains some malicious code inside/backdoor/whatever. I think that is the concern here.

It is safe because there is no some malicious code end up in your product. It is one of the advantage of Roslyn.

ghostnguyen avatar Feb 14 '24 09:02 ghostnguyen

@ghostnguyen @bugproof This conversation has nothing to do with Refit, please take it to a more appropriate venue.

anaisbetts avatar Feb 14 '24 10:02 anaisbetts

Lack of trimmer support is the reason I stopped using Refit after my first .NET project years ago. For anything with a UI, my platform of choice is UWP/Uno Platform, and they both make good use of AoT:

  • .NET Native under UWP has never been optional for Microsoft Store apps. Like Sergio mentioned, we can use Resource Directives as a workaround for any non-linker-friendly code, but it's cumbersome for most and confusing for many.
  • Uno Platform under Wasm uses an IL Linker and AoT to improve performance and reduce final package sizes. It's a must for users on mobile browsers.

I'd like to use Refit again, but I can't until refit is trimmer-friendly.

Arlodotexe avatar Apr 09 '24 16:04 Arlodotexe

Lack of trimmer support is the reason I stopped using Refit after my first .NET project years ago. For anything with a UI, my platform of choice is UWP/Uno Platform, and they both make good use of AoT:

* [.NET Native](https://learn.microsoft.com/en-us/windows/uwp/dotnet-native/) under UWP has never been optional for Microsoft Store apps. Like Sergio mentioned, we can use Resource Directives as a workaround for any non-linker-friendly code, but it's cumbersome for most and confusing for many.

* Uno Platform under Wasm uses an [IL Linker](https://platform.uno/docs/articles/features/using-il-linker-webassembly.html) and [AoT](https://platform.uno/docs/articles/external/uno.wasm.bootstrap/doc/runtime-execution-modes.html#profile-guided-aot) to improve performance and reduce final package sizes. It's a must for users on mobile browsers.

I'd like to use Refit again, but I can't until refit is trimmer-friendly.

You can try this library, this Refit like and AoT support. https://www.nuget.org/packages/HttpClientFiller

HttpClient Filler Code Generate in C# - Refit like but support AOT

ghostnguyen avatar Apr 10 '24 08:04 ghostnguyen