Skip to content

Ensure that JSON properties can be skipped without buffering their entire contents. #96856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 12, 2024
Merged

Ensure that JSON properties can be skipped without buffering their entire contents. #96856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7fb72ef
Ensure that JSON properties can be skipped without buffering their en…
eiriktsarpalis Jan 11, 2024
2f4841f
Merge branch 'main' into feture/async-skip
eiriktsarpalis Jan 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/libraries/System.Text.Json/src/System.Text.Json.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
<Compile Include="System\Text\Json\Serialization\IgnoreReferenceHandler.cs" />
<Compile Include="System\Text\Json\Serialization\JsonConverter.cs" />
<Compile Include="System\Text\Json\Serialization\JsonConverter.MetadataHandling.cs" />
<Compile Include="System\Text\Json\Serialization\JsonConverter.ReadAhead.cs" />
<Compile Include="System\Text\Json\Serialization\JsonConverterFactory.cs" />
<Compile Include="System\Text\Json\Serialization\JsonConverterOfT.ReadCore.cs" />
<Compile Include="System\Text\Json\Serialization\JsonConverterOfT.WriteCore.cs" />
Expand Down Expand Up @@ -393,10 +392,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
</ItemGroup>

<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)System.Text.RegularExpressions\gen\System.Text.RegularExpressions.Generator.csproj"
ReferenceOutputAssembly="false"
SetTargetFramework="TargetFramework=netstandard2.0"
OutputItemType="Analyzer" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Text.RegularExpressions\gen\System.Text.RegularExpressions.Generator.csproj" ReferenceOutputAssembly="false" SetTargetFramework="TargetFramework=netstandard2.0" OutputItemType="Analyzer" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
Expand Down
53 changes: 53 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,59 @@ public static ReadOnlySpan<byte> GetSpan(this scoped ref Utf8JsonReader reader)
return reader.HasValueSequence ? reader.ValueSequence.ToArray() : reader.ValueSpan;
}

/// <summary>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic moved essentially unchanged from JsonConverter.ReadAhead.cs

/// Attempts to perform a Read() operation and optionally checks that the full JSON value has been buffered.
/// The reader will be reset if the operation fails.
/// </summary>
/// <param name="reader">The reader to advance.</param>
/// <param name="requiresReadAhead">If reading a partial payload, read ahead to ensure that the full JSON value has been buffered.</param>
/// <returns>True if the the reader has been buffered with all required data.</returns>
// AggressiveInlining used since this method is on a hot path and short. The AdvanceWithReadAhead method should not be inlined.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool TryAdvanceWithOptionalReadAhead(this scoped ref Utf8JsonReader reader, bool requiresReadAhead)
{
// No read-ahead necessary if we're at the final block of JSON data.
bool readAhead = requiresReadAhead && !reader.IsFinalBlock;
return readAhead ? TryAdvanceWithReadAhead(ref reader) : reader.Read();

// The read-ahead method is not inlined
static bool TryAdvanceWithReadAhead(scoped ref Utf8JsonReader reader)
{
// When we're reading ahead we always have to save the state
// as we don't know if the next token is a start object or array.
Utf8JsonReader restore = reader;

if (!reader.Read())
{
return false;
}

// Perform the actual read-ahead.
JsonTokenType tokenType = reader.TokenType;
if (tokenType is JsonTokenType.StartObject or JsonTokenType.StartArray)
{
// Attempt to skip to make sure we have all the data we need.
bool complete = reader.TrySkipPartial(targetDepth: reader.CurrentDepth);

// We need to restore the state in all cases as we need to be positioned back before
// the current token to either attempt to skip again or to actually read the value.
reader = restore;

if (!complete)
{
// Couldn't read to the end of the object, exit out to get more data in the buffer.
return false;
}

// Success, requeue the reader to the start token.
reader.ReadWithVerify();
Debug.Assert(tokenType == reader.TokenType);
}

return true;
}
}

#if !NETCOREAPP
/// <summary>
/// Returns <see langword="true"/> if <paramref name="value"/> is a valid Unicode scalar
Expand Down
65 changes: 41 additions & 24 deletions src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public readonly int CurrentDepth
get
{
int readerDepth = _bitStack.CurrentDepth;
if (TokenType == JsonTokenType.StartArray || TokenType == JsonTokenType.StartObject)
if (TokenType is JsonTokenType.StartArray or JsonTokenType.StartObject)
{
Debug.Assert(readerDepth >= 1);
readerDepth--;
Expand All @@ -115,7 +115,7 @@ public readonly int CurrentDepth
}
}

internal bool IsInArray => !_inObject;
internal readonly bool IsInArray => !_inObject;

/// <summary>
/// Gets the type of the last processed JSON token in the UTF-8 encoded JSON text.
Expand Down Expand Up @@ -322,15 +322,15 @@ private void SkipHelper()
{
Debug.Assert(_isFinalBlock);

if (TokenType == JsonTokenType.PropertyName)
if (TokenType is JsonTokenType.PropertyName)
{
bool result = Read();
// Since _isFinalBlock == true here, and the JSON token is not a primitive value or comment.
// Read() is guaranteed to return true OR throw for invalid/incomplete data.
Debug.Assert(result);
}

if (TokenType == JsonTokenType.StartObject || TokenType == JsonTokenType.StartArray)
if (TokenType is JsonTokenType.StartObject or JsonTokenType.StartArray)
{
int depth = CurrentDepth;
do
Expand Down Expand Up @@ -375,41 +375,58 @@ public bool TrySkip()
return true;
}

return TrySkipHelper();
Utf8JsonReader restore = this;
bool success = TrySkipPartial(targetDepth: CurrentDepth);
if (!success)
{
// Roll back the reader if it contains partial data.
this = restore;
}

return success;
}

private bool TrySkipHelper()
/// <summary>
/// Tries to skip the children of the current JSON token, advancing the reader even if there is not enough data.
/// The skip operation can be resumed later, provided that the same <paramref name="targetDepth" /> is passed.
/// </summary>
/// <param name="targetDepth">The target depth we want to eventually skip to.</param>
/// <returns>True if the entire JSON value has been skipped.</returns>
internal bool TrySkipPartial(int targetDepth)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing TrySkipHelper has been retrofitted to the resumable TrySkipPartial method, minus the checkpointing semantics.

{
Debug.Assert(!_isFinalBlock);

Utf8JsonReader restore = this;
Debug.Assert(0 <= targetDepth && targetDepth <= CurrentDepth);

if (TokenType == JsonTokenType.PropertyName)
if (targetDepth == CurrentDepth)
{
if (!Read())
// This is the first call to TrySkipHelper.
if (TokenType is JsonTokenType.PropertyName)
{
goto Restore;
// Skip any property name tokens preceding the value.
if (!Read())
{
return false;
}
}

if (TokenType is not (JsonTokenType.StartObject or JsonTokenType.StartArray))
{
// The next value is not an object or array, so there is nothing to skip.
return true;
}
}

if (TokenType == JsonTokenType.StartObject || TokenType == JsonTokenType.StartArray)
// Start or resume iterating through the JSON object or array.
do
{
int depth = CurrentDepth;
do
if (!Read())
{
if (!Read())
{
goto Restore;
}
return false;
}
while (depth < CurrentDepth);
}
while (targetDepth < CurrentDepth);

Debug.Assert(targetDepth == CurrentDepth);
return true;

Restore:
this = restore;
return false;
}

