Inconsistent inlining between Span.ToArray and ReadOnlySpan.ToArray

It's odd that Span.ToArray is marked AggressiveInlining but ReadOnlySpan.ToArray isn't.
This is the only shared helper where they differ in this way.

Seems to have snuck in during dotnet/corefx#26667

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

Lines 435 to 437 in c1b153a

    
           /// </summary> 
        
           [MethodImpl(MethodImplOptions.AggressiveInlining)] 
        
           public T[] ToArray()

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

Lines 406 to 407 in c1b153a

    
           /// </summary> 
        
           public T[] ToArray()

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

Inlining seems beneficial if the span length is known to be non-zero, as the static ctor for Array.Empty<T>() can then be elided.

Some nice code size improvements by removing AggressiveInlining and splitting the implementation across two methods: MihuBot/runtime-utils#1497

Marked as help wanted, inviting a thorough analysis of the tradeoffs.

Some nice code size improvements by removing AggressiveInlining and splitting the implementation across two methods: MihuBot/runtime-utils#1497

Code size improvement is even better now: MihuBot/runtime-utils#2119

code size improvements

These numbers do not seem to account for the code size of the non-inlined method.

Also, I assume that the code size improvements would be even better if the method is not split. Is that right? I do not see why we would want to pay extra code bloat to optimize ToArray() on empty Span.

The way I see it, the useful case is when the span is known to be non-empty, since the JIT can then get rid of the static constructor for Array.Empty<T>. So it's a way of having the JIT inline everything in that case without using AggressiveInlining.

Let’s see anyway: MihuBot/runtime-utils#2120, MihuBot/runtime-utils#2121.

when the span is known to be non-empty

How common is that with ToArray?

Code size improvement is even better now

Note that this is not an apples-apples comparison since the default MihuBot job is now also testing against a small subset of NuGet packages

646 total methods with Code Size differences (598 improved, 48 regressed), 249906 unchanged.
916 total methods with Code Size differences (859 improved, 57 regressed), 815988 unchanged.

when the span is known to be non-empty

How common is that with ToArray?

Not that uncommon, here's an example:

runtime/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray.cs

Lines 91 to 100 in 45ad194

    
           public static ImmutableArray<T> Create<T>(params ReadOnlySpan<T> items) 
        
           { 
        
               if (items.IsEmpty) 
        
               { 
        
                   return ImmutableArray<T>.Empty; 
        
               } 
        
               T[] array = items.ToArray(); 
        
               return new ImmutableArray<T>(array); 
        
           }

Not that uncommon, here's an example:

The special casing for items.IsEmpty should not be needed in this case. I think we should delete it. Span.ToArray takes care of special casing the empty array already.

In the case of ImmutableArray, we also target .NET Framework, so this may well be written this way for a reason. If not, there doesn’t seem to be any reason to use ImmutableArray<T>.Empty rather than new ImmutableArray<T>(new T[0]) internally.

this may well be written this way for a reason.

I doubt it. For reference, this was introduced in #61196. .NET Framework Span is very slow, there is no point in trying to optimize for it.

there doesn’t seem to be any reason to use ImmutableArray.Empty rather than new ImmutableArray(new T[0]) internally.

ImmutableArray.Empty is a singleton. It avoids GC heap allocation. The same optimization is in .NET Framework Span.

There is also Create() that is wrapper around ImmutableArray.Empty. There is no reason to use Create() internally - it is just an extra overhead. You can replace all Create() with ImmutableArray.Empty if you are going to submit a PR for this.

I think the special casing avoids an extra generic instantiation for Array.Empty<T>() in Span<T>.ToArray. Perhaps this explains the significant code size improvement in MihuBot/runtime-utils#2119

Remove forced inlining from Span<T>.ToArray (#132635)

Navigation Menu

Repository navigation

Inconsistent inlining between Span.ToArray and ReadOnlySpan.ToArray #119713

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	/// </summary>
	[MethodImpl(MethodImplOptions.AggressiveInlining)]
	public T[] ToArray()

	public static ImmutableArray<T> Create<T>(params ReadOnlySpan<T> items)
	{
	if (items.IsEmpty)
	{
	return ImmutableArray<T>.Empty;
	}

	T[] array = items.ToArray();
	return new ImmutableArray<T>(array);
	}

Inconsistent inlining between Span.ToArray and ReadOnlySpan.ToArray #119713

Description

Activity

dotnet-policy-service commented on Sep 15, 2025

xtqqczze commented on Sep 15, 2025

xtqqczze commented on Sep 15, 2025

jeffhandley commented on Aug 17, 2026

xtqqczze commented on Aug 17, 2026

jkotas commented on Aug 17, 2026

xtqqczze commented on Aug 17, 2026

jkotas commented on Aug 17, 2026

MihaZupan commented on Aug 17, 2026

xtqqczze commented on Aug 17, 2026

jkotas commented on Aug 17, 2026

xtqqczze commented on Aug 17, 2026

jkotas commented on Aug 17, 2026

xtqqczze commented on Aug 17, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions