Skip to content
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.

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

Add DuplicatePropertyNameChecker to ObjectPool #2328

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b8f75ec
Add DuplicatePropertyNameChecker to ObjectPool
KenitoInc Feb 25, 2022
8dff302
Fix Action delegate and add tests
KenitoInc Feb 28, 2022
9ecf332
Release DuplicatePropertyNameChecker at the end of scope
KenitoInc Mar 2, 2022
505af7e
Remove wrong tags
KenitoInc Mar 4, 2022
4a03fbf
Remove unused Lists
KenitoInc Mar 7, 2022
884059f
Change list ops
KenitoInc Mar 7, 2022
23359a8
Refactor ObjectPool
KenitoInc Mar 8, 2022
24ed770
More optimizations
KenitoInc Mar 8, 2022
6facb2f
Fix csharp version issue
KenitoInc Mar 8, 2022
e0ea495
Add docstring
KenitoInc Mar 8, 2022
c351053
Fix based on review comments
KenitoInc Mar 16, 2022
eea468b
Fixes named argument specifications
KenitoInc Mar 16, 2022
734388a
review comments changes
KenitoInc Mar 18, 2022
16791b4
Return elements to pool
KenitoInc Mar 18, 2022
7d481f7
Return object to pool
KenitoInc Mar 18, 2022
1f3e72c
Code refactoring
KenitoInc Mar 23, 2022
abb2f5c
Remove unused property
KenitoInc Mar 23, 2022
d103f5b
Refactor how we handle NullDuplicatePropertyNameChecker
KenitoInc Mar 23, 2022
39dffb8
Return instance from LightParameterWriter
KenitoInc Mar 24, 2022
a78ceaf
Return more objects to the pool
KenitoInc Mar 24, 2022
6c8be85
modify ObjectPool
KenitoInc Mar 24, 2022
409faea
Dispose ObjectPool and excess array items
KenitoInc Mar 31, 2022
2a27da5
Code cleanup
KenitoInc Apr 1, 2022
4afdaa5
Use DefaultObjectPool
KenitoInc Apr 11, 2022
2b95462
Cleanup WriterValidator
KenitoInc Apr 12, 2022
0090551
Minor style fixes
KenitoInc Apr 12, 2022
af6293b
Fix review comments
KenitoInc Apr 14, 2022
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
11 changes: 8 additions & 3 deletions src/Microsoft.OData.Core/IWriterValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@ namespace Microsoft.OData
internal interface IWriterValidator
{
/// <summary>
/// Creates a DuplicatePropertyNameChecker instance.
/// Get a DuplicatePropertyNameChecker instance from the object pool.
/// </summary>
/// <returns>The created instance.</returns>
IDuplicatePropertyNameChecker CreateDuplicatePropertyNameChecker();
/// <returns>The instance retrieved from the object pool.</returns>
IDuplicatePropertyNameChecker GetDuplicatePropertyNameChecker();

/// <summary>
/// Return a DuplicatePropertyNameChecker instance to the object pool.
/// </summary>
void ReturnDuplicatePropertyNameChecker(IDuplicatePropertyNameChecker duplicatePropertyNameChecker);

/// <summary>
/// Validates a resource in an expanded link to make sure that the types match.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,13 @@ internal void WriteInstanceAnnotation(
ODataResourceValue resourceValue = value as ODataResourceValue;
if (resourceValue != null)
{
IDuplicatePropertyNameChecker duplicatePropertyNameChecker = this.valueSerializer.GetDuplicatePropertyNameChecker();
this.WriteInstanceAnnotationName(propertyName, name);
this.valueSerializer.WriteResourceValue(resourceValue,
expectedType,
treatLikeOpenProperty,
this.valueSerializer.CreateDuplicatePropertyNameChecker());
duplicatePropertyNameChecker);
this.valueSerializer.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);
return;
}

Expand Down Expand Up @@ -389,12 +391,15 @@ await this.valueSerializer.WriteNullValueAsync()
ODataResourceValue resourceValue = annotationValue as ODataResourceValue;
if (resourceValue != null)
{
IDuplicatePropertyNameChecker duplicatePropertyNameChecker = this.valueSerializer.GetDuplicatePropertyNameChecker();
await this.WriteInstanceAnnotationNameAsync(propertyName, annotationName)
.ConfigureAwait(false);
await this.valueSerializer.WriteResourceValueAsync(resourceValue,
expectedType,
treatLikeOpenProperty,
this.valueSerializer.CreateDuplicatePropertyNameChecker()).ConfigureAwait(false);
duplicatePropertyNameChecker).ConfigureAwait(false);
this.valueSerializer.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);

