Replace some stackallocs with collection expressions #93126

stephentoub · 2023-10-06T15:04:56Z

These will result in using an InlineArray rather than a localloc

ghost · 2023-10-06T15:05:07Z

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

Issue Details

These will result in using an InlineArray rather than a localloc

Author:	stephentoub
Assignees:	stephentoub
Labels:	`area-Meta`
Milestone:	-

tannergooding

LGTM.

Would be good to resolve the styling question I asked on the other PR and update if we decide having the space is better

gfoidl

@stephentoub I played around with collection expression, and therefore your PRs are always a welcoming guide to get familiar with new features.

But here something seems strange, where either I miss something or this PR is actually a de-optimization.

will result in using an InlineArray rather than a localloc

When looking at the generated IL or reverse engineered C# with sharplab (main, 12 Oct 2023, so quite actual) I see array allocations instead the use of InlineArrays.

Can you please have a look and clarify?

I didn't comment on all places, to avoid noise.

gfoidl · 2023-10-14T12:14:52Z

src/libraries/System.Private.CoreLib/src/System/IO/DriveInfoInternal.Windows.cs

@@ -24,7 +24,7 @@ public static string[] GetLogicalDrives()
            int count = BitOperations.PopCount((uint)drives);

            string[] result = new string[count];
-            Span<char> root = stackalloc char[] { 'A', ':', '\\' };
+            Span<char> root = ['A', ':', '\\'];


According to sharplab, main 12 Oct 2023 this allocates.

Looks like the collection expression only avoid the allocation for ROS, but a ROS can't be used here.

gfoidl · 2023-10-14T12:18:17Z

src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLDecimal.cs

@@ -922,7 +922,7 @@ public override string ToString()
            AssertValid();

            // Make local copy of data to avoid modifying input.
-            Span<uint> rgulNumeric = stackalloc uint[4] { _data1, _data2, _data3, _data4 };
+            Span<uint> rgulNumeric = [_data1, _data2, _data3, _data4];


sharplab, main 12 Oct 2023 allocates.

gfoidl · 2023-10-14T12:23:12Z

src/libraries/System.Private.CoreLib/src/System/Array.cs

@@ -323,15 +323,15 @@ public static unsafe void Copy(Array sourceArray, int sourceIndex, Array destina
            if (Rank != 2)
                ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_Need2DArray);

-            return InternalGetValue(GetFlattenedIndex(stackalloc int[] { index1, index2 }));
+            return InternalGetValue(GetFlattenedIndex([index1, index2]));


sharplab, main 12 Oct 2023 allocates.

gfoidl · 2023-10-14T12:24:53Z

src/libraries/System.Private.CoreLib/src/System/SearchValues/Strings/Helpers/AhoCorasick.cs

@@ -331,7 +331,7 @@ private static void SurrogateToUpperNLS(char h, char l, out char hr, out char lr
            Debug.Assert(char.IsHighSurrogate(h));
            Debug.Assert(char.IsLowSurrogate(l));

-            Span<char> chars = stackalloc char[] { h, l };
+            ReadOnlySpan<char> chars = [h, l];


sharplab, main 12 Oct 2023 allocates.

stephentoub · 2023-10-14T12:41:22Z

sharplab is targeting a runtime that doesn't have InlineArray yet. Its main is about roslyn, not runtime.

gfoidl · 2023-10-14T13:39:09Z

Locally with .NET 8 RC2 I also see the array allocations (<LangVersion>preview</LangVersion> is set).

For example:

static void Test0(int a, int b)
{
    Span<int> foo = [a, b];
    ReadOnlySpan<int> bar = [a, b];
}

emits

.method assembly hidebysig static 
	void '<<Main>$>g__Test0|0_0' (
		int32 a,
		int32 b
	) cil managed 
{
	.custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
		01 00 00 00
	)
	// Method begins at RVA 0x2080
	// Header size: 12
	// Code size: 44 (0x2c)
	.maxstack 5
	.locals init (
		[0] valuetype [System.Runtime]System.Span`1<int32> foo,
		[1] valuetype [System.Runtime]System.ReadOnlySpan`1<int32> bar
	)

	IL_0000: nop
	IL_0001: ldloca.s 0
	IL_0003: ldc.i4.2
	IL_0004: newarr [System.Runtime]System.Int32			// <-- newarr for Span<int> foo = [a, b];
	IL_0009: dup
	IL_000a: ldc.i4.0
	IL_000b: ldarg.0
	IL_000c: stelem.i4
	IL_000d: dup
	IL_000e: ldc.i4.1
	IL_000f: ldarg.1
	IL_0010: stelem.i4
	IL_0011: call instance void valuetype [System.Runtime]System.Span`1<int32>::.ctor(!0[])
	IL_0016: ldloca.s 1
	IL_0018: ldc.i4.2
	IL_0019: newarr [System.Runtime]System.Int32			// <-- newarr for ReadOnlySpan<int> bar = [a, b];
	IL_001e: dup
	IL_001f: ldc.i4.0
	IL_0020: ldarg.0
	IL_0021: stelem.i4
	IL_0022: dup
	IL_0023: ldc.i4.1
	IL_0024: ldarg.1
	IL_0025: stelem.i4
	IL_0026: call instance void valuetype [System.Runtime]System.ReadOnlySpan`1<int32>::.ctor(!0[])
	IL_002b: ret
} // end of method Program::'<<Main>$>g__Test0|0_0'

Or when running:

int a = 3;
int b = 4;

long allocated0 = GC.GetTotalAllocatedBytes(precise: true);
Test0(a, b);
long allocated1 = GC.GetTotalAllocatedBytes(precise: true);
Console.WriteLine(allocated1 - allocated0);

static void Test0(int a, int b)
{
    Span<int> foo = [a, b];
    ReadOnlySpan<int> bar = [a, b];
}

it prints (in Release):

but I'd expect with that optimization to be 0.

Or when looking at godbolt (main) I see the two allocations that should not be there.

stephentoub · 2023-10-14T21:16:41Z

Sounds like your C# compiler is too old.

For your Test0, I get the equivalent of this:

private static void Test0(int a, int b)
{
	<>y__InlineArray2<int> buffer = default(<>y__InlineArray2<int>);
	global::<PrivateImplementationDetails>.InlineArrayElementRef<<>y__InlineArray2<int>, int>(ref buffer, 0) = a;
	global::<PrivateImplementationDetails>.InlineArrayElementRef<<>y__InlineArray2<int>, int>(ref buffer, 1) = b;
	global::<PrivateImplementationDetails>.InlineArrayAsSpan<<>y__InlineArray2<int>, int>(ref buffer, 2);
	<>y__InlineArray2<int> buffer2 = default(<>y__InlineArray2<int>);
	global::<PrivateImplementationDetails>.InlineArrayElementRef<<>y__InlineArray2<int>, int>(ref buffer2, 0) = a;
	global::<PrivateImplementationDetails>.InlineArrayElementRef<<>y__InlineArray2<int>, int>(ref buffer2, 1) = b;
	global::<PrivateImplementationDetails>.InlineArrayAsReadOnlySpan<<>y__InlineArray2<int>, int>(in buffer2, 2);
}

gfoidl · 2023-10-14T21:57:59Z

Fine, than it's as intended / expected. Thanks for clarification!

Sounds like your C# compiler is too old.

Today .NET 8 (RC2 and RTM (latest 8.0.1xx)) is already too old for this optimization...

stephentoub · 2023-10-14T22:03:40Z

Today .NET 8 (RC2 and RTM (latest 8.0.1xx)) is already too old for this optimization...

That should not be the case. Are you building in VS or from the command line?

Regardless, you need a compiler with this:
dotnet/roslyn#69412

gfoidl · 2023-10-14T22:12:54Z

Are you building in VS or from the command line?

VS tricked me into this.
With the command line I see now the expected code.
Tried both with the .NET 8 RTM SDK.

Thanks!

radical · 2023-10-16T18:30:25Z

