API proposal: Array.IsNullOrEmpty

[GrabYourPitchforks]

Justification

A helper method along the same lines of string.IsNullOrEmpty. We have several dozen places within System.Private.CoreLib alone which perform array null or empty checks: see GrabYourPitchforks@ef639cd for a prototype of this change and a list of candidate call sites.

Furthermore, we can perform tricks within Array.IsNullOrEmpty similar to what we already do in string.IsNullOrEmpty. These take advantage of patterns the JIT already recognizes in order to optimize the codegen for code later in the method.

For example:

int[] theArray = GetArray();
if (!Array.IsNullOrEmpty(theArray))
{
    int element = theArray[0]; // JIT elides bounds check here
}

This API obviously isn't required for apps to have correct behavior (devs have gone without it for 20+ years). But it is a convenience method that could add succinctness and clarity to call sites, and it could give a minor perf boost to some scenarios.

API Proposal

namespace System
{
    public abstract partial class Array
    {
        public static bool IsNullOrEmpty([NotNullWhen(false)] Array? array) { throw null; }
    }
}

There's no need for this method to be generic on T; taking a standard Array would be sufficient to cover all use cases.

[stephentoub]

#16359

[GrabYourPitchforks]

You're dashing my hopes and dreams, man. :)

[stephentoub]

😄

[AaronRobinsonMSFT]

Within the link that @stephentoub mention above, there is a comment about ICollection.IsNullOrEmpty(). Perhaps that would have a bit more traction. I am personally convinced by the argument this isn't needed specifically for Array, but would be open to the debate for ICollection.

[GrabYourPitchforks]

ICollection would definitely be more generalizable. The primary arguments I can think of for a method which takes Array directly are: (a) no virtual dispatch is needed and it keeps the call site very tight, and (b) it allows the JIT to elide accesses to theArray[0] immediately after the check. See for instance GrabYourPitchforks@ef639cd#diff-6e53989dedc88bcbe4187d065ee392f1R31.

(Edit: wrong link above.)

[GrabYourPitchforks]

FWIW I did verify that the JIT does elide the bounds check on the pattern if (!Array.IsNullOrEmpty(someArray)) { /* use someArray[0] */ }. Still not quite sure if it's an important enough optimization for this.

[stephentoub]

Does it not elide it for:

if (someArray != null && someArray.Length != 0) { /* use someArray[0] */ }

or

if (someArray?.Length != 0) { /* use someArray[0] */ }

?

[GrabYourPitchforks]

Correct, those patterns aren't elided. There's a code comment in string.IsNullOrEmpty explaining it in more detail.

runtime/src/libraries/System.Private.CoreLib/src/System/String.cs

Lines 458 to 467 in 97b0b37

	public static bool IsNullOrEmpty([NotNullWhen(false)] string? value)
	{
	// Using 0u >= (uint)value.Length rather than
	// value.Length == 0 as it will elide the bounds check to
	// the first char: value[0] if that is performed following the test
	// for the same test cost.
	// Ternary operator returning true/false prevents redundant asm generation:
	// https://github.com/dotnet/runtime/issues/4207
	return (value == null || 0u >= (uint)value.Length) ? true : false;
	}

[jkotas]

Correct, those patterns aren't elided. There's a code comment in string.IsNullOrEmpty explaining it in more detail.

That is a bug to fix in the JIT, not to workaround by adding new APIs.

[GrabYourPitchforks]

Yeah, I only found around 30-ish call sites within System.Private.CoreLib that would be candidates for using this helper method. (See GrabYourPitchforks@ef639cd.) That might not be enough to justify the work. Even string.IsNullOrEmpty has around 5x that many call sites within CoreLib.

[juliusfriedman]

Justification

A helper method along the same lines of string.IsNullOrEmpty. We have several dozen places within System.Private.CoreLib alone which perform array null or empty checks: see GrabYourPitchforks@ef639cd for a prototype of this change and a list of candidate call sites.

Furthermore, we can perform tricks within Array.IsNullOrEmpty similar to what we already do in string.IsNullOrEmpty. These take advantage of patterns the JIT already recognizes in order to optimize the codegen for code later in the method.

For example:

int[] theArray = GetArray();
if (!Array.IsNullOrEmpty(theArray))
{
    int element = theArray[0]; // JIT elides bounds check here
}

This API obviously isn't required for apps to have correct behavior (devs have gone without it for 20+ years). But it is a convenience method that could add succinctness and clarity to call sites, and it could give a minor perf boost to some scenarios.

API Proposal

namespace System
{
    public abstract partial class Array
    {
        public static bool IsNullOrEmpty([NotNullWhen(false)] Array? array) { throw null; }
    }
}

There's no need for this method to be generic on T; taking a standard Array would be sufficient to cover all use cases.

Having this method not return the length via out is [faux pas] e.g. NOT 聡.

I hate to be a troll but since the JIT can't be fixed so easily why not take the easy road and then have the jit eliminate calls to this method later on... i.e.

You can teach the JIT to see this method and then when the problem is fixed that code can go away because then as you state the asm will be the same.

And btw string needs this also:
#39560

[GrabYourPitchforks]

Previously considered and rejected. Closing.