return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ protected override void StartCollection(ODataCollectionStart collectionStart)
/// </summary>
protected override void EndCollection()
{
this.jsonLightCollectionSerializer.ReturnDuplicatePropertyNameChecker(this.DuplicatePropertyNameChecker);
this.jsonLightCollectionSerializer.WriteCollectionEnd();
}

Expand Down Expand Up @@ -199,6 +200,8 @@ protected override Task StartCollectionAsync(ODataCollectionStart collectionStar
/// <returns>A task that represents the asynchronous write operation.</returns>
protected override Task EndCollectionAsync()
{
this.jsonLightCollectionSerializer.ReturnDuplicatePropertyNameChecker(this.DuplicatePropertyNameChecker);

return this.jsonLightCollectionSerializer.WriteCollectionEndAsync();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ protected override void StartPayload()
protected override void EndPayload()
{
// NOTE: we are always writing a request payload here.
this.jsonLightValueSerializer.ReturnDuplicatePropertyNameChecker(this.DuplicatePropertyNameChecker);
this.jsonLightOutputContext.JsonWriter.EndObjectScope();
this.jsonLightValueSerializer.WritePayloadEnd();
}
Expand Down Expand Up @@ -174,6 +175,7 @@ await this.jsonLightOutputContext.AsynchronousJsonWriter.StartObjectScopeAsync()
protected override async Task EndPayloadAsync()
{
// NOTE: we are always writing a request payload here.
this.jsonLightValueSerializer.ReturnDuplicatePropertyNameChecker(this.DuplicatePropertyNameChecker);
await this.jsonLightOutputContext.AsynchronousJsonWriter.EndObjectScopeAsync()
.ConfigureAwait(false);
await this.jsonLightValueSerializer.WritePayloadEndAsync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,17 @@ internal void WriteTopLevelProperty(ODataProperty property)

// Note we do not allow named stream properties to be written as top level property.
this.JsonLightValueSerializer.AssertRecursionDepthIsZero();
IDuplicatePropertyNameChecker duplicatePropertyNameChecker = this.GetDuplicatePropertyNameChecker();

this.WriteProperty(
property,
null /*owningType*/,
true /* isTopLevel */,
this.CreateDuplicatePropertyNameChecker(),
null /* metadataBuilder */);
this.JsonLightValueSerializer.AssertRecursionDepthIsZero();
owningType: null,
isTopLevel: true,
duplicatePropertyNameChecker: duplicatePropertyNameChecker,
metadataBuilder : null);

this.JsonLightValueSerializer.AssertRecursionDepthIsZero();
this.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);
this.JsonWriter.EndObjectScope();
});
}
Expand Down Expand Up @@ -276,14 +279,17 @@ await this.WriteContextUriPropertyAsync(kind, () => contextInfo)

// Note we do not allow named stream properties to be written as top level property.
this.JsonLightValueSerializer.AssertRecursionDepthIsZero();
IDuplicatePropertyNameChecker duplicatePropertyNameChecker = this.GetDuplicatePropertyNameChecker();

await this.WritePropertyAsync(
property,
null /*owningType*/,
true /* isTopLevel */,
this.CreateDuplicatePropertyNameChecker(),
null /* metadataBuilder */).ConfigureAwait(false);
this.JsonLightValueSerializer.AssertRecursionDepthIsZero();
owningType : null,
isTopLevel : true,
duplicatePropertyNameChecker : duplicatePropertyNameChecker,
metadataBuilder : null).ConfigureAwait(false);

this.JsonLightValueSerializer.AssertRecursionDepthIsZero();
this.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);
await this.AsynchronousJsonWriter.EndObjectScopeAsync().ConfigureAwait(false);
});
}
Expand Down Expand Up @@ -645,11 +651,15 @@ private void WriteResourceProperty(
Debug.Assert(!this.currentPropertyInfo.IsTopLevel, "Resource property should not be top level");
this.JsonWriter.WriteName(property.Name);

IDuplicatePropertyNameChecker duplicatePropertyNameChecker = this.GetDuplicatePropertyNameChecker();

this.JsonLightValueSerializer.WriteResourceValue(
resourceValue,
this.currentPropertyInfo.MetadataType.TypeReference,
isOpenPropertyType,
this.CreateDuplicatePropertyNameChecker());
duplicatePropertyNameChecker);

this.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);
}

