Skip to content

Commit 4102a94

Browse files
committed
Fix to #31159 - Query/Json: use Utf8JsonReader in materializer
Using Utf8JsonReader to read JSON data rather than caching it using DOM. This should reduce allocations significantly. Tricky part is that entity materializers are build in a way that assumes we have random access to all the data we need. This is not the case here. We read JSON data sequentially and can only do it once, and we don't know the order in which we get the data. This is somewhat problematic in case where entity takes argument in the constructor. Those could be at the very end of the JSON string, so we must read all the data before we can instantiate the object, and populate it's properties and do navigation fixup. This requires us reading all the JSON data, store them in local variables, and only when we are done reading we instantiate the entity and populate all the properties with data stored in those variables. This adds some allocations (specifically navigations). We also have to disable de-duplication logic - we can't always safely re-read the JSON string, and definitely can't start reading it from arbitrary position, so now we have to add JSON string for every aggregate projected, even if we already project it's parent. Also fix to #30993 - Query/Json: data corruption for tracking queries with nested json entities, then updating nested entities outside EF and re-querying Fix is to recognize and modify shaper in case of tracking query, so that nav expansions are not skipped when parent entity is found in Change Tracker. This is necessary to fix alongside streaming, because now we throw exception from reader (unexpected token) if we don't process the entire stream correctly. Before it would be silently ignored apart from the edge case described in the bug. Fixes #31159 Fixes #30993
1 parent 81c2440 commit 4102a94

18 files changed

+1858
-599
lines changed

Diff for: src/EFCore.Relational/Query/Internal/JsonProjectionInfo.cs

+1-15
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@ public readonly struct JsonProjectionInfo
1919
/// </summary>
2020
public JsonProjectionInfo(
2121
int jsonColumnIndex,
22-
List<(IProperty?, int?, int?)> keyAccessInfo,
23-
(string?, int?, int?)[] additionalPath)
22+
List<(IProperty?, int?, int?)> keyAccessInfo)
2423
{
2524
JsonColumnIndex = jsonColumnIndex;
2625
KeyAccessInfo = keyAccessInfo;
27-
AdditionalPath = additionalPath;
2826
}
2927

3028
/// <summary>
@@ -55,16 +53,4 @@ public JsonProjectionInfo(
5553
/// doing so can result in application failures when updating to a new Entity Framework Core release.
5654
/// </remarks>
5755
public List<(IProperty? KeyProperty, int? ConstantKeyValue, int? KeyProjectionIndex)> KeyAccessInfo { get; }
58-
59-
/// <summary>
60-
/// List of additional path elements, only one of the values in the tuple is non-null
61-
/// this information is used to access the correct sub-element of a JsonElement that we materialized
62-
/// </summary>
63-
/// <remarks>
64-
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
65-
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
66-
/// any release. You should only use it directly in your code with extreme caution and knowing that
67-
/// doing so can result in application failures when updating to a new Entity Framework Core release.
68-
/// </remarks>
69-
public (string? JsonPropertyName, int? ConstantArrayIndex, int? NonConstantArrayIndex)[] AdditionalPath { get; }
7056
}

Diff for: src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs

+2-8
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ private static readonly MethodInfo GetParameterValueMethodInfo
2525

2626
private bool _indexBasedBinding;
2727
private Dictionary<EntityProjectionExpression, ProjectionBindingExpression>? _entityProjectionCache;
28-
private Dictionary<JsonQueryExpression, ProjectionBindingExpression>? _jsonQueryCache;
2928
private List<Expression>? _clientProjections;
3029

