Skip to content

Conversation

@albert-du
Copy link
Contributor

Description

Delegate array slicing to System.Array.Copy.

Fixes #18644

Benchmark Results:

The MicroPerf benchmarks that are committed aren't launching on my installed version of .NET (I don't have .NET 10 Preview 7 access)

Using benchmarks from #18644:

[<MemoryDiagnoser; ShortRunJob>]
type Benchmarks () =
    let b = [| for _ in 1 .. 100_000 -> byte DateTime.Now.Ticks |]

    [<Benchmark>]
    member _.x () =
        b.[ 20 .. 4999 ]

    [<Benchmark>]
    member f.xx () =
        let out = GC.AllocateUninitializedArray<byte> (4980, false)
        Array.Copy (b, 20, out, 0, 4980)
        out

    [<Benchmark>]
    member gg.xxx () =
        b.AsSpan(20, 4980).ToArray ()

Before:

BenchmarkDotNet v0.15.2, Windows 11 (10.0.26100.4652/24H2/2024Update/HudsonValley)
AMD Ryzen 7 5800X, 1 CPU, 16 logical and 8 physical cores
.NET SDK 10.0.100-preview.6.25358.103
  [Host]   : .NET 10.0.0 (10.0.25.35903), X64 RyuJIT AVX2 DEBUG
  ShortRun : .NET 10.0.0 (10.0.25.35903), X64 RyuJIT AVX2

Job=ShortRun  IterationCount=3  LaunchCount=1
WarmupCount=3

| Method | Mean       | Error     | StdDev   | Gen0   | Allocated |
|------- |-----------:|----------:|---------:|-------:|----------:|
| x      | 2,514.5 ns | 690.39 ns | 37.84 ns | 0.2975 |   4.89 KB |
| xx     |   224.0 ns | 158.36 ns |  8.68 ns | 0.2983 |   4.89 KB |
| xxx    |   210.8 ns |  88.84 ns |  4.87 ns | 0.2992 |   4.89 KB |

After:

BenchmarkDotNet v0.15.2, Windows 11 (10.0.26100.4652/24H2/2024Update/HudsonValley)
AMD Ryzen 7 5800X, 1 CPU, 16 logical and 8 physical cores
.NET SDK 10.0.100-preview.6.25358.103
  [Host]   : .NET 10.0.0 (10.0.25.35903), X64 RyuJIT AVX2 DEBUG
  ShortRun : .NET 10.0.0 (10.0.25.35903), X64 RyuJIT AVX2

Job=ShortRun  IterationCount=3  LaunchCount=1
WarmupCount=3

| Method | Mean     | Error    | StdDev   | Gen0   | Allocated |
|------- |---------:|---------:|---------:|-------:|----------:|
| x      | 238.1 ns | 108.7 ns |  5.96 ns | 0.2992 |   4.89 KB |
| xx     | 218.4 ns | 210.3 ns | 11.53 ns | 0.2983 |   4.89 KB |
| xxx    | 232.6 ns | 389.8 ns | 21.37 ns | 0.2992 |   4.89 KB |

90% performance improvement on my machine. An AVX-512 CPU would be even faster.

Tests

Existing tests already cover array slicing get/set. This PR adds an additional test to verify an array returned as a slice is unique (different object) as is the existing behavior.

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated

Array slicing get and set delegated to System.Array.Copy.
@github-actions
Copy link
Contributor

github-actions bot commented Jul 25, 2025

✅ No release notes required

@albert-du
Copy link
Contributor Author

@kerams

Returning [||] for a zero length slice might be a breaking change.

Currently, creating two empty slices results in two empty arrays without referential equality.

let a = [|1;2;3|]

let b = a[1..0]
let c = a[1..0]

printfn $"{b.Length}" // 0
printfn $"{c.Length}" // 0

printfn $"{b = c}" // True
printfn $"{System.Object.ReferenceEquals(b,c)}" // False

sharplab

By returning [||], all empty slices are represented by the same empty array.

let b = [||]
let c = [||]

printfn $"{b.Length}" // 0
printfn $"{c.Length}" // 0

printfn $"{b = c}" // True
printfn $"{System.Object.ReferenceEquals(b,c)}" // True // this is different

sharplab

This doesn't affect non zero sized slices, so there won't be inconsistent behavior due to array mutation. If there was code dependent on a slice always returning a unique object, regardless of size, (perhaps for a lock) this change would break it.

Returning [||] would be more performant, fewer objects for the GC, but I think this change in behavior would need to be noted somewhere.

@brianrourkeboll
Copy link
Contributor

Returning [||] for a zero length slice might be a breaking change.

Currently, creating two empty slices results in two empty arrays without referential equality.

I think it's an acceptable change. [||] itself used to return a new array but no longer does.

@albert-du
Copy link
Contributor Author

Agreed, but should it be documented somewhere?

@T-Gro T-Gro added the needs-breaking-change-doc-created A PR needs a doc entry describing a new breaking change label Jul 28, 2025
@T-Gro
Copy link
Member

T-Gro commented Jul 28, 2025

@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Jul 29, 2025
@T-Gro T-Gro enabled auto-merge (squash) July 29, 2025 07:50
@T-Gro T-Gro merged commit de59365 into dotnet:main Jul 29, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in F# Compiler and Tooling Jul 29, 2025
@albert-du albert-du deleted the optimize-array-slicing branch August 21, 2025 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-breaking-change-doc-created A PR needs a doc entry describing a new breaking change

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Array slicing should be delegated to the BCL

4 participants