This broke a bunch of dotnet/performance benchmarks. See #93348 .

stephentoub · 2023-10-16T19:11:08Z

This broke a bunch of dotnet/performance benchmarks. See #93348 .

Sounds like it's an issue with the mono interpreter not handling [InlineArray]s correctly.

Replace some stackallocs with collection expressions

531612e

These will result in using an InlineArray rather than a localloc

dotnet-issue-labeler bot added the area-Meta label Oct 6, 2023

ghost assigned stephentoub Oct 6, 2023

tannergooding approved these changes Oct 6, 2023

View reviewed changes

build-analysis bot mentioned this pull request Oct 6, 2023

[Test Failure] System.Net.Security.Tests.CertificateValidationClientServer.CertificateValidationClientServer_EndToEnd_Ok on Windows AoT #88832

Closed

stephentoub merged commit 110d84d into dotnet:main Oct 6, 2023
174 checks passed

stephentoub deleted the stackallocce branch October 6, 2023 19:41

EgorBo mentioned this pull request Oct 10, 2023

[Perf] Linux/x64: 2 Improvements on 10/6/2023 11:50:29 PM dotnet/perf-autofiling-issues#22865

Closed

AndyAyersMS mentioned this pull request Oct 10, 2023

[Perf] Windows/x64: 12 Improvements on 10/7/2023 7:16:36 AM dotnet/perf-autofiling-issues#22873

Closed

radical mentioned this pull request Oct 11, 2023

[wasm] Inline array struct must have a single field. assembly:System.Private.Xml.dll type:<>y__InlineArray21 member:(null)` - perf run broken #93348

Closed

EgorBo mentioned this pull request Oct 12, 2023

[Perf] Linux/arm64: 8 Regressions on 10/7/2023 2:03:18 PM #93401

Closed

gfoidl reviewed Oct 14, 2023

View reviewed changes

halgab mentioned this pull request Oct 21, 2023

Use collection expressions to init spans where possible dotnet/winforms#10114

Merged

gfoidl mentioned this pull request Nov 14, 2023

Console.Unix: for echoing, write back to the terminal instead of writing to Console.Out. #94414

Merged

ghost locked as resolved and limited conversation to collaborators Nov 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace some stackallocs with collection expressions #93126

Replace some stackallocs with collection expressions #93126

stephentoub commented Oct 6, 2023

ghost commented Oct 6, 2023

tannergooding left a comment

gfoidl left a comment

gfoidl Oct 14, 2023

gfoidl Oct 14, 2023

gfoidl Oct 14, 2023

gfoidl Oct 14, 2023

stephentoub commented Oct 14, 2023 •

edited

Loading

gfoidl commented Oct 14, 2023

stephentoub commented Oct 14, 2023

gfoidl commented Oct 14, 2023

stephentoub commented Oct 14, 2023 •

edited

Loading

gfoidl commented Oct 14, 2023

radical commented Oct 16, 2023

stephentoub commented Oct 16, 2023

Replace some stackallocs with collection expressions #93126

Replace some stackallocs with collection expressions #93126

Conversation

stephentoub commented Oct 6, 2023

ghost commented Oct 6, 2023

tannergooding left a comment

Choose a reason for hiding this comment

gfoidl left a comment

Choose a reason for hiding this comment

gfoidl Oct 14, 2023

Choose a reason for hiding this comment

gfoidl Oct 14, 2023

Choose a reason for hiding this comment

gfoidl Oct 14, 2023

Choose a reason for hiding this comment

gfoidl Oct 14, 2023

Choose a reason for hiding this comment

stephentoub commented Oct 14, 2023 • edited Loading

gfoidl commented Oct 14, 2023

stephentoub commented Oct 14, 2023

gfoidl commented Oct 14, 2023

stephentoub commented Oct 14, 2023 • edited Loading

gfoidl commented Oct 14, 2023

radical commented Oct 16, 2023

stephentoub commented Oct 16, 2023

stephentoub commented Oct 14, 2023 •

edited

Loading

stephentoub commented Oct 14, 2023 •

edited

Loading