Introduce static methods to allocate and throw key exception types.

[geeknoid]

EDITED 03/06/2021 by @stephentoub to add revised proposal:

public class ArgumentNullException
{
+    public static void ThrowIfNull([NotNull] object? argument, [CallerArgumentExpression("argument")] string? argumentName = null);
}

Background and Motivation

The .NET ecosystem uses extensive argument checking to improve code reliability and predictability. These checks have a substantial impact on code size and often dominate the code for small functions and property setters. This in consumes more RAM, takes more time to JIT, prevents inlining, and most important it causes substantial instruction cache pollution. Ultimately, the presence of these checks slows code down.

Many libraries, including the framework libraries themselves, implement exception throwing helpers to compensate for this bloat. These simple static functions centralize the exception creation and throwing logic. Why not enshrine this pattern in the exception API surface to encourage smaller/faster code, and avoid library authors having to create these stubs themselves?

Proposed API

I propose that the core framework ArgumentXXXException classes be augmented with static functions responsible for both allocating and throwing the exceptions:

public class ArgumentNullException : ArgumentException
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void Throw(string paramName) =>
        throw new ArgumentNullException(nameof(paramName));
}

There would be one static method corresponding to each constructor signature of the exception type.

This pattern would be warranted for any exception type that has a sufficiently large usage footprint. Certainly the ArgumentXXXException types would qualify for this, perhaps a few others.

Usage Examples

With such functions, the following code

public void DoSomething(Foo foo)
{
    if (foo == null) throw new ArgumentNullException(nameof(foo));
}

would become

public void DoSomething(Foo foo)
{
    if (foo == null) ArgumentNullException.Throw(nameof(foo));
}

Analyzer and Fixer or Compiler Voodoo

It would be trivial to include an analyzer and associated fixer to upgrade a code base to the new approach, thus encouraging a rapid migration.

Alternatively, the C# compiler could potentially be upgraded to automatically replace canonical uses into calls to the static method which wouldn't require any code changes to yield the perf benefits.

Generated Code

The code required to create and throw an exception costs > 70 bytes of instructions. Here is an example null check compiled in release mode for .NET 5:

00007FFADA6D510B 48837D1800           cmp     qword ptr [rbp+18h],0
00007FFADA6D5110 7541                 jne     short LBL_0
00007FFADA6D5112 48B980C974DAFA7F0000 mov     rcx,offset methodtable(System.ArgumentNullException)
00007FFADA6D511C E83F26B25F           call    CORINFO_HELP_NEWSFAST
00007FFADA6D5121 488945A0             mov     [rbp-60h],rax
00007FFADA6D5125 B901000000           mov     ecx,1
00007FFADA6D512A 48BA70DB88DAFA7F0000 mov     rdx,7FFADA88DB70h
00007FFADA6D5134 E8C7B3C45F           call    CORINFO_HELP_STRCNS
00007FFADA6D5139 48894598             mov     [rbp-68h],rax
00007FFADA6D513D 488B5598             mov     rdx,[rbp-68h]
00007FFADA6D5141 488B4DA0             mov     rcx,[rbp-60h]
00007FFADA6D5145 E8D61BFEFF           call    System.ArgumentNullException..ctor(System.String)
00007FFADA6D514A 488B4DA0             mov     rcx,[rbp-60h]
00007FFADA6D514E E87D62AE5F           call    CORINFO_HELP_THROW
LBL_0:
00007FFADA6D5153 488B5510             mov     rdx,[rbp+10h]

[jkotas]

Note that this pattern is not a clear win today: #47353 (comment)

I think it would make sense to introduce this method to support the planned null checking auto-generated by the compiler. cc @jaredpar @stephentoub

[geeknoid]

There are a couple things relative to #47353:

• That PR talks about the impact to the IL size. I think for most uses, IL size doesn't matter to the runtime of the code. I am much more concerned by the size of the native code produced by the JIT. This has a real RAM cost, and impacts execution performance in a measurable way. But I believe my proposal here would also result in smaller IL overall.

• That PR references the idea of auto-generated ModuleThrowHelpers, whereas I'm proposing to add those static methods directly into the exception types. That makes them shared between all assemblies in a process, so the gains will be substantial overall, both in total IL size and in native code size. Additionally, the pattern I propose here is considerably more discoverable. And heck, it's ever fewer characters to type in the source as well :-)

[jkotas]

more concerned by the size of the native code produced by the JIT. This has a real RAM cost

I agree. Switching inline throw new ArgumentNullException to use this API everywhere would likely increase the RAM cost due to optimizations for the inline throw pattern in the JIT (you have to count the code size and also size of the supporting datastructures). It can be fixed with some work in the JIT.

In other words, this API sounds like a good idea, but turning it into actual RAM win likely requires work in the JIT to make it as efficient as the inline throw pattern.

[jkotas]

@EgorBo Interested in looking into this?

[EgorBo]

@jkotas it's not clear to me what is missing from the JIT side
These new Throw-helpers will be imported as cold-blocks. Are you referring to the LazyStringHelper optimization?
It can be enabled for all cold blocks: https://github.com/dotnet/runtime/blob/master/src/coreclr/jit/morph.cpp#L9472

[sharwell]

@jkotas It might not be a big win from generated code or IL size in applications, but it's a very big win for localization of common validation scenarios. Localization of ThrowHelper is the largest impediment to distributing Microsoft.CodeAnalysis.Collections today.

[stephentoub]

Localization of ThrowHelper is the largest impediment to distributing Microsoft.CodeAnalysis.Collections today.

I don't understand. What in ThrowHelper itself needs to be localized, and how does that impact whether code uses a ThrowHelper-like pattern or not?

[sharwell]

If ThrowHelper was a public API, we could use it directly and not need to include localized messages in Microsoft.CodeAnalysis.Collections. Since ThrowHelper isn't public, we had to port it over as part of the collections library and handle provide all the localized resources. Since the package is distributed as a source package, we have to distribute these resources as part of the source package, and it's difficult to incorporate those localized resources as part of downstream builds.

The point of this is less that we need ThrowHelper to be a public API, but rather the value we would gain from having ThrowHelper be a public API isn't derived from the specific points being considered previously (IL gains, perf, etc.).

[stephentoub]

That's very different from what's being proposed here. You're actually asking for public APIs for resources Corelib uses?

[sharwell]

@stephentoub looks like I got mixed up between the two proposals when following links and seeing comments like #47353 (comment)

[danmoseley]

Could it even be ArgumentNullException.ThrowIfNull (with the test and the throw internally in separate methods so the test could be inclined)

[EgorBo]

Could it even be ArgumentNullException.ThrowIfNull (with the test and the throw internally in separate methods so the test could be inclined)

For performance reasons it must be a "never returns" type of method to be recognized by jit as a "throw helper", otherwise it might be inlined, e.g. https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKBpuIGYACMpgYSYG8aneme+jFiiYAVABZQIAdwCSAMwByAVwA2qgBQB5YACsYYDEwgBKAb27U+1pgEt5TDRDu4mAOzWqTTDJJnuYaSYAUQQwGAAHDFsINw0TAG5zJgBfZOShYhEJKWl45MsbPl9cgKDQ8KiYuMTktKs+dIbeTOyYXAxybT0DI1MC5Ot7R2cAXlH3TzNmor4cmXiEpgB6ZaZYSABzN1sALxgAEyY8Y6YAIhL/cRhVCJgoM5d3CCNbN1U3w7QmSFUDuGAqggYAA1nUmtZWmJ2hhSN19IZjNNrIVZvM5EpPE5Eis1r4YExpOIIKoCfgYL4IEdbK43h83IcmNdYHY3GBVMoDm9NudLkEGdIzuDqCkgA=

[danmoseley]

@EgorBo your example has the test and the throw in the same method.

[EgorBo]

@danmoseley see Test2

[danmoseley]

I'm not at my computer so I may be missing something, but what I meant is

// user code
ANE.ThrowIfNull(arg, nameof(arg));

// ANE
public ThrowifNull(object arg, string argname) // inlines 
{
   if (arg is null)
       Throw(argname);
}

private Throw(string argname) // not inlined 
{
    throw new ANE(..);
}

[jkotas]

Are you referring to the LazyStringHelper optimization?

Yep.

It can be enabled for all cold blocks

It should be enabled for all paths that are guaranteed to end up throwing exception. Cold blocks do not necessarily end up throwing exceptions, they are just executed less frequently, but they may be still pretty performance sensitive and the lazy string optimizations may be unacceptable overhead in some cold blocks. It is what the TODO that you linked to is about.

[EgorBo]

I'm not at my computer so I may be missing something, but what I meant is

// user code
ANE.ThrowIfNull(arg, nameof(arg));

// ANE
public ThrowifNull(object arg, string argname) // inlines 
{
   if (arg is null)
       Throw(argname);
}

private Throw(string argname) // not inlined 
{
    throw new ANE(..);
}

Ah, I see - yeah it should work

[stephentoub]

What is the exact API proposal here at this point?

Note that a ThrowIfNull method won't play well with nullable reference types. We'd need a DoesNotReturnIf attribute that supports null rather than just bool.
cc: @jcouv, @jaredpar

