-
-
Notifications
You must be signed in to change notification settings - Fork 191
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
[BUG] Possible interoperability problem between .NET Standard 2.1 packages and .NET 7 projects #96
Comments
I said target-framework should be This is documented. |
Hi @neuecc ,
to which you answered:
but also added:
So I went and used the same code.
I missed that part, sorry. Here instead the design is such that if my lib targets .net std 2.1 and uses your lib (which also targets .net std 2.1), it would still not work on a project that targets a version of .net that is compatible with .net std 2.1. This is the first time I saw such approach and I think it breaks the expectations of the users. If you feel like, I would gladly chat about this in more details via DMs or a videocall. |
Oh, sorry, some of my responses were misleading and incorrect.
Yes. However, static abstract members are the first significant runtime-incompatible change in a long time (like Net 1 -> 2 Generics for that matter). Not using static abstract members for compatibility can not choose in this library, for ultimate performance with current .NET. |
Me too! It's a delicate subject and we'll see how it works out in the field with other projects.
Got it, makes sense. Would it be possible though to have them work, like, side-by-side with the standard approach? What I mean is that the generated code for the .net std 2.1 version would work even when running in a .net 7 project? Otherwise as I pointed out the promise of .net std 2.1 <-> .net 7 compatibility would be broken, which is a big surprise for users. I don't know the intricacies of the MemoryPack codebase right now, but by doing some mental reverse-engineering I think the main issue is that internally you are using 2 very different workflows for .net std 2.1 and .net 7, which are not compatible with each other. Maybe a unified workflow with an automatic runtime opt-in switch would be possible? Something like the .net 7 workflow being able to also support a .net std 2.1 formatter or something like this. Thanks for your support. PS: the combo of "new runtime features" + "source generators" we are seeing here for the very first time may highlight a design detail never happened before, and it may lead to some new guidelines for source generator users. It will be interesting to see how it'll go. |
Btw, an update: in the meantime I've updated my lib to explicitly use multi-targeting, and I've switched from grabbing the generated code and copying it manually into a class (previous approach) to the other approach you suggested, which uses external formatters. Now it allocates more (since it has to allocate the serializable class) but it seems to "technically" work. |
Maybe @mgravell , who seems to be experimenting a lot with source generators, can give us some opinions? |
Hi @neuecc , what is changed? |
It seems to have been released and I saw no reason to continue to keep this Issue. |
Hi @neuecc , the issue on my repo was the one to track and solve my users' issue with FusionCache + MemoryPack + .NET 7. This issue instead was meant to discuss the non-standard dependency behaviour of MemoryPack regarding .NET Standard 2.1 and .NET 7. RecapTo quickly recap the issue I think we have here:
ExamplePractical example:
The practical issue is that currently MemoryPack, when it sees it is running in .NET 7, expect all the dependency chain to be .NET 7, as is stated on the repo's README:
This, even though is stated in the README, is still a behaviour that violates the standard compatibility rules of .NET as a whole, and breaks the users' expectations about compatibility. As an additional prove that this violates the .NET compatibility rules, this is my package previous version (which was not working well in a .NET 7 runtime) on the Nuget site where, based on the declared target frameworks, renders like this: As you can see, by applying the standard dependency logic, the Nuget website (and the entire ecosystem of tools, compilers, plugins, etc) is saying that my package explicitly targets .NET Standard 2.1 (dark blue) but is also compatible and can run on all the others shown .NET versions (light blue), including .NET 7. And that was not the case, because of the above issue. It's your projectNow to be clear: of course the project is yours, and you can decide this is the way to go! A possible solutionMy 2 cents about a solution (admittedly without knowing MemoryPack codebase that much): would it be possible for MemoryPack, when running on .NET 7, to be able to work with code generated by the source generator in both a .NET Standard 2.1 AND .NET 7 versions? Meaning that to get the full blown perf you must have all the formatters generated on .NET 7, ok, but it would still be possible for it to work with .NET Standard 2.1 formatters, just in a less performant way? This would:
An additional idea may be to log a message that warns about a possible perf boost opportunity, in case it detects a .NET 7 runtime with some .NET Standard 2.1 formatters, to ease the experience for new users and highlight a path to success. Again just my 2 cents, since I would really like MemoryPack to win big in this space 😊 ps: thanks again for your openness and support, it's not a given! |
This is not a Source Generator issue, but an interface definition issue, because we are using different interfaces (although the names are the same). |
Hi @neuecc , which interface (definition) are you talking about? I'm not understanding, sorry. Basically though, as I tried to clarify above, the main point is this: currently the chain (MemoryPack -> NS 2.1 LIB -> N7 APP) is broken, when instead it should work, and this is against the .NET compatibility guidelines and every supporting tool out there (like Nuget itself). I think it would be better to change the approach as I described above, but of course this is your project so it's up to you. I just hope I've been able to explain myself, maybe something has been lost in translation. Thanks! |
Obviously, the definition of interface is different, isn't it? I have no content to discuss. |
Hi, I opened this issue to highlight that the current design violates the .NET compatibility guidelines, and to discuss possible alternatives. I didn't had a solution ready, since the codebase is quite complex. Anyway since you want content to discuss I can try with some pseudo code (so, NOT actual code) to give you an idea of what I'm talking about. I am thinking about interfaces like these to define formatters: // FOR NS 2.1
public interface IFormatter {
void Serialize(...) {
// ...
}
void Deserialize(...) {
// ...
}
}
#if NET7_0_OR_GREATER
// FOR NET 7
public interface IFormatterNet7 : IFormatter {
// STUFF COMPATIBLE -ONLY- WITH NET 7
static abstract void SerializeNet7(...) {
// ...
}
static abstract void DeserializeNet7(...) {
// ...
}
}
#endif Please note that the The generator can then generate the needed concrete formatter classes (like Then, in the code for the public static MemoryPackSerializer {
// ...
public static byte[] Serialize<T>(...) {
IFormatter f = GetTheFormatterFor<T>();
#if NET7_0_OR_GREATER
if (f is IFormatterNet7 f7) {
f7.SerializeNet7(...);
return;
}
#endif
f.Serialize(...);
return;
}
// ...
} Again, this is just a very simplified example to show you the approach. In this way MemoryPack:
Of course to get the 100% best performance someone would have to multi-target every piece of the chain, so that on NET 7 every formatter would have been generated targeting specifically NET 7, and use the code generated specifically for that. I hope I've been able to show you the approach I'm talking about. And of course: maybe there's a reason why an approach like this is not possible, I don't know, and therefore I'm asking. Thanks. |
if (f is IFormatterNet7 f7) {
f7.SerializeNet7(...);
return;
} This is one of the methods I would like to achieve, but it is not possible with the current C#. |
Hi, sorry but I've been ill for some time now. Probably It's my fault for not having been clear enough, sorry but language barrier or something I suppose.
I don't know what you mean, but anyway yes, you already can (at least what I meant). I prepared a POC (Proof Of Concept) repo here https://github.com/jodydonetti/FooPack to showcase the approach I'm talking about. As said there, it is NOT a working serializer or anything, it's just a proof of concept for the approach. I hope the README is clear enough. If you look at the results here you can see that it never crashes in .NET Standard 2.1 nor in .NET 7, and it always tries to use the best formatter available or it fallbacks to the standard one when a .NET 7 version is not available (so running in the .NET 7 console, it can work with the formatter generated in the .NET Standard 2.1 lib). Hope this helps clarify my approach. Let me know what you think about this, thanks! |
Ah, sorry it is using formatter, not formattable, I misread it. |
Hi @neuecc , and thanks for creating MemoryPack, it's damn good!
A user of my lib has found a runtime problem while using my library, which is a .NET Standard 2.1 package that uses MemoryPack.
What I think is happening here is that I compiled my library while targeting .NET Standard 2.1, since that is a valid target TFM for MemoryPack: this means that the generator did run with that TFM.
But then the user referenced my .NET Standard 2.1 library from a .NET 7 project, and here is where my theory for the problem is: I think that the code you use at runtime is strictly related to the TFM used by the generator, meaning that if a project is targeting .NET 7, than all the code that your generator creates must also target .NET 7.
But the problem is that your generator may have already run for other classes in a referenced package, that targeted .NET Standard 2.1 (which is also valid, and a subset of .NET 7).
By having .NET Standard 2.1 as a valid target you are basically saying that anyone can use the MemoryPack generator and create a package that targets .NET Standard 2.1. But someone else may use that package (which contains the generated code) and use that with a .NET 7 project (which is a superset of .NET Standard 2.1), and this seems to break things.
Is my analysis correct?
Thanks!
The text was updated successfully, but these errors were encountered: