Runtime: Need to teach IL linker to strip `[Nullable]` attribute from implementation assemblies

[stephentoub]

It appears the [Nullable(…)] attributes generated by the compiler for the nullable annotations aren't being stripped out by the linker. Having annotated ~25% of corelib, it appears the attributes are contributing around a 2% size increase for the assembly. We need to teach the linker to remove the attributes from the implementation assembly for shipping.

cc: @jaredpar, @dotnet/nullablefc, @jkotas, @ericstj

[jaredpar]

Is there a reason we'd stop with only [Nullable]? If we're doing this work I think it would make sense to strip out all compiler metadata: dynamic, readonly, etc ...

[RussKeldorph]

@jeffschwMSFT @sbomer @swaroop-sridhar

[stephentoub]

Is there a reason we'd stop with only [Nullable]?

No, we could do that, too (I hope we don't use dynamic anywhere in corelib, but we certainly have IsReadOnlyAttributes). I only noticed this in the context of [Nullable], which seems like it'll end up on pretty much everything in the assembly.

[jeffschwMSFT]

Our initial goal for the linker is to enable it as part of the sdk by default. I have initially marked this issue for consideration for .NET Core 3.0. For my knowledge, the linker that corefex uses is a private copy in the toolset, correct?

[jkotas]

I do not think readonly and nullable are same category:

• readonly is part of the type system (for some of the definition of type system). It affects binding, existing dynamic binders are aware of them. If you strip them in dependencies, your program will no longer compile - there is no way to ignore them.
• Nullable annotations are in the same family as SuppressMessageAttribute or ObsoleteAttribute. They are pure compiler diagnostic. If you strip them in dependencies, your program will still compile fine as long as you pass all the right command line options to ignore them.

[stephentoub]

readonly is part of the type system (for some of the definition of type system)

That's true, but if we're talking about implementation assembly, then it shouldn't affect dependencies...

…except that I just remembered we actually do have some dependencies in corefx that bind directly against the implementation assembly rather than via the ref assembly...

...which will affect nullable as well.

[sbomer]

@jeffschwMSFT coreclr and corefx depend directly on a specific version of the linker:

• Coreclr: https://github.com/dotnet/coreclr/blob/769850c7426c1e358157e288e04ebe3e6b391467/dependencies.props#L58
• Corefx: https://github.com/dotnet/corefx/blob/2be1075a381c11330907392951813c7bbab8b1ad/external/ILLink/ILLink.depproj#L7

[safern]

What about corefx libraries that have a reference from runtime into corelib from the nullability perspective, they would loose all the nullable annotations from corelib so the data flow analysis would be incomplete when annotating those libraries?

EDIT

...which will affect nullable as well.
😄 commented at the same time.

[jaredpar]

@terrajobst will this affect the desire to have nullability surfaced in the reflection APIs?

[jkotas]

What about corefx libraries that have a reference from runtime into corelib from the nullability perspective,

We would need to include two copies of CoreLib in the transport package: One with the annotations to compile the CoreFX against (verify that the annotations in refs match implementation, etc.), and second one without the annotations that we actually ship.

[stephentoub]

@jaredpar, related but separate, is it also possible for the compiler to emit less attribution? e.g. why does it emit attributes on privates... is that in order to enable reflection as you suggest?

For reference, for this input (and, yes, I can count, I just deleted the method I'd named with '3' :-)
```C#

nullable enable

public class Program
{
public static void Main() { }

public static object PublicMethod1() => new object();
public static object? PublicMethod2() => null;
public static void PublicMethod4(object o) { }
public static void PublicMethod5(object? o) { }
public static void PublicMethod6(object o1, object o2, object? o3, object? o4) { }

private static object PrivateMethod1() => new object();
private static object? PrivateMethod2() => null;
private static void PrivateMethod4(object o) { }
private static void PrivateMethod5(object? o) { }
private static void PrivateMethod6(object o1, object o2, object? o3, object? o4) { }

}

this is what I see in the IL:

.class public auto ansi beforefieldinit Program
extends [System.Runtime]System.Object
{
.method public hidebysig static void Main() cil managed
{
.entrypoint
// Code size 1 (0x1)
.maxstack 8
IL_0000: ret
} // end of method Program::Main

.method public hidebysig static object
PublicMethod1() cil managed
{
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
// Code size 6 (0x6)
.maxstack 8
IL_0000: newobj instance void [System.Runtime]System.Object::.ctor()
IL_0005: ret
} // end of method Program::PublicMethod1

.method public hidebysig static object
PublicMethod2() cil managed
{
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
// Code size 2 (0x2)
.maxstack 8
IL_0000: ldnull
IL_0001: ret
} // end of method Program::PublicMethod2

.method public hidebysig static void PublicMethod4(object o) cil managed
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
// Code size 1 (0x1)
.maxstack 8
IL_0000: ret
} // end of method Program::PublicMethod4

.method public hidebysig static void PublicMethod5(object o) cil managed
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
// Code size 1 (0x1)
.maxstack 8
IL_0000: ret
} // end of method Program::PublicMethod5

.method public hidebysig static void PublicMethod6(object o1,
object o2,
object o3,
object o4) cil managed
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param [2]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param [3]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.param [4]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
// Code size 1 (0x1)
.maxstack 8
IL_0000: ret
} // end of method Program::PublicMethod6

.method private hidebysig static object
PrivateMethod1() cil managed
{
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
// Code size 6 (0x6)
.maxstack 8
IL_0000: newobj instance void [System.Runtime]System.Object::.ctor()
IL_0005: ret
} // end of method Program::PrivateMethod1

.method private hidebysig static object
PrivateMethod2() cil managed
{
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
// Code size 2 (0x2)
.maxstack 8
IL_0000: ldnull
IL_0001: ret
} // end of method Program::PrivateMethod2

.method private hidebysig static void PrivateMethod4(object o) cil managed
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
// Code size 1 (0x1)
.maxstack 8
IL_0000: ret
} // end of method Program::PrivateMethod4

.method private hidebysig static void PrivateMethod5(object o) cil managed
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
// Code size 1 (0x1)
.maxstack 8
IL_0000: ret
} // end of method Program::PrivateMethod5

.method private hidebysig static void PrivateMethod6(object o1,
object o2,
object o3,
object o4) cil managed
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param [2]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param [3]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.param [4]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
// Code size 1 (0x1)
.maxstack 8
IL_0000: ret
} // end of method Program::PrivateMethod6

.method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
IL_0006: ret
} // end of method Program::.ctor

} // end of class Program
```

[jaredpar]

@stephentoub

e.g. why does it emit attributes on privates... is that in order to enable reflection as you suggest?

Simplicity of implementation, lack of motivating reason to change. There is no functional reason for the compiler to emit them, or other compiler attributes, on private members. If there was a significant reasaon to do so and we decided against reflection I believe it would be fine to omit them in some cases.

[Tornhoof]

Would stripping the Attributes affect the methods described by @RicoSuter in https://blog.rsuter.com/the-output-of-nullable-reference-types-and-how-to-reflect-it/?

[stephentoub]

Would stripping the Attributes affect the methods described

Yes, removing the attributes would remove the ability to reflect over the attributes. That's what @jaredpar was asking about earlier.

[stephentoub]

If we decide not to strip nullable attributes on exported members, we should at least do so for internals/privates.

[terrajobst]

@jaredpar

I understand why the compiler needs to emit [Nullable] attribute on all types, due to the fact that we need to differentiate between oblivious and non-null (and #pragma can be anywhere). However, once the entire module is annotated (i.e. the global nullable option is on), could the compile instead mark the asssembly module as annotated? This way, we'd only need to annotate cases where the declaration differs from the global default, which should now much fewer cases. That would cause much less bloat and be simpler to use than IL linker.

[jaredpar]

@terrajobst

Sure the implementation could be tweaked various ways based on usage patterns. It doesn't even need to be the global setting. It is more of a question of:

Which is more prevalent: annotations or lack of annotations

The code generation strategy can be tweaked either direction based on that.

[stephentoub]

That would cause much less bloat and be simpler to use than IL linker.

Even if we reduce the "bloat", we should still at least remove the unnecessary remaining attributes with the linker, as we do with other dead code. Unnecessary may just be internal/private if the compiler persists in generating those.

[terrajobst]

Also, are we sure removing them is the right thing? It seems preserving them would be useful for reflection, especially in customer code.

[stephentoub]

are we sure removing them is the right thing? It seems preserving them would be useful for reflection, especially in customer code.

Why would the attributes on internals/privates in corelib be important? As an option in the linker, we could turn it on for corelib and everyone else could choose.

[RicoSuter]

Would stripping the Attributes affect the methods described by @RicoSuter in https://blog.rsuter.com/the-output-of-nullable-reference-types-and-how-to-reflect-it/?

Stripping attributes on private/internal APIs should not be a big issue in general as I will only use them on public/protected APIs (not sure however if others are interested in reflecting private/internal APIs too and then this information would be missing).

But if you want to reduce the amount of generated attributes, e.g. by defining a module-level attribute which then changes the default for all places in the module, e.g. to not nullable by default (i.e. no attribute means not nullable for the type and all generic arguments) then I'd need to add this rule to my library too - but this described case (module-level switch only) should be straight forward to implement...

[davidwrighton]

Fyi, recent investigations about application startup performance have discovered that these attributes are contributing to slowdown on application startup. If we can remove most of these from the implementation assembly, there may be as much as a ~0.5% or so performance win for startup of some applications. I'm looking at whether or not we can mitigate this with some ready to run goodness and some delicate tweaks to the metadata api, but there is measurable perf impact here.

[danmosemsft]

@jaredpar is there a separate issue (in the Roslyn repo) for the compiler to be more parsimonous in its attribute writing? There seem to be several opportunities to write less, and the linker can't remove them all.

[danmosemsft]

cc @safern

[jaredpar]

@danmosemsft

At the moment no there is no issue.

There seem to be several opportunities to write less,

Where has that been specified? To our understanding these are all necessary unless we want to cut runtime inspection of nullability as a scenario. That's fine to make as a decision but at the moment my understanding is it's still on the table.

[tannergooding]

To our understanding these are all necessary unless we want to cut runtime inspection of nullability as a scenario.

I would imagine this is useful for things like dynamic code compilation (expression trees, IL emit, etc).

[stephentoub]

I think what @danmosemsft meant was exploring options for changing how the attributes are expressed, still with the same expressiveness, but in a way that's potentially smaller. For example, outputting a default at the assembly level that applies to everything and is set based on what attribution is most commonly needed, and then only outputting attributes on things that differ and need to override that default.

[danmosemsft]

That's right

runtime inspection of nullability as a scenario.

Was that scenario an originally enumerated goal, or is it hypothetical?

[jaredpar]

@danmosemsft

Was that scenario an originally enumerated goal, or is it hypothetical?

This is a scenario @terrajobst was originally brought up around System.Type and understanding nullability. It was also echo'd by the entity framework team.

I think what @danmosemsft meant was exploring options for changing how the attributes are expressed, still with the same expressiveness, but in a way that's potentially smaller.

Gotcha. If there are patterns we think could be improved I'm happy to have us look into it. I'm a bit skeptical we'll get huge wins here though.

[safern]

Would a flag set at the project level make sense to tell the compiler to not emit attributes for internal/private APIs? Since those are not really needed for the analysis. I guess those are emitted for internals visible to, reflection and dissassembly scenarios?

[sbomer]

The linker already has a feature that is able to strip attributes whose types aren't used. I think the work that needs to be done to make this usable is to determine which attributes are important to keep. I tried running the linker with attribute stripping on a recent System.Private.Corelib.dll, rooting public surface area, and the following attributes were removed:

-Microsoft.CodeAnalysis.EmbeddedAttribute
-System.Runtime.CompilerServices.IntrinsicAttribute
-System.Runtime.CompilerServices.NullableAttribute
-System.Runtime.CompilerServices.TypeDependencyAttribute
-System.Runtime.Versioning.NonVersionableAttribute
-System.Security.SecuritySafeCriticalAttribute
-System.Security.UnverifiableCodeAttribute

[jkotas]

There is active discussion about what we want to do here. https://github.com/dotnet/roslyn/pull/36152 is part of it. @sbomer If you plan to do any actual work here, coordinate it with @cston

[sbomer]

Thanks for the pointer @jkotas. It's my understanding that @cston's proposal will replace most of the NullableAttributes currently in corelib with NullableContextAttribute at the type level, which sounds like it will mitigate most of the size impact. As such I don't think we should set up a separate build of SPC with the attributes stripped. If that's not the case I can continue looking into this.

[jkotas]

I agree with that building two different flavors of CoreLib is not worth it.

We still care about stripping the nullable annotations on private members in the one-and-only CoreLib. The discussions are still ongoing on whether the C# compiler will support it or not. If the C# compiler does not support it, we would still want to strip them using the linker.

[stephentoub]

This can be closed now that the C# compiler is providing the option to not emit this metadata for internals/privates.