Seq.empty renders as "EmptyEnumerable"

Seq.empty<MyType> renders as the string "EmptyEnumerable" when fed through AspNetCore 8 ObjectResult

The same does not happen for Seq.choose and other such functions when they yield an empty sequence, as they yield a different internal impl type wrapping their output, which in my context winds up producing [] as required

Expected behavior

render as []

Actual behavior

renders as "EmptyEnumerable"

Known workarounds

use Array.empty<MyType> instead

Related information

happen to be using JSON.NET and a random mix of middlewares and don't have time to root cause deeper, but it surprised me
MyType has a JsonConverter registered
Did not yet try with System.Linq.Enumerable.Empty<MyType>()

I realize this is horrendously incomplete, but I didn't find the condition to be easy to search for, and saying nothing is not ideal either.

If it was me fixing as a quick win without root causing / understanding the required interfaces/moving parts more deeply, I'd make Seq.empty delegate to Array.empty (which is my actual workaround for now), but obviously there's plenty to be won from an optimal implementation too.

Here's hoping I eventually get to the point of circling back with a trimmed down repro that identifies whether this is a JSON.NET problem, a JsonConverter+JSON.NET problem, or only solvable by implementing a relevant interface on EmptyEnumerable

Interesting:

> let es: seq<int> = [];;
val es: int seq = []

> let es2: seq<int> = Seq.empty;;
val es2: int seq

> es;;
val it: int seq = []

> es2;;
val it: int seq = seq []

Yeah, it'd definitely be a more widespread problem if it was something fundamental.
There are a lot of potential things in play with ObjectResult, JSON.NET and JsonConverters in the picture - JSON.NET covers a heck of a lot of scenarios and has plenty idiosyncracies so not ruling anything out.
The biggest thing I can do to narrow it down is use Enumerable.Empty instead in my context and see whether it happens to work correctly, but this unfortunately hasn't made it to the top of the TODO list atm....

I don't think it is in aspnetcore.

> let es: seq<int> = [];;
val es: int seq = []

> let es2: seq<int> = Seq.empty;;
val es2: int seq

> printfn $"{es}";;
[]
val it: unit = ()

> printfn $"{es2}";;
Microsoft.FSharp.Collections.IEnumerator+EmptyEnumerable`1[System.Int32]
val it: unit = ()

But tbh I find EmptyEnumerable more correct than [].

I'm not ruling anything in or out
In my context, [] is much more 'correct' than a sequence mapping to a string JSON value
The specific reasons why fsi will render in a given manner are definitely something that's interesting and relevant, but for me isn't an open and shut answer either.

> printfn $"{System.Linq.Enumerable.Empty<unit>()}"
System.Linq.EmptyPartition`1[Microsoft.FSharp.Core.Unit]

(But I have yet to test how that renders either - All I know for a fact is that Array.empty renders equivalent to the wrapper that Seq.choose yields)

F# type checking infers es in the above example to be of type int list and es2 to be of type ÈmptyEnumerable.
I don't think there is anything wrong with it.
I guess that in the case that you mention above, using Seq.choose, there is a reason why type checking infers a list or array.

Yeah, it's not entirely/only about the static type from the perspective of the compiler though - ultimately ObjectResult is not strongly typed (the data is an obj), so it's about the interfaces and other aspects of the wrapper type; that's what dictates how JSON.NET and/or AspNetCore's wiring ends up rendering the result. (Seq.empty casts the internalt wrapper type to seq<'t>, but that turns out not to be enough)

So overall, changing current implementation to Array.empty gives performance boost everywhere. I can create a PR if this change is approved.

Changing it to Array.empty is technically semantically incorrect. Also, a breaking change. It needs to be assessed from the perspective of consuming both IL as well as having it in sigdata/inline data when having different versions of compile-/run-time dependencies.

It will also need to show how initial problem is solved with changing implementation.

@Lanayx

UPDATED: Indeed, the fixed version is much faster with zero allocations, current implemention array.Clone() is extremely slow

module My =
let toArray (source: seq<'T>) =
match source with
| :? ('T array) as res -> res.ToArray()
| _ -> source |> Seq.toArray

This change makes sense. Clone as a low level primitive always allocated.
But with an empty array, this is not needed, as the same 0-length array can be reused and contents of 0 cannot be mutated - ToArray() in LINQ special cases the Lenght=0 situation.

This should also bring other benefits for tfm9+, as the doubling algorithm now allocated less on heap and is optimized for a few more special cases - dotnet/runtime#96570

And LINQ methods in net9 are also checking for array empty in their optimized code paths now - dotnet/runtime#96602 (note this hasn't been the case all the time).

==> I do see the benefit now in the arguments @Lanayx provided here. I assume this also will also circumvent the ObjectResult behavior just fine.

Per comments in EmptyEnumerable, it may be a good idea to consider delegating to Enumerable.Empty in order to benefit from current and/or future special casing it has.

Indeed, that would be my suggestion as well.
As of now, it does indeed return a shared singleton empty array, but it does not have to stay like this forever.

As of now, it does indeed return a shared singleton empty array

Decompiled sources say it returns EmptyPartition

But I don't mind using Enumerable.Empty anyway instead of SCDU, although this would require check other FSharp.Core array functions to make sure that they end up using optimized methods (like EmptyPartition.ToArray etc)

Additional question whether Fable can rely on types when they transpile. Changing type might be breaking for them (hopefully they don't rely on EmptyEnumerable directly though).

No, this won't break anything for Fable, since it relies on the IEnumerable<'T> interface, and uses custom implementation, so internal .NET implementation doesn't affect it in any way:

let x = Seq.empty<int>

gets translated into

import { empty } from "fable-library-js/Seq.js";

export const x = empty();

And actually it does return empty array in Fable :)

Good to know

Fix Seq.empty rendering as "EmptyEnumerable" in serializers (dotnet#1…

Change Seq.empty to use Enumerable.Empty

Fix Seq.empty rendering as "EmptyEnumerable" in serializers (dotnet#1…

Navigation Menu

Repository navigation

Seq.empty renders as `"EmptyEnumerable"` #17864

28 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Seq.empty renders as "EmptyEnumerable" #17864

Description

Activity

bartelink commented on Oct 10, 2024

Martin521 commented on Oct 10, 2024

bartelink commented on Oct 10, 2024

Martin521 commented on Oct 10, 2024

bartelink commented on Oct 10, 2024

Martin521 commented on Oct 10, 2024

bartelink commented on Oct 10, 2024

28 remaining items

Lanayx commented on Dec 2, 2024

vzarytovskii commented on Dec 2, 2024

T-Gro commented on Dec 2, 2024

bartelink commented on Dec 2, 2024

T-Gro commented on Dec 2, 2024

Lanayx commented on Dec 2, 2024

vzarytovskii commented on Dec 2, 2024

Lanayx commented on Dec 2, 2024

vzarytovskii commented on Dec 2, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Seq.empty renders as `"EmptyEnumerable"` #17864