diff --git a/docs/release-notes/.FSharp.Core/11.0.100.md b/docs/release-notes/.FSharp.Core/11.0.100.md index 521901da210..004c714ba70 100644 --- a/docs/release-notes/.FSharp.Core/11.0.100.md +++ b/docs/release-notes/.FSharp.Core/11.0.100.md @@ -23,6 +23,7 @@ * Deduplicate repeated XML documentation (sort stability/complexity, `Parallel.For`, dynamic entry points) via the compile-time `` tag; generated FSharp.Core.xml is unchanged. ([PR #20231](https://github.com/dotnet/fsharp/pull/20231)) * Added modules for `Async`, `Task` and `ValueTask` with consistent `result`, `map`, `bind`, `ignore`, `catchWith`, `catch`, and `empty` functions ([LanguageSuggestion #1466](https://github.com/fsharp/fslang-suggestions/issues/1466), [PR #19844](https://github.com/dotnet/fsharp/pull/19844)) +* Most `Array` and `Array.Parallel` functions now return the shared empty-array singleton instead of allocating a fresh zero-length array when the result is empty (`collect`, `map`, `concat`, `append`, `zip`/`unzip`, `partition`, `filter`, `distinct`, the slicing functions, `zeroCreate`/`create`/`init` at length 0, and more). ([Issue #20382](https://github.com/dotnet/fsharp/issues/20382), [PR #20388](https://github.com/dotnet/fsharp/pull/20388)) * Added conversion functions `Task.ofValueTask` and `ValueTask.ofTask`. ([LanguageSuggestion #1466](https://github.com/fsharp/fslang-suggestions/issues/1466), [PR #19844](https://github.com/dotnet/fsharp/pull/19844)) * Add `Async.StartTaskImmediate`: passes the ambient `Async.CancellationToken` to a task factory, then await the result using `Async.Await` semantics. Overloads for `Task`, `Task<'T>`, `ValueTask`, `ValueTask<'T>` and task-like `.GetAwaiter()` (via SRTP). ([Language Suggestion #1284](https://github.com/fsharp/fslang-suggestions/issues/1284), [PR #20258](https://github.com/dotnet/fsharp/pull/20258)) * Add `Async.RunSynchronouslyImmediate`: runs work on the calling thread until the first asynchronous suspension (as opposed to `RunSynchronously`, which immediately offloads if not on a background and/or threadpool thread). ([Issue #1042](https://github.com/fsharp/fslang-suggestions/issues/1042), [PR #19804](https://github.com/dotnet/fsharp/pull/19804)) diff --git a/src/FSharp.Core/array.fs b/src/FSharp.Core/array.fs index 967285a909b..d8a3b2086c4 100644 --- a/src/FSharp.Core/array.fs +++ b/src/FSharp.Core/array.fs @@ -641,7 +641,7 @@ module Array = if i <> array.Length then let chunk1: 'U array = - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked ((array.Length >>> 2) + 1) + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty ((array.Length >>> 2) + 1) chunk1.[0] <- first let mutable count = 1 @@ -660,7 +660,7 @@ module Array = if i < array.Length then let chunk2: 'U array = - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked (array.Length - i) + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty (array.Length - i) count <- 0 @@ -676,7 +676,7 @@ module Array = i <- i + 1 let res: 'U array = - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked (chunk1.Length + count) + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty (chunk1.Length + count) Array.Copy(chunk1, res, chunk1.Length) Array.Copy(chunk2, 0, res, chunk1.Length, count) @@ -848,7 +848,7 @@ module Array = if maskArrayLength = 0 then null else - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked maskArrayLength + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty maskArrayLength let mutable count = match maskArray with @@ -1032,7 +1032,7 @@ module Array = let private filterViaMask (maskArray: uint32 array | null) (leftoverMask: uint32) (count: int) (src: _ array) = - let dst = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked count + let dst = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty count let mutable dstIdx = 0 @@ -1244,7 +1244,7 @@ module Array = empty else let res: 'T array array = - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked (len - windowSize + 1) + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty (len - windowSize + 1) for i = 0 to len - windowSize do res.[i] <- Microsoft.FSharp.Primitives.Basics.Array.subUnchecked i windowSize array @@ -1268,7 +1268,7 @@ module Array = let chunkCount = (len - 1) / chunkSize + 1 let res: 'T array array = - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked chunkCount + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty chunkCount for i = 0 to len / chunkSize - 1 do res.[i] <- Microsoft.FSharp.Primitives.Basics.Array.subUnchecked (i * chunkSize) chunkSize array @@ -1860,7 +1860,7 @@ module Array = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked lenInner for i in 0 .. lenInner - 1 do - result.[i] <- Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked len + result.[i] <- Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty len for j in 0 .. len - 1 do result.[i].[j] <- array.[j].[i] @@ -2397,7 +2397,7 @@ module Array = let chunks = createPartitionsUpToWithMinChunkSize array.Length 2 array // We need at least 2 elements/chunk for 'reduction' let chunkResults = - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked chunks.Length + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty chunks.Length Parallel.For( 0, @@ -2543,7 +2543,7 @@ module Array = for kvp in counts do let arrayForThisGroup = - Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked kvp.Value.Value + Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUncheckedNonEmpty kvp.Value.Value finalResults.[finalIdx] <- getKey kvp.Key, arrayForThisGroup finalResultsLookup[kvp.Key] <- kvp.Value, arrayForThisGroup diff --git a/src/FSharp.Core/local.fs b/src/FSharp.Core/local.fs index c365f9e5bdb..5b96bda4bfd 100644 --- a/src/FSharp.Core/local.fs +++ b/src/FSharp.Core/local.fs @@ -987,10 +987,20 @@ module internal Array = let inline fastComparerForArraySort<'t when 't : comparison> () = LanguagePrimitives.FastGenericComparerCanBeNull<'t> - // The input parameter should be checked by callers if necessary - let inline zeroCreateUnchecked (count:int) = + // Allocates a new array of the given length without validating the count, always returning + // a freshly allocated array (even for count = 0). Prefer zeroCreateUnchecked unless the + // length is provably > 0; this variant exists only to skip the empty-array branch on hot, + // known-non-empty paths. + let inline zeroCreateUncheckedNonEmpty (count: int) = (# "newarr !0" type ('T) count : 'T array #) + // Allocates a new array of the given length without validating the count, but shares the + // empty-array singleton (System.Array.Empty<_>(), which the compiler emits for the [||] + // literal) when count = 0, so empty results allocate nothing. This is the default; use + // zeroCreateUncheckedNonEmpty only where the length is provably > 0 to skip the branch. + let inline zeroCreateUnchecked (count: int) : 'T array = + if count = 0 then [||] else zeroCreateUncheckedNonEmpty count + let inline init (count:int) ([] f: int -> 'T) = if count < 0 then invalidArgInputMustBeNonNegative "count" count let arr = (zeroCreateUnchecked count : 'T array) @@ -1069,7 +1079,7 @@ module internal Array = let scanSubRight f (array: _ array) start fin initState = let f = OptimizedClosures.FSharpFunc<_, _, _>.Adapt(f) let mutable state = initState - let res = zeroCreateUnchecked (fin-start+2) + let res = zeroCreateUncheckedNonEmpty (fin-start+2) res.[fin - start + 1] <- state for i = fin downto start do state <- f.Invoke(array.[i], state) @@ -1166,7 +1176,7 @@ module internal Array = [| |] else let count = min count len - let res = zeroCreateUnchecked count : 'T array array + let res = zeroCreateUncheckedNonEmpty count : 'T array array let minChunkSize = len / count let mutable startIndex = 0 for i = 0 to len % count - 1 do diff --git a/src/FSharp.Core/local.fsi b/src/FSharp.Core/local.fsi index 11f645c9788..9423b461837 100644 --- a/src/FSharp.Core/local.fsi +++ b/src/FSharp.Core/local.fsi @@ -84,7 +84,10 @@ module internal List = val tryLastV: 'T list -> 'T ValueOption module internal Array = - // The input parameter should be checked by callers if necessary + // Allocates without validating the count; always a fresh array. Use only where count > 0. + val inline zeroCreateUncheckedNonEmpty: int -> 'T array + + // Allocates without validating the count, sharing the empty-array singleton when count = 0. val inline zeroCreateUnchecked: int -> 'T array val inline init: int -> (int -> 'T) -> 'T array diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/ArrayModule.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/ArrayModule.fs index ba6e001fd31..a2d297ca213 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/ArrayModule.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/ArrayModule.fs @@ -461,6 +461,63 @@ type ArrayModule() = member this.``Parallel.Collect`` () = this.CollectTester Array.Parallel.collect Array.Parallel.collect + [] + member _.EmptyResultsShareSingleton () = + // A zero-length result should reuse the shared Array.empty singleton rather than + // allocating a fresh zero-length array (see issue #20382). + let isShared (a: 'a[]) = obj.ReferenceEquals(a, Array.empty<'a>) + let ints: int[] = [||] + + Assert.True(isShared (Array.collect (fun x -> [| x |]) ints), "collect (empty input)") + Assert.True(isShared (Array.collect (fun _ -> [||]) [| 1; 2; 3 |]), "collect (all-empty results)") + Assert.True(isShared (Array.map (fun x -> x + 1) ints), "map") + Assert.True(isShared (Array.mapi (fun i x -> i + x) ints), "mapi") + Assert.True(isShared (Array.indexed ints), "indexed") + Assert.True(isShared (Array.concat ([]: int[] list)), "concat (empty)") + Assert.True(isShared (Array.concat [ [||]; [||] ]), "concat (of empties)") + Assert.True(isShared (Array.take 0 [| 1; 2; 3 |]), "take 0") + Assert.True(isShared (Array.skip 3 [| 1; 2; 3 |]), "skip all") + Assert.True(isShared (Array.sub [| 1; 2; 3 |] 1 0), "sub len 0") + Assert.True(isShared (Array.distinctBy id ints), "distinctBy") + Assert.True(isShared (Array.groupBy id ints), "groupBy") + Assert.True(isShared (Array.Parallel.map (fun x -> x + 1) ints), "Parallel.map") + Assert.True(isShared (Array.Parallel.collect (fun x -> [| x |]) ints), "Parallel.collect") + + // Leaf builders that previously allocated a throwaway zero-length array for empty inputs. + Assert.True(isShared (Array.append ints ints), "append") + Assert.True(isShared (Array.map2 (fun a b -> a + b) ints ints), "map2") + Assert.True(isShared (Array.mapi2 (fun i a b -> i + a + b) ints ints), "mapi2") + Assert.True(isShared (Array.map3 (fun a b c -> a + b + c) ints ints ints), "map3") + Assert.True(isShared (Array.zip ints ints), "zip") + Assert.True(isShared (Array.zip3 ints ints ints), "zip3") + Assert.True(isShared (Array.allPairs ints ints), "allPairs") + Assert.True(isShared (Array.rev ints), "rev") + Assert.True(isShared (Array.distinct ints), "distinct") + Assert.True(isShared (Array.removeAt 0 [| 1 |]), "removeAt last") + Assert.True(isShared (Array.removeManyAt 0 1 [| 1 |]), "removeManyAt last") + Assert.True(isShared (Array.transpose [| ([||]: int[]) |]), "transpose (empty inner)") + + let p1, p2 = Array.partition (fun _ -> true) ints + Assert.True(isShared p1 && isShared p2, "partition") + let u1, u2 = Array.unzip ([||]: (int * int)[]) + Assert.True(isShared u1 && isShared u2, "unzip") + let t1, t2, t3 = Array.unzip3 ([||]: (int * int * int)[]) + Assert.True(isShared t1 && isShared t2 && isShared t3, "unzip3") + + Assert.True(isShared (Array.Parallel.mapi (fun i x -> i + x) ints), "Parallel.mapi") + Assert.True(isShared (Array.Parallel.choose Some ints), "Parallel.choose") + Assert.True(isShared (Array.Parallel.zip ints ints), "Parallel.zip") + Assert.True(isShared (Array.Parallel.filter (fun _ -> true) ints), "Parallel.filter") + Assert.True(isShared (Array.Parallel.groupBy id ints), "Parallel.groupBy") + let pp1, pp2 = Array.Parallel.partition (fun _ -> true) ints + Assert.True(isShared pp1 && isShared pp2, "Parallel.partition") + + // Improve-by-default: even the count-based constructors share the singleton at length 0. + Assert.True(isShared (Array.zeroCreate 0: int[]), "zeroCreate 0") + Assert.True(isShared (Array.create 0 0), "create 0") + Assert.True(isShared (Array.init 0 id), "init 0") + Assert.True(isShared (Array.replicate 0 0), "replicate 0") + [] member this.compareWith() = // compareWith should work on empty arrays