/// <summary>
Expand Down
10 changes: 5 additions & 5 deletions ....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,13 @@ internal override bool OnTryRead(
{
if (state.Current.PropertyState < StackFramePropertyState.ReadValue)
{
state.Current.PropertyState = StackFramePropertyState.ReadValue;

if (!SingleValueReadWithReadAhead(elementConverter.RequiresReadAhead, ref reader, ref state))
if (!reader.TryAdvanceWithOptionalReadAhead(elementConverter.RequiresReadAhead))
{
value = default;
return false;
}

state.Current.PropertyState = StackFramePropertyState.ReadValue;
}

if (state.Current.PropertyState < StackFramePropertyState.ReadValueIsEnd)
Expand Down Expand Up @@ -246,8 +246,6 @@ internal override bool OnTryRead(

if (state.Current.ObjectState < StackFrameObjectState.EndToken)
{
state.Current.ObjectState = StackFrameObjectState.EndToken;

// Array payload is nested inside a $values metadata property.
if ((state.Current.MetadataPropertyNames & MetadataPropertyName.Values) != 0)
{
Expand All @@ -257,6 +255,8 @@ internal override bool OnTryRead(
return false;
}
}

state.Current.ObjectState = StackFrameObjectState.EndToken;
}

if (state.Current.ObjectState < StackFrameObjectState.EndTokenValidation)
Expand Down
10 changes: 5 additions & 5 deletions ....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,14 @@ internal sealed override bool OnTryRead(
{
if (state.Current.PropertyState == StackFramePropertyState.None)
{
state.Current.PropertyState = StackFramePropertyState.ReadName;

// Read the key name.
if (!reader.Read())
{
value = default;
return false;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the order of state update operations since we no longer rely on the root method to advance the reader for us on resumed operations.

state.Current.PropertyState = StackFramePropertyState.ReadName;
}

// Determine the property.
Expand Down Expand Up @@ -274,14 +274,14 @@ internal sealed override bool OnTryRead(

if (state.Current.PropertyState < StackFramePropertyState.ReadValue)
{
state.Current.PropertyState = StackFramePropertyState.ReadValue;

if (!SingleValueReadWithReadAhead(_valueConverter.RequiresReadAhead, ref reader, ref state))
if (!reader.TryAdvanceWithOptionalReadAhead(_valueConverter.RequiresReadAhead))
{
state.Current.DictionaryKey = key;
value = default;
return false;
}

state.Current.PropertyState = StackFramePropertyState.ReadValue;
}

if (state.Current.PropertyState < StackFramePropertyState.TryRead)
Expand Down
37 changes: 11 additions & 26 deletions ....Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,23 +148,20 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
// Determine the property.
if (state.Current.PropertyState == StackFramePropertyState.None)
{
state.Current.PropertyState = StackFramePropertyState.ReadName;

if (!reader.Read())
{
// The read-ahead functionality will do the Read().
state.Current.ReturnValue = obj;
value = default;
return false;
}

state.Current.PropertyState = StackFramePropertyState.ReadName;
}

JsonPropertyInfo jsonPropertyInfo;

if (state.Current.PropertyState < StackFramePropertyState.Name)
{
state.Current.PropertyState = StackFramePropertyState.Name;

JsonTokenType tokenType = reader.TokenType;
if (tokenType == JsonTokenType.EndObject)
{
Expand All @@ -183,6 +180,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
out bool useExtensionProperty);

state.Current.UseExtensionProperty = useExtensionProperty;
state.Current.PropertyState = StackFramePropertyState.Name;
}
else
{
Expand All @@ -194,7 +192,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
{
if (!jsonPropertyInfo.CanDeserializeOrPopulate)
{
if (!reader.TrySkip())
if (!reader.TrySkipPartial(targetDepth: state.Current.OriginalDepth + 1))
{
state.Current.ReturnValue = obj;
value = default;
Expand All @@ -211,6 +209,8 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
value = default;
return false;
}

state.Current.PropertyState = StackFramePropertyState.ReadValue;
}

if (state.Current.PropertyState < StackFramePropertyState.TryRead)
Expand Down Expand Up @@ -258,7 +258,9 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
}

// This method is using aggressive inlining to avoid extra stack frame for deep object graphs.
#if !DEBUG
[MethodImpl(MethodImplOptions.AggressiveInlining)]
#endif
internal static void PopulatePropertiesFastPath(object obj, JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options, ref Utf8JsonReader reader, scoped ref ReadStack state)
{
jsonTypeInfo.OnDeserializing?.Invoke(obj);
Expand Down Expand Up @@ -485,26 +487,9 @@ protected static void ReadPropertyValue(

protected static bool ReadAheadPropertyValue(scoped ref ReadStack state, ref Utf8JsonReader reader, JsonPropertyInfo jsonPropertyInfo)
{
// Returning false below will cause the read-ahead functionality to finish the read.
state.Current.PropertyState = StackFramePropertyState.ReadValue;

if (!state.Current.UseExtensionProperty)
{
if (!SingleValueReadWithReadAhead(jsonPropertyInfo.EffectiveConverter.RequiresReadAhead, ref reader, ref state))
{
return false;
}
}
else
{
// The actual converter is JsonElement, so force a read-ahead.
if (!SingleValueReadWithReadAhead(requiresReadAhead: true, ref reader, ref state))
{
return false;
}
}

return true;
// Extension properties can use the JsonElement converter and thus require read-ahead.
bool requiresReadAhead = jsonPropertyInfo.EffectiveConverter.RequiresReadAhead || state.Current.UseExtensionProperty;
return reader.TryAdvanceWithOptionalReadAhead(requiresReadAhead);
}
}
}
Loading