3130
private readonly Dictionary<ProjectionMember, Expression> _projectionMapping = new();
@@ -66,7 +65,6 @@ public virtual Expression Translate(SelectExpression selectExpression, Expressio
6665
{
6766
_indexBasedBinding = true;
6867
_entityProjectionCache = new Dictionary<EntityProjectionExpression, ProjectionBindingExpression>();
69-
_jsonQueryCache = new Dictionary<JsonQueryExpression, ProjectionBindingExpression>();
7068
_projectionMapping.Clear();
7169
_clientProjections = new List<Expression>();
7270

@@ -307,11 +305,8 @@ protected override Expression VisitExtension(Expression extensionExpression)
307305
{
308306
if (_indexBasedBinding)
309307
{
310-
if (!_jsonQueryCache!.TryGetValue(jsonQueryExpression, out var jsonProjectionBinding))
311-
{
312-
jsonProjectionBinding = AddClientProjection(jsonQueryExpression, typeof(ValueBuffer));
313-
_jsonQueryCache[jsonQueryExpression] = jsonProjectionBinding;
314-
}
308+
_clientProjections!.Add(jsonQueryExpression);
309+
var jsonProjectionBinding = new ProjectionBindingExpression(_selectExpression, _clientProjections.Count - 1, typeof(ValueBuffer));
315310

316311
return entityShaperExpression.Update(jsonProjectionBinding);
317312
}
@@ -654,7 +649,6 @@ private ProjectionBindingExpression AddClientProjection(Expression expression, T
654649
return new ProjectionBindingExpression(_selectExpression, existingIndex, type);
655650
}
656651

657-
#pragma warning disable IDE0052 // Remove unread private members
658652
private static T GetParameterValue<T>(QueryContext queryContext, string parameterName)
659653
#pragma warning restore IDE0052 // Remove unread private members
660654
=> (T)queryContext.ParameterValues[parameterName]!;

Diff for: src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs

+123-61
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Text.Json;
66
using Microsoft.EntityFrameworkCore.Internal;
77
using Microsoft.EntityFrameworkCore.Query.Internal;
8+
using Microsoft.EntityFrameworkCore.Storage.Json;
89

910
namespace Microsoft.EntityFrameworkCore.Query;
1011

@@ -67,8 +68,8 @@ private static readonly MethodInfo MaterializeJsonEntityMethodInfo
6768
private static readonly MethodInfo MaterializeJsonEntityCollectionMethodInfo
6869
= typeof(ShaperProcessingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(MaterializeJsonEntityCollection))!;
6970

70-
private static readonly MethodInfo ExtractJsonPropertyMethodInfo
71-
= typeof(ShaperProcessingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(ExtractJsonProperty))!;
71+
private static readonly MethodInfo InverseCollectionFixupMethod
72+
= typeof(ShaperProcessingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(InverseCollectionFixup))!;
7273

7374
[MethodImpl(MethodImplOptions.AggressiveInlining)]
7475
private static TValue ThrowReadValueException<TValue>(
@@ -123,13 +124,6 @@ private static TValue ThrowExtractJsonPropertyException<TValue>(
123124
exception);
124125
}
125126

126-
private static T? ExtractJsonProperty<T>(JsonElement element, string propertyName, bool nullable)
127-
=> nullable
128-
? element.TryGetProperty(propertyName, out var jsonValue)
129-
? jsonValue.Deserialize<T>()
130-
: default
131-
: element.GetProperty(propertyName).Deserialize<T>();
132-
133127
private static void IncludeReference<TEntity, TIncludingEntity, TIncludedEntity>(
134128
QueryContext queryContext,
135129
TEntity entity,
@@ -869,105 +863,173 @@ static async Task<RelationalDataReader> InitializeReaderAsync(
869863
dataReaderContext.HasNext = false;
870864
}
871865

872-
private static void IncludeJsonEntityReference<TIncludingEntity, TIncludedEntity>(
866+
private static TEntity? MaterializeJsonEntity<TEntity>(
873867
QueryContext queryContext,
874-
JsonElement? jsonElement,
875868
object[] keyPropertyValues,
876-
TIncludingEntity entity,
877-
Func<QueryContext, object[], JsonElement, TIncludedEntity> innerShaper,
878-
Action<TIncludingEntity, TIncludedEntity> fixup)
879-
where TIncludingEntity : class
880-
where TIncludedEntity : class
869+
JsonReaderData jsonReaderData,
870+
bool nullable,
871+
Func<QueryContext, object[], JsonReaderData, TEntity> shaper)
872+
where TEntity : class
881873
{
882-
if (jsonElement.HasValue && jsonElement.Value.ValueKind != JsonValueKind.Null)
874+
if (jsonReaderData == null)
875+
{
876+
return nullable
877+
? default
878+
: throw new InvalidOperationException(
879+
RelationalStrings.JsonRequiredEntityWithNullJson(typeof(TEntity).Name));
880+
}
881+
882+
var manager = new Utf8JsonReaderManager(jsonReaderData);
883+
884+
if (manager.CurrentReader.TokenType == JsonTokenType.Null)
883885
{
884-
var included = innerShaper(queryContext, keyPropertyValues, jsonElement.Value);
885-
fixup(entity, included);
886+
return nullable
887+
? default
888+
: throw new InvalidOperationException(
889+
RelationalStrings.JsonRequiredEntityWithNullJson(typeof(TEntity).Name));
886890
}
891+
892+
if (manager.CurrentReader.TokenType == JsonTokenType.StartObject)
893+
{
894+
manager.CaptureState();
895+
var result = shaper(queryContext, keyPropertyValues, jsonReaderData);
896+
897+
return result;
898+
}
899+
900+
// TODO: cleanup & add strings when all is ready
901+
throw new InvalidOperationException("Invalid token type: " + manager.CurrentReader.TokenType.ToString());
887902
}
888903

889-
private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedCollectionElement>(
904+
private static TResult? MaterializeJsonEntityCollection<TEntity, TResult>(
890905
QueryContext queryContext,
891-
JsonElement? jsonElement,
892906
object[] keyPropertyValues,
893-
TIncludingEntity entity,
894-
Func<QueryContext, object[], JsonElement, TIncludedCollectionElement> innerShaper,
895-
Action<TIncludingEntity, TIncludedCollectionElement> fixup)
896-
where TIncludingEntity : class
897-
where TIncludedCollectionElement : class
907+
JsonReaderData jsonReaderData,
908+
INavigationBase navigation,
909+
Func<QueryContext, object[], JsonReaderData, TEntity> innerShaper)
910+
where TEntity : class
911+
where TResult : ICollection<TEntity>
898912
{
899-
if (jsonElement.HasValue && jsonElement.Value.ValueKind != JsonValueKind.Null)
913+
if (jsonReaderData == null)
900914
{
915+
return default;
916+
}
917+
918+
var manager = new Utf8JsonReaderManager(jsonReaderData);
919+
var tokenType = manager.CurrentReader.TokenType;
920+
if (tokenType == JsonTokenType.StartArray)
921+
{
922+
var collectionAccessor = navigation.GetCollectionAccessor();
923+
var result = (TResult)collectionAccessor!.Create();
924+
901925
var newKeyPropertyValues = new object[keyPropertyValues.Length + 1];
902926
Array.Copy(keyPropertyValues, newKeyPropertyValues, keyPropertyValues.Length);
903927

928+
tokenType = manager.MoveNext();
929+
904930
var i = 0;
905-
foreach (var jsonArrayElement in jsonElement.Value.EnumerateArray())
931+
while (tokenType != JsonTokenType.EndArray)
906932
{
907933
newKeyPropertyValues[^1] = ++i;
908934

909-
var resultElement = innerShaper(queryContext, newKeyPropertyValues, jsonArrayElement);
935+
if (tokenType == JsonTokenType.StartObject)
936+
{
937+
manager.CaptureState();
938+
var entity = innerShaper(queryContext, newKeyPropertyValues, jsonReaderData);
939+
result.Add(entity);
940+
manager = new Utf8JsonReaderManager(manager.Data);
941+
942+
if (manager.CurrentReader.TokenType != JsonTokenType.EndObject)
943+
{
944+
// TODO: cleanup & add strings when all is ready
945+
throw new InvalidOperationException("expecting end object, got: " + tokenType.ToString());
946+
}
910947

911-
fixup(entity, resultElement);
948+
tokenType = manager.MoveNext();
949+
}
912950
}
951+
952+
manager.CaptureState();
953+
954+
return result;
913955
}
956+
957+
// TODO: cleanup & add strings when all is ready
958+
throw new InvalidOperationException("Expecting StartArray token, got: " + tokenType.ToString());
914959
}
915960

916-
private static TEntity? MaterializeJsonEntity<TEntity>(
961+
private static void IncludeJsonEntityReference<TIncludingEntity, TIncludedEntity>(
917962
QueryContext queryContext,
918-
JsonElement? jsonElement,
919963
object[] keyPropertyValues,
920-
bool nullable,
921-
Func<QueryContext, object[], JsonElement, TEntity> shaper)
922-
where TEntity : class
964+
JsonReaderData jsonReaderData,
965+
TIncludingEntity entity,
966+
Func<QueryContext, object[], JsonReaderData, TIncludedEntity> innerShaper,
967+
Action<TIncludingEntity, TIncludedEntity> fixup)
968+
where TIncludingEntity : class
969+
where TIncludedEntity : class
923970
{
924-
if (jsonElement.HasValue && jsonElement.Value.ValueKind != JsonValueKind.Null)
925-
{
926-
var result = shaper(queryContext, keyPropertyValues, jsonElement.Value);
927-
928-
return result;
929-
}
930-
931-
if (nullable)
971+
if (jsonReaderData == null)
932972
{
933-
return default;
973+
return;
934974
}
935975

936-
throw new InvalidOperationException(
937-
RelationalStrings.JsonRequiredEntityWithNullJson(typeof(TEntity).Name));
976+
var included = innerShaper(queryContext, keyPropertyValues, jsonReaderData);
977+
fixup(entity, included);
938978
}
939979

940-
private static TResult? MaterializeJsonEntityCollection<TEntity, TResult>(
980+
private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedCollectionElement>(
941981
QueryContext queryContext,
942-
JsonElement? jsonElement,
943982
object[] keyPropertyValues,
944-
INavigationBase navigation,
945-
Func<QueryContext, object[], JsonElement, TEntity> innerShaper)
946-
where TEntity : class
947-
where TResult : ICollection<TEntity>
983+
JsonReaderData jsonReaderData,
984+
TIncludingEntity entity,
985+
Func<QueryContext, object[], JsonReaderData, TIncludedCollectionElement> innerShaper,
986+
Action<TIncludingEntity, TIncludedCollectionElement> fixup)
987+
where TIncludingEntity : class
988+
where TIncludedCollectionElement : class
948989
{
949-
if (jsonElement.HasValue && jsonElement.Value.ValueKind != JsonValueKind.Null)
990+
if (jsonReaderData == null)
950991
{
951-
var collectionAccessor = navigation.GetCollectionAccessor();
952-
var result = (TResult)collectionAccessor!.Create();
992+
return;
993+
}
953994

995+
var manager = new Utf8JsonReaderManager(jsonReaderData);
996+
var tokenType = manager.CurrentReader.TokenType;
997+
if (tokenType == JsonTokenType.StartArray)
998+
{
954999
var newKeyPropertyValues = new object[keyPropertyValues.Length + 1];
9551000
Array.Copy(keyPropertyValues, newKeyPropertyValues, keyPropertyValues.Length);
9561001

1002+
tokenType = manager.MoveNext();
1003+
9571004
var i = 0;
958-
foreach (var jsonArrayElement in jsonElement.Value.EnumerateArray())
1005+
while (tokenType != JsonTokenType.EndArray)
9591006
{
9601007
newKeyPropertyValues[^1] = ++i;
9611008

962-
var resultElement = innerShaper(queryContext, newKeyPropertyValues, jsonArrayElement);
1009+
if (tokenType == JsonTokenType.StartObject)
1010+
{
1011+
manager.CaptureState();
1012+
var resultElement = innerShaper(queryContext, newKeyPropertyValues, jsonReaderData);
1013+
fixup(entity, resultElement);
1014+
manager = new Utf8JsonReaderManager(manager.Data);
9631015

964-
result.Add(resultElement);
1016+
if (manager.CurrentReader.TokenType != JsonTokenType.EndObject)
1017+
{
1018+
// TODO: cleanup & add strings when all is ready
1019+
throw new InvalidOperationException("expecting end object, got: " + tokenType.ToString());
1020+
}
1021+
1022+
tokenType = manager.MoveNext();
1023+
}
9651024
}
9661025

967-
return result;
1026+
manager.CaptureState();
1027+
}
1028+
else
1029+
{
1030+
// TODO: cleanup & add strings when all is ready
1031+
throw new InvalidOperationException("Expecting StartArray token, got: " + tokenType.ToString());
9681032
}
969-
970-
return default;
9711033
}
9721034

9731035
private static async Task TaskAwaiter(Func<Task>[] taskFactories)

0 commit comments

Comments
 (0)