/// <summary>
Expand Down Expand Up @@ -1071,11 +1081,15 @@ private async Task WriteResourcePropertyAsync(
await this.AsynchronousJsonWriter.WriteNameAsync(property.Name)
.ConfigureAwait(false);

IDuplicatePropertyNameChecker duplicatePropertyNameChecker = this.GetDuplicatePropertyNameChecker();

await this.JsonLightValueSerializer.WriteResourceValueAsync(
resourceValue,
this.currentPropertyInfo.MetadataType.TypeReference,
isOpenPropertyType,
this.CreateDuplicatePropertyNameChecker()).ConfigureAwait(false);
duplicatePropertyNameChecker).ConfigureAwait(false);

this.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public virtual void WriteCollectionValue(
{
if (duplicatePropertyNamesChecker == null)
{
duplicatePropertyNamesChecker = this.CreateDuplicatePropertyNameChecker();
duplicatePropertyNamesChecker = this.GetDuplicatePropertyNameChecker();
}

this.WriteResourceValue(
Expand Down Expand Up @@ -286,6 +286,11 @@ public virtual void WriteCollectionValue(
}
}
}

if (duplicatePropertyNamesChecker != null)
{
this.ReturnDuplicatePropertyNameChecker(duplicatePropertyNamesChecker);
}
}

// End the array scope which holds the items
Expand Down Expand Up @@ -567,7 +572,7 @@ await this.AsynchronousODataAnnotationWriter.WriteODataTypeInstanceAnnotationAsy
{
if (duplicatePropertyNamesChecker == null)
{
duplicatePropertyNamesChecker = this.CreateDuplicatePropertyNameChecker();
duplicatePropertyNamesChecker = this.GetDuplicatePropertyNameChecker();
}

await this.WriteResourceValueAsync(
Expand Down Expand Up @@ -608,6 +613,11 @@ await this.WriteResourceValueAsync(
}
}
}

if (duplicatePropertyNamesChecker != null)
{
this.ReturnDuplicatePropertyNameChecker(duplicatePropertyNamesChecker);
}
}

// End the array scope which holds the items
Expand Down
6 changes: 6 additions & 0 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,9 @@ protected override void EndResource(ODataResource resource)

this.jsonLightResourceSerializer.WriteResourceEndMetadataProperties(resourceScope, resourceScope.DuplicatePropertyNameChecker);

// Release the DuplicatePropertyNameChecker to the object pool since an instance is removed from the object pool each time we create a ResourceScope.
this.jsonLightResourceSerializer.ReturnDuplicatePropertyNameChecker(resourceScope.DuplicatePropertyNameChecker);

// Close the object scope
this.jsonWriter.EndObjectScope();
}
Expand Down Expand Up @@ -1438,6 +1441,9 @@ await this.jsonLightResourceSerializer.WriteResourceEndMetadataPropertiesAsync(
resourceScope,
resourceScope.DuplicatePropertyNameChecker).ConfigureAwait(false);

// Release the DuplicatePropertyNameChecker to the object pool since an instance is removed from the object pool each time we create a ResourceScope.
this.jsonLightResourceSerializer.ReturnDuplicatePropertyNameChecker(resourceScope.DuplicatePropertyNameChecker);

// Close the object scope
await this.asynchronousJsonWriter.EndObjectScopeAsync()
.ConfigureAwait(false);
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.OData.Core/Microsoft.OData.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces">
<Version>5.0.0</Version>
</PackageReference>
<PackageReference Include="Microsoft.Extensions.ObjectPool">
<Version>6.0.3</Version>
</PackageReference>
</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion src/Microsoft.OData.Core/ODataCollectionWriterCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ protected IDuplicatePropertyNameChecker DuplicatePropertyNameChecker
{
return duplicatePropertyNameChecker
?? (duplicatePropertyNameChecker
= outputContext.MessageWriterSettings.Validator.CreateDuplicatePropertyNameChecker());
= outputContext.MessageWriterSettings.Validator.GetDuplicatePropertyNameChecker());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OData.Core/ODataParameterWriterCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected IDuplicatePropertyNameChecker DuplicatePropertyNameChecker
return this.duplicatePropertyNameChecker ??
(this.duplicatePropertyNameChecker =
outputContext.MessageWriterSettings.Validator
.CreateDuplicatePropertyNameChecker());
.GetDuplicatePropertyNameChecker());
}
}

Expand Down
16 changes: 12 additions & 4 deletions src/Microsoft.OData.Core/ODataSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,20 @@ internal IEdmModel Model
}

