Skip to content

Commit b7bc2ca

Browse files
goswinrbaronfel
authored andcommitted
Optimize Seq.Last to behave like Seq.Length (incl. tests) (#7765)
* Optimize Seq.Last like Seq.Length Add type check to Seq.Last and Seq.TryLast to avoid full iteration if not necessary. Like Seq.Length does it at https://github.com/dotnet/fsharp/blob/c18e1780b3f3f345364cb1ad8e510ea9f4590d3a/src/fsharp/FSharp.Core/seq.fs#L709 * Add test for Optimized Seq.Last and Seq.TryLast * style update * style update 2 * update comments and fix build error Github Build error was: Check failure on line 132 in tests\FSharp.Core.UnitTests\FSharp.Core\Microsoft.FSharp.Collections\SeqModule2.fs @azure-pipelines azure-pipelines / fsharp-ci (Build Windows vs_release) tests\FSharp.Core.UnitTests\FSharp.Core\Microsoft.FSharp.Collections\SeqModule2.fs#L132 tests\FSharp.Core.UnitTests\FSharp.Core\Microsoft.FSharp.Collections\SeqModule2.fs(132,58): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'a has been constrained to be type 'unit'. * include List.last case * include list in tests * ensure same exception is raised on empty list * typo * Added implementation of List.last since it is not available at this point due to compilation error The recursive function could also be defined inside but i guess then it would be reallocated on every call to Seq.Last (in case of list match). An alternative would be to add .Last member on list type (like .Length member) * inline List.last reverting my previous attempt dotnet/fsharp@b329e23 tail rec functions should be inlined * typo in tests(build failed) * typo2 in test (Build failed) * move implemnetation to internal module as suggested by dsyme * renamed internal tryLast to tryLastV
1 parent 96fc7b0 commit b7bc2ca

File tree

4 files changed

+50
-26
lines changed

4 files changed

+50
-26
lines changed

src/fsharp/FSharp.Core/list.fs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ namespace Microsoft.FSharp.Collections
99
open Microsoft.FSharp.Collections
1010
open Microsoft.FSharp.Core.CompilerServices
1111
open System.Collections.Generic
12+
1213

1314
[<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
1415
[<RequireQualifiedAccess>]
@@ -24,18 +25,16 @@ namespace Microsoft.FSharp.Collections
2425
let length (list: 'T list) = list.Length
2526

2627
[<CompiledName("Last")>]
27-
let rec last (list: 'T list) =
28-
match list with
29-
| [x] -> x
30-
| _ :: tail -> last tail
31-
| [] -> invalidArg "list" (SR.GetString(SR.inputListWasEmpty))
28+
let last (list: 'T list) =
29+
match Microsoft.FSharp.Primitives.Basics.List.tryLastV list with
30+
| ValueSome x -> x
31+
| ValueNone -> invalidArg "list" (SR.GetString(SR.inputListWasEmpty))
3232

3333
[<CompiledName("TryLast")>]
3434
let rec tryLast (list: 'T list) =
35-
match list with
36-
| [x] -> Some x
37-
| _ :: tail -> tryLast tail
38-
| [] -> None
35+
match Microsoft.FSharp.Primitives.Basics.List.tryLastV list with
36+
| ValueSome x -> Some x
37+
| ValueNone -> None
3938

4039
[<CompiledName("Reverse")>]
4140
let rev list = Microsoft.FSharp.Primitives.Basics.List.rev list

src/fsharp/FSharp.Core/local.fs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -988,6 +988,12 @@ module internal List =
988988
takeWhileFreshConsTail cons p xs
989989
cons
990990

991+
let rec tryLastV (list: 'T list) =
992+
match list with
993+
| [] -> ValueNone
994+
| [x] -> ValueSome x
995+
| _ :: tail -> tryLastV tail
996+
991997
module internal Array =
992998

993999
open System
@@ -1187,3 +1193,26 @@ module internal Array =
11871193
res.[i] <- subUnchecked !startIndex minChunkSize array
11881194
startIndex := !startIndex + minChunkSize
11891195
res
1196+
1197+
module internal Seq =
1198+
let tryLastV (source : seq<_>) =
1199+
//checkNonNull "source" source //done in main Seq.tryLast
1200+
match source with
1201+
| :? ('T[]) as a ->
1202+
if a.Length = 0 then ValueNone
1203+
else ValueSome(a.[a.Length - 1])
1204+
1205+
| :? ('T IList) as a -> //ResizeArray and other collections
1206+
if a.Count = 0 then ValueNone
1207+
else ValueSome(a.[a.Count - 1])
1208+
1209+
| :? ('T list) as a -> List.tryLastV a
1210+
1211+
| _ ->
1212+
use e = source.GetEnumerator()
1213+
if e.MoveNext() then
1214+
let mutable res = e.Current
1215+
while (e.MoveNext()) do res <- e.Current
1216+
ValueSome(res)
1217+
else
1218+
ValueNone

src/fsharp/FSharp.Core/local.fsi

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ module internal List =
6565
val splitAt : int -> 'T list -> ('T list * 'T list)
6666
val transpose : 'T list list -> 'T list list
6767
val truncate : int -> 'T list -> 'T list
68+
val tryLastV : 'T list -> 'T ValueOption
6869

6970
module internal Array =
7071
// The input parameter should be checked by callers if necessary
@@ -101,3 +102,6 @@ module internal Array =
101102
val stableSortInPlaceWith: comparer:('T -> 'T -> int) -> array:'T[] -> unit
102103

103104
val stableSortInPlace: array:'T[] -> unit when 'T : comparison
105+
106+
module internal Seq =
107+
val tryLastV : 'T seq -> 'T ValueOption

src/fsharp/FSharp.Core/seq.fs

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,29 +1377,21 @@ namespace Microsoft.FSharp.Collections
13771377
invalidArg "source" (SR.GetString(SR.notEnoughElements))
13781378
while e.MoveNext() do
13791379
yield e.Current }
1380-
1380+
13811381
[<CompiledName("Last")>]
13821382
let last (source : seq<_>) =
13831383
checkNonNull "source" source
1384-
use e = source.GetEnumerator()
1385-
if e.MoveNext() then
1386-
let mutable res = e.Current
1387-
while (e.MoveNext()) do res <- e.Current
1388-
res
1389-
else
1390-
invalidArg "source" LanguagePrimitives.ErrorStrings.InputSequenceEmptyString
1391-
1384+
match Microsoft.FSharp.Primitives.Basics.Seq.tryLastV source with
1385+
| ValueSome x -> x
1386+
| ValueNone -> invalidArg "source" LanguagePrimitives.ErrorStrings.InputSequenceEmptyString
1387+
13921388
[<CompiledName("TryLast")>]
13931389
let tryLast (source : seq<_>) =
13941390
checkNonNull "source" source
1395-
use e = source.GetEnumerator()
1396-
if e.MoveNext() then
1397-
let mutable res = e.Current
1398-
while (e.MoveNext()) do res <- e.Current
1399-
Some res
1400-
else
1401-
None
1402-
1391+
match Microsoft.FSharp.Primitives.Basics.Seq.tryLastV source with
1392+
| ValueSome x -> Some x
1393+
| ValueNone -> None
1394+
14031395
[<CompiledName("ExactlyOne")>]
14041396
let exactlyOne (source : seq<_>) =
14051397
checkNonNull "source" source

0 commit comments

Comments
 (0)