Add JsonElement.ValueSpan

[krwq]

Updated API Proposal

Following conversation with @annelo-msft we identified the key use case is offering a non-allocating variant of the JsonElement.GetRawText method. As such we should probably go ahead with the JsonMarshal design:

namespace System.Runtime.InteropServices;

public partial class JsonMarshal
{
    public static bool TryGetRawValue(JsonElement element, out ReadOnlySpan<byte> rawValue);
}

Furthermore:

1. The raw value should always be a valid JSON value, therefore strings/properties must always include quotes.
2. Composite values preserve whitespace and comments contained in the original JSON document.

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Currently Utf8JsonReader exposes ValueSpan. It would be useful to have the same member on JsonElement.

cc: @eiriktsarpalis @KrzysztofCwalina

Author:	krwq
Assignees:	-
Labels:	api-suggestion, area-System.Text.Json
Milestone:	-

[mwadams]

While I like this suggestion in principle, there has been much discussion elsewhere about the level of abstraction expected from JsonElement, and whether this should be providing an encoded or decoded span, and (optionally) efficient conversion to ReadOnlySpan<char>.

There is a proposal in #74028 which addresses these issues for the JsonValueKind.String case (which is, in general, the only time you actually need this API).

[eiriktsarpalis]

One concern I have with this proposal is that similar to Utf8JsonReader.ValueSpan this would not be accounted for escaped JSON strings (per section 7 of RFC 8259). Users would need to account for this and implement their own unescaping routines. Bare minimum, this should be accompanied with a JsonElement.ValueIsEscaped flag and possibly a CopyString() method that copies the unescaped string to a buffer provided by the caller.

[eiriktsarpalis]

FWIW this was considered in the past and rejected, cf. #30207 (comment) cc @bartonjs

[bartonjs]

A CopyString method, either escaping or not (maybe CopyString (unescaping) and CopyRawString (only transcoding, when needed)?), would be OK. The Span property is inappropriate, because the lifetime isn't obvious.

ReadOnlySpan<byte> propValRaw = element.ValueSpan;
DoSomeStuff();
DoMoreStuff(propValRaw);

If DoSomeStuff disposes the JsonDocument that produced the element, then that span is probably pointing to the middle of a buffer that was returned to the array pool. If that array got re-rented, it's now garbage data. So, as a property this fails the guideline

AVOID returning a Span<T> or ReadOnlySpan<T> unless the lifetime of the returned Span is very clear.

The "I'll copy it to your buffer for you" method is more clear, and someone who needs the hyper-performance of inspecting it just has to drop to using the reader directly.

[bartonjs]

Also, for what it's worth, I still think that anything that removes the possibility of UTF-8-or-UTF-16(-or-other) from JsonDocument/JsonElement is bad... which also favors the copy method over the property.

[eiriktsarpalis]

If DoSomeStuff disposes the JsonDocument that produced the element, then that span is probably pointing to the middle of a buffer that was returned to the array pool. If that array got re-rented, it's now garbage data. So, as a property this fails the guideline

Is this by virtue of the fact that JsonDocument is IDisposable? Because one could argue that a similar pattern is possible with Utf8JsonReader.ValueSpan if users return the buffer before the span gets consumed.

cc @KrzysztofCwalina who had mentioned in the past that a copying approach would not meet their requirements.