/// <summary>
/// Creates a new instance of a duplicate property names checker.
/// Get an instance of a duplicate property names checker from the object pool.
/// </summary>
/// <returns>The newly created instance of duplicate property names checker.</returns>
internal IDuplicatePropertyNameChecker CreateDuplicatePropertyNameChecker()
/// <returns>The instance retrieved from the object pool.</returns>
internal IDuplicatePropertyNameChecker GetDuplicatePropertyNameChecker()
{
return MessageWriterSettings.Validator.CreateDuplicatePropertyNameChecker();
return MessageWriterSettings.Validator.GetDuplicatePropertyNameChecker();
}

/// <summary>
/// Returns an instance of a duplicate property names checker to the object pool.
/// </summary>
internal void ReturnDuplicatePropertyNameChecker(IDuplicatePropertyNameChecker duplicatePropertyNameChecker)
{
MessageWriterSettings.Validator.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);
}
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.OData.Core/ODataWriterCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4133,7 +4133,7 @@ internal ResourceBaseScope(WriterState state, ODataResourceBase resource, ODataR

if (resource != null)
{
duplicatePropertyNameChecker = writerSettings.Validator.CreateDuplicatePropertyNameChecker();
duplicatePropertyNameChecker = writerSettings.Validator.GetDuplicatePropertyNameChecker();
}

this.serializationInfo = serializationInfo;
Expand Down
39 changes: 26 additions & 13 deletions src/Microsoft.OData.Core/Uri/ODataUriConversionUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,11 @@ internal static string ConvertToResourceLiteral(ODataResourceValue resourceValue
model,
messageWriterSettings,
textWriter,
(serializer) => serializer.WriteResourceValue(
resourceValue, /* resourceValue */
null, /* metadataTypeReference */
true, /* isOpenPropertyType */
serializer.CreateDuplicatePropertyNameChecker()));
(serializer, duplicatePropertyNamesChecker) => serializer.WriteResourceValue(
resourceValue,
metadataTypeReference : null,
isOpenPropertyType : true,
duplicatePropertyNamesChecker: duplicatePropertyNamesChecker));
}

return builder.ToString();
Expand Down Expand Up @@ -313,13 +313,14 @@ internal static string ConvertToUriCollectionLiteral(ODataCollectionValue collec
model,
messageWriterSettings,
textWriter,
(serializer) => serializer.WriteCollectionValue(
(serializer, duplicatePropertyNameChecker) => serializer.WriteCollectionValue(
collectionValue,
null /*metadataTypeReference*/,
null /*valueTypeReference*/,
false /*isTopLevelProperty*/,
true /*isInUri*/,
false /*isOpenPropertyType*/));
metadataTypeReference : null,
valueTypeReference : null,
isTopLevelProperty: false,
isInUri: true,
isOpenPropertyType: false),
isResourceValue: false);
}

return builder.ToString();
Expand Down Expand Up @@ -554,7 +555,8 @@ private static void WriteStartResource(ODataWriter writer, ODataResourceBase res
/// <param name="messageWriterSettings">Settings to use when writing.</param>
/// <param name="textWriter">TextWriter to use as the output for the value.</param>
/// <param name="writeValue">Delegate to use to actually write the value.</param>
private static void WriteJsonLightLiteral(IEdmModel model, ODataMessageWriterSettings messageWriterSettings, TextWriter textWriter, Action<ODataJsonLightValueSerializer> writeValue)
/// <param name="isResourceValue">We want to pass the <see cref="IDuplicatePropertyNameChecker"/> instance to the Action delegate when writing Resource value but not Collection value.</param>
private static void WriteJsonLightLiteral(IEdmModel model, ODataMessageWriterSettings messageWriterSettings, TextWriter textWriter, Action<ODataJsonLightValueSerializer, IDuplicatePropertyNameChecker> writeValue, bool isResourceValue = true)
{
IEnumerable<KeyValuePair<string, string>> parameters = new Dictionary<string, string>
{
Expand All @@ -577,7 +579,18 @@ private static void WriteJsonLightLiteral(IEdmModel model, ODataMessageWriterSet
new ODataJsonLightOutputContext(textWriter, messageInfo, messageWriterSettings))
{
ODataJsonLightValueSerializer jsonLightValueSerializer = new ODataJsonLightValueSerializer(jsonOutputContext);
writeValue(jsonLightValueSerializer);

if (!isResourceValue)
{
writeValue(jsonLightValueSerializer, null);
}
else
{
IDuplicatePropertyNameChecker duplicatePropertyNameChecker = jsonLightValueSerializer.GetDuplicatePropertyNameChecker();
writeValue(jsonLightValueSerializer, duplicatePropertyNameChecker);
jsonLightValueSerializer.ReturnDuplicatePropertyNameChecker(duplicatePropertyNameChecker);
}

jsonLightValueSerializer.AssertRecursionDepthIsZero();
}
}
Expand Down
Loading