Actually, I take that back, we should be able to make the argument [NotNull] object?.

So the proposal at this point is just:

public class ArgumentNullException
{
    public static void ThrowIfNull([NotNull] object? argument, string argumentName);
}

? And then presumably the C# compiler would use this if available to implement !!. (The method could also use CallerArgumentExpression if that ever exists.)

[stephentoub]

This proposal is also another form of #20604.

[stephentoub]

(It'd be nice if the linker could optionally recognize this as well, to strip out the argument name parameter in a release app build where these are never expected, anyway, and are just for debugging purposes.)

[geeknoid]

@jkotas How could introducing these simple non-inlinable methods increase RAM usage? Considering that every current inline use of "throw new ArgumentNullException(nameof(arg))" costs 72 bytes of jitted RAM, cutting that down to a dozen bytes repeated across hundreds of call sites is bound to reduce overall RAM usage.

I don't think you want ThrowIfNull since that makes the call site considerably less efficient. I want the null check at the call site, and the failure handling code in the stub helper function. That keeps the mainline code as small and efficient as possible, and moves the slow never-used error handling code out of the way.

[stephentoub]

since that makes the call site considerably less efficient

ThrowIfNull will get inlined; how does it make the call site less efficient?

[jkotas]

"throw new ArgumentNullException(nameof(arg))" costs 72 bytes of jitted RAM, cutting that down to a dozen bytes repeated across hundreds of call sites is bound to reduce overall RAM usage.

It costs 72 bytes of jitted RAM, but avoids allocating the string constant eagerly. The string literal constant will be allocated lazily that almost never happens on exception throwing paths.

If you change it to non-inlineable helper method, the string constant object will be allocated eagerly that will cost at least:

• The string object itself. 28 bytes
• Handle to keep the string object alive: 8 bytes
• Space in the string interning hashtable that keeps track of all string literals (StringLiteralEntry): ~20 bytes
• Ongoing GC cost to visit these never used strings

As I have said, this is can be fixed. And the overhead may be amortized over multiple callsites if they use the same argument name.

[geeknoid]

@stephentoub

The point of this proposal was to move the code that allocates and throws an exception out of the main hot code path. This is why my code example showed the Throw method as being marked for non-inlinable. The proposal I showed means that in the main code you're left with:

A trivial null check
A branch to continue execution in the function later
A branch to the Throw method

And the 72 bytes of code to create and throw an exception is kept out of the hot path.

[stephentoub]

The point of this proposal was to move the code that allocates and throws an exception out of the main hot code path.

I understand. This achieves that:

public static void ThrowIfNull([NotNull] object? argument, string argumentName)
{
    if (argument is null)
        Throw(argumentName);
}

private static void Throw(string argumentName) =>
    throw new ArgumentNullException(argumentName);

with less IL at each call site but the same generated asm at each call site. ThrowIfNull gets inlined and Throw does not.

e.g.
https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8BiAOwFcAbS7YSmAAhlNvoFgAoAAQCYBGTlwAMDLnwB0AEQCW2AOakIuDNLC5xAYQgATGAEEWlAJ65puANydBAZlE8GGhgG9ODN6NtcUDACoxlfAAUEMAAVjBgGAzYUHIAlK7uLhzuqQzSAGYMgTFy6bgMFNQJKWllPgAWUBAA7oEs+DAQGTmxcXGWpW4AvolufR6i3n7KPMFhEVG5JanJZW6V1TUAkhkAclSUrXJohdiNzdvtnam9XQwDAA5Q0gBu2BiMYkhDvlW1qxvUgQDaaxAYL6UAC6DBC4UiAH5orFyI1SBhdmIRLk4cxAfsYDMkgNUplsqj4VEzIVNtj5u5FrVtmiEWtMR0Bmcrjd7o9RHwXl43ktAsiYXJaRjGnEGABeAB8uPcGHeNUKMHlelhRKBAFEEGAYJcVBBSDTVQzOt0gA===

#nullable enable
using System;
using System.Diagnostics.CodeAnalysis;

public class C {
    public void Test1(object arg)
    {
        if (arg is null)
            Throw(nameof(arg));
    }
    
    public void Test2(object arg)
    {
        ThrowIfNull(arg, nameof(arg));
    }
    
    private static void ThrowIfNull([NotNull] object? argument, string argumentName)
    {
        if (argument is null)
            Throw(argumentName);
    }

    private static void Throw(string argumentName) =>
        throw new ArgumentNullException(argumentName);
}

produces smaller IL for Test2:

    .method public hidebysig 
        instance void Test1 (
            object arg
        ) cil managed 
    {
        // Method begins at RVA 0x208e
        // Code size 14 (0xe)
        .maxstack 8

        IL_0000: ldarg.1
        IL_0001: brtrue.s IL_000d

        IL_0003: ldstr "arg"
        IL_0008: call void C::Throw(string)

        IL_000d: ret
    } // end of method C::Test1

    .method public hidebysig 
        instance void Test2 (
            object arg
        ) cil managed 
    {
        // Method begins at RVA 0x209d
        // Code size 12 (0xc)
        .maxstack 8

        IL_0000: ldarg.1
        IL_0001: ldstr "arg"
        IL_0006: call void C::ThrowIfNull(object, string)
        IL_000b: ret
    } // end of method C::Test2

and identical asm for Test1 and Test2:

C.Test1(System.Object)
    L0000: test rdx, rdx
    L0003: jne short L0017
    L0005: mov rcx, 0x14fc806f730
    L000f: mov rcx, [rcx]
    L0012: jmp C.Throw(System.String)
    L0017: ret

C.Test2(System.Object)
    L0000: test rdx, rdx
    L0003: jne short L0017
    L0005: mov rcx, 0x14fc806f730
    L000f: mov rcx, [rcx]
    L0012: jmp C.Throw(System.String)
    L0017: ret

[EgorBo]

#50112 and #48589 improved jit codegen for these new kind of throw helpers.

[geeknoid]

My initial proposal above was to introduce methods such as:

public class ArgumentNullException : ArgumentException
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void Throw(string paramName) =>
        throw new ArgumentNullException(nameof(paramName));
}

public class ArgumentOutOfRangeException : ArgumentException
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void Throw(string paramName) =>
        throw new ArgumentOutOfRangeException(nameof(paramName));
}

which is a general pattern that can apply to any exception type. @stephentoub advocated instead for:

public class ArgumentNullException
{
    public static void ThrowIfNull([NotNull] object? argument, [CallerArgumentExpression("argument")] string? argumentName = null);
}

which is considerably less generic. It doesn't directly translate to other types of exceptions. For example, what would be the equivalent stub for ArgutmentOutOfRangeException?

I believe leaving the conditional check in the original code is more generally applicable and would serve the framework better as a generalized pattern.

At the call site you end up with:

void Foo(object o)
{
    _ = o ?? ArgumentNullException.Throw(nameof(o));
}

as opposed to:

void Foo(object o)
{
   ArgumentNullException.ThrowIfNull(o, nameof(o));
}

[stephentoub]

For example, what would be the equivalent stub for ArgutmentOutOfRangeException?

I don't think we should have this for most other exception types. In many such situations the arguments to the exception isn't just a const, e.g. you're passing a resource message to ArgumentOutOfRangeException, and if the goal is to keep the calling code lean/small, that resource access should also be part of whatever throw helper you're using to factor out the exception-related code. I called out ArgumentNullException.ThrowIfNull because it's the vastly most common case and generally doesn't suffer from that issue.

_ = o ?? ArgumentNullException.Throw(nameof(o));

FWIW, that's not valid C#, with Throw being void-returning. (And even if it were, I personally find that construction to be cumbersome.)

[terrajobst]

• Let's scope this to just ArgumentNullException
• This API might be less useful with C#' upcoming parameter null-validation feature
  • Still useful for other languages or potentially useful for letting the compiler use a centralized method
  • Also useful in cases the compiler injected validation wouldn't work, e.g. due to ordering
• We can do it for .NET 6 'cause it's cheap
• We might be able to mark this API as "doesn't change between versions" so that there is no penalty for ready-to-run

namespace System
{
    public partial class ArgumentNullException
    {
        public static void ThrowIfNull([NotNull] object? argument,
                                       [CallerArgumentExpression("argument")] string? paramName = null);
    }
}

[xtqqczze]

This API might be less useful with C#' upcoming parameter null-validation feature

For reference: dotnet/csharplang#2145

[EgorBo]

Does arg! feature emit "throw new" or a call to this helper? (I hope the latter)

[stephentoub]

It will emit a call to a ThrowIfNull method identical to this one but injected as internal into the assembly that's using it. As some point once we better address cross-module inlining with R2R, the compiler might switch to using this method rather than having one per assembly.

[danmoseley]

Do we need an issue open to take advantage of this new method throughout our repo?

[stephentoub]

Do we need an issue open to take advantage of this new method throughout our repo?

No, see #55594 (comment).

[danmoseley]

Ah - I missed that, perhaps I searched only issues.

I marked #62628 as ready for review.