dotnet / runtimelab

This repo is for experimentation and exploring new ideas that may or may not make it into the main dotnet/runtime repo.
MIT License
1.42k stars 199 forks source link

Delete --removefeature ILC command line option #446

Closed MichalStrehovsky closed 3 years ago

MichalStrehovsky commented 3 years ago

We should be able to replace this with the "official" feature switches: #422

This item tracks:

MichalStrehovsky commented 3 years ago

One thing that is left before we can delete this command line option is --removefeature:Comparers. I can't decide about the right course of action.

The current mechanism is a bit inconsistent. It rewrites the fallback method body, which means that if Comparer/EqualityComparer<Foo>.Default is statically known, the compiler will still do the method body rewriting and produce a "good" comparer (one that respects whether Foo implements IComparable<T>/IEquatable<T>). If we don't know the answer statically (because we're e.g. in shared code), it will do the fallback and the fallback with --removefeature:Comparers will ignore the generic interfaces.

We could make this fully consistent (never respect the generic interfaces), but this code also interacts with RyuJIT (RyuJIT knows what concrete type get_Default is supposed to return), so it can't just be done with ILLinkTrim files. Being fully consistent would also means we get boxing for valuetypes, which is pretty bad.

This would push me towards just won't fixing this and dropping the feature.

But on the other hand, a reflection-free Hello World is currently 910 kB (with --removefeature:Comparers). Once we drop that argument, the size jumps to 1,370 kB because of all the type loader junk to load the appropriate comparer types.

jkotas commented 3 years ago

--removefeature:Comparers. I can't decide about the right course of action.

If we care enough, I think we can preserve the current inconsistent behavior with low cost by ILLinkTrim files and a property to tell that we are in the reflection-free mode. Something like: ReflectionFreeMode ? new ObjectComparer<T>() : ... the type loader path ....

Also, is there a replacement for XmlDownloadNonFileStream?

MichalStrehovsky commented 3 years ago

Fixed in #797.