[API Proposal]: ArgumentNullException.ThrowIfNull<T>

Background and motivation

While working on a generic-class, that accepts both classes and structs, I noticed that 'ArgumentNullException.ThrowIfNull' performs a boxing-operation by converting the passed-variable to 'object?'.

The immediate notice is that all structs are boxed but, a seemingly unintended side-effect, is that 'struct?' is also boxed if its not null.

I would like to propose using generic-method functionality instead. This would shave the unnecessary boxing operations, for struct and 'struct?', which could be expensive.

API Proposal

namespace System;

public class ArgumentNullException : ArgumentException
{
    // SNIPPED

    public static void ThrowIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null)
    {
        if (argument is null)
        {
            Throw(paramName);
        }
    }

    // SNIPPED
}

API Usage

// Under the current-API, these will be boxed
int valueStruct = 25;
int? maybeNullStruct = 30;

// Under the current-API, this will not be boxed
int? definitelyNullStruct = null;

// When using the *new-API* ThrowIfNull<T>, none of these will be boxed
ArgumentNullException.ThrowIfNull(valueStruct);
ArgumentNullException.ThrowIfNull(maybeNullStruct);
ArgumentNullException.ThrowIfNull(definitelyNullStruct);

Alternative Designs

Creation of a separate-API, ArgumentNullException.ThrowGenericIfNull:

namespace System;

public class ArgumentNullException : ArgumentException
{
    // SNIPPED

    public static void ThrowGenericIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null)
    {
        if (argument is null)
        {
            Throw(paramName);
        }
    }

    // SNIPPED
}

Risks

Before making this issue, I looked extensively for resources showing the costs of generics and boxing; and could not find anything showing generic-methods being a worse option. However I also am not working directly on the runtime. Nor am I staring at C# every waking hour… Well I do actually, but you get what I mean.

There is the possibility of a performance-regression if my knowledge of the situation wasn’t sufficient.

I’ve also noticed that the runtime sparingly uses generic-methods in non-generic classes. This may be a styling-guideline not mentioned in the contribution-guide.

There is the possibility of a styling-conflict, that wasn’t explicitly mentioned prior.

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

While working on a generic-class, that accepts both classes and structs, I noticed that 'ArgumentNullException.ThrowIfNull' performs a boxing-operation by converting the passed-variable to 'object?'.

The immediate notice is that all structs are boxed but, a seemingly unintended side-effect, is that 'struct?' is also boxed if its not null.

I would like to propose using generic-method functionality instead. This would shave the unnecessary boxing operations, for struct and 'struct?', which could be expensive.

API Proposal

namespace System;

public class ArgumentNullException : ArgumentException
{
    // SNIPPED

    public static void ThrowIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null)
    {
        if (argument is null)
        {
            Throw(paramName);
        }
    }

    // SNIPPED
}

API Usage

// Under the current-API, these will be boxed
int valueStruct = 25;
int? maybeNullStruct = 30;

// Under the current-API, this will not be boxed
int? definitelyNullStruct = null;

// When using the *new-API* ThrowIfNull<T>, none of these will be boxed
ArgumentNullException.ThrowIfNull(valueStruct);
ArgumentNullException.ThrowIfNull(maybeNullStruct);
ArgumentNullException.ThrowIfNull(definitelyNullStruct);

Alternative Designs

Creation of a separate-API, ArgumentNullException.ThrowGenericIfNull:

namespace System;

public class ArgumentNullException : ArgumentException
{
    // SNIPPED

    public static void ThrowGenericIfNull<T>([NotNull] T? argument, [CallerArgumentExpression("argument")] string? paramName = null)
    {
        if (argument is null)
        {
            Throw(paramName);
        }
    }

    // SNIPPED
}

Risks

Before making this issue, I looked extensively for resources showing the costs of generics and boxing; and could not find anything showing generic-methods being a worse option. However I also am not working directly on the runtime. Nor am I staring at C# every waking hour… Well I do actually, but you get what I mean.

There is the possibility of a performance-regression if my knowledge of the situation wasn’t sufficient.

I’ve also noticed that the runtime sparingly uses generic-methods in non-generic classes. This may be a styling-guideline not mentioned in the contribution-guide.

There is the possibility of a styling-conflict, that wasn’t explicitly mentioned prior.

Author:	meg-io
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, untriaged
Milestone:	-

[stephentoub]

I noticed that 'ArgumentNullException.ThrowIfNull' performs a boxing-operation by converting the passed-variable to 'object?'.

You noticed that where? As long as ThrowIfNull is inlined, which it should be in general, the JIT is good at eliding that boxing in optimized code. If you're seeing boxing allocation coming from ThrowIfNull while running under a profiler, I expect you're likely profiling unoptimized code (e.g. tier 0 code that hasn't been optimized yet, or a debug build). If this statement is just based on boxing instructions in the IL, that's not necessarily reflective of what actually happens at run time based on optimizations.

You can see this, for example, with a repro like:

using System.Runtime.CompilerServices;

var c = new C<int>();
for (int i = 0; i < 1_000_000_000; i++)
{
    c.DoWork(i);
}

class C<T>
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public void DoWork(T item)
    {
        ArgumentNullException.ThrowIfNull(item);
    }
}

If ThrowIfNull actually resulted in boxing, this should end up allocating a billion Int32 boxes. But an allocation profile instead shows these as the Int32-related allocations (for the whole program, including work performed by the runtime during startup):

Notice the distinct lack of a billion boxes 😄

There is the possibility of a performance-regression if my knowledge of the situation wasn’t sufficient.

There are a few reasons generics can be worse here. One of the main ones is that this API is intended to be used primarily with reference types, and it's supposed to be a really cheap helper that's invariably inlined and thus no worse performance-wise than if you wrote the longer if (arg is null) throw new ArgumentNullException(nameof(arg)); out manually. But because of runtime limitations, there are situations where it won't end up getting inlined with reference types, meaning every such arg validation would incur the cost of a method call, plus the associated costs associated with runtime generic dictionary lookup. You can see this with a benchmark like this:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Runtime.CompilerServices;

[MemoryDiagnoser(false)]
public partial class Program
{
    static void Main(string[] args) => BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);

    private OuterGeneric1<object> _1 = new OuterGeneric1<object>();
    private OuterGeneric2<object> _2 = new OuterGeneric2<object>();
    private object _obj = new object();

    [Benchmark]
    public void Test1() => _1.Method(_obj);

    [Benchmark(Baseline = true)]
    public void Test2() => _2.Method(_obj);
}

public class OuterGeneric1<T>
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public void Method(T value) => Inner1.Method(value);
}

public class OuterGeneric2<T>
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public void Method(T value) => Inner2.Method(value);
}

public class Inner1
{
    public static int Method<T>(T value)
    {
        Throws1.ThrowIfNull(value);
        return 42;
    }
}

public class Inner2
{
    public static int Method<T>(T value)
    {
        Throws2.ThrowIfNull(value);
        return 42;
    }
}

public static class Throws1
{
    public static void ThrowIfNull<T>(T arg)
    {
        if (arg is null) Throw();
        static void Throw() => throw new ArgumentNullException();
    }
}

public static class Throws2
{
    public static void ThrowIfNull(object arg)
    {
        if (arg is null) Throw();
        static void Throw() => throw new ArgumentNullException();
    }
}

Note that the two versions are identical, with the exception that the ThrowIfNull in one case is generic and in the other is object. And here's what I get on current builds of .NET 8:

Method	Mean	Error	StdDev	Ratio
Test1	3.2050 ns	0.0091 ns	0.0080 ns	4.56
Test2	0.7028 ns	0.0067 ns	0.0063 ns	1.00

So with just a single argument being validated, the generic one is more than 4x as expensive. It's possible such issues could be addressed with significant runtime work in the future, but at present, this is a non-trivial tax on the method being used in a common use case. And with the JIT eliminating the boxing in the majority use case where it is used with value types, on balance it's better for it to be non-generic.

@stephentoub super insightful, thank you.

I did assume that boxing was occurring based off of the IL, and I was also working with full-debugger. I also performed benchmarks, but now that you brought it up, the debugger very likely played a part in execution-times.

Also learned a lot about how the compiler handles inlining and some inefficiency with generic, so all-in-all very helpful.

With this information at hand, I will now close the issue as it is the inferior option.