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

Performance refactor - elide async and await #2481

Merged
21 changes: 15 additions & 6 deletions src/Microsoft.OData.Core/Batch/ODataBatchOperationMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,22 @@ public override Task<Stream> GetStreamAsync()
{
this.VerifyNotCompleted();

// notify the listener that the stream has been requested
Task listenerTask = this.operationListener.StreamRequestedAsync();
return GetStreamInnerAsync();

// now remember that we are done processing the part header data (and only the payload is missing)
Stream contentStream = this.contentStreamCreatorFunc();
this.PartHeaderProcessingCompleted();
return listenerTask.FollowOnSuccessWith(task => { return (Stream)contentStream; });
async Task<Stream> GetStreamInnerAsync()
gathogojr marked this conversation as resolved.
Show resolved Hide resolved
{
// notify the listener that the stream has been requested
Task listenerTask = this.operationListener.StreamRequestedAsync();

// now remember that we are done processing the part header data (and only the payload is missing)
Stream contentStream = this.contentStreamCreatorFunc();
this.PartHeaderProcessingCompleted();

await listenerTask
.ConfigureAwait(false);

return contentStream;
}
}

/// <summary>
Expand Down
66 changes: 50 additions & 16 deletions src/Microsoft.OData.Core/Batch/ODataBatchWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,10 @@ public void WriteStartBatch()

/// <summary>Asynchronously starts a new batch; can be only called once and as first call.</summary>
/// <returns>A task instance that represents the asynchronous write operation.</returns>
public async Task WriteStartBatchAsync()
public Task WriteStartBatchAsync()
{
this.VerifyCanWriteStartBatch(false);
await this.WriteStartBatchImplementationAsync()
.ConfigureAwait(false);
return this.WriteStartBatchImplementationAsync();
}

/// <summary>Ends a batch; can only be called after WriteStartBatch has been called and if no other active changeset or operation exist.</summary>
Expand Down Expand Up @@ -340,12 +339,16 @@ public Task<ODataBatchOperationRequestMessage> CreateOperationRequestMessageAsyn
/// The format of operation Request-URI, which could be AbsoluteUri, AbsoluteResourcePathAndHost, or RelativeResourcePath.</param>
/// <param name="dependsOnIds">The prerequisite request ids of this request.</param>
/// <returns>A task that when completed returns the newly created operation request message.</returns>
public async Task<ODataBatchOperationRequestMessage> CreateOperationRequestMessageAsync(string method, Uri uri, string contentId,
BatchPayloadUriOption payloadUriOption, IList<string> dependsOnIds)
public Task<ODataBatchOperationRequestMessage> CreateOperationRequestMessageAsync(
string method,
Uri uri,
string contentId,
BatchPayloadUriOption payloadUriOption,
IList<string> dependsOnIds)
{
this.VerifyCanCreateOperationRequestMessage(false, method, uri, contentId);
return await this.CreateOperationRequestMessageInternalAsync(
method, uri, contentId, payloadUriOption, dependsOnIds).ConfigureAwait(false);
return this.CreateOperationRequestMessageInternalAsync(
method, uri, contentId, payloadUriOption, dependsOnIds);
}

/// <summary>Creates a message for writing an operation of a batch response.</summary>
Expand All @@ -360,11 +363,10 @@ public ODataBatchOperationResponseMessage CreateOperationResponseMessage(string
/// <summary>Asynchronously creates an <see cref="Microsoft.OData.ODataBatchOperationResponseMessage" /> for writing an operation of a batch response.</summary>
/// <param name="contentId">The Content-ID value to write in ChangeSet head.</param>
/// <returns>A task that when completed returns the newly created operation response message.</returns>
public async Task<ODataBatchOperationResponseMessage> CreateOperationResponseMessageAsync(string contentId)
public Task<ODataBatchOperationResponseMessage> CreateOperationResponseMessageAsync(string contentId)
{
this.VerifyCanCreateOperationResponseMessage(false);
return await this.CreateOperationResponseMessageImplementationAsync(contentId)
.ConfigureAwait(false);
return this.CreateOperationResponseMessageImplementationAsync(contentId);
}

/// <summary>Flushes the write buffer to the underlying stream.</summary>
Expand Down Expand Up @@ -717,6 +719,38 @@ private void InterceptException<TArg0>(Action<ODataBatchWriter, TArg0> action, T
}
}

/// <summary>
/// Catch any exception thrown by the action passed in; in the exception case move the writer into
/// state Error and then rethrow the exception.
/// </summary>
/// <typeparam name="TResult">The type of the result returned by the operation.</typeparam>
/// <typeparam name="TArg">The action argument type.</typeparam>
/// <param name="operation">The action to execute.</param>
/// <param name="arg">The argument value provided to the action.</param>
/// <returns>The result if the operation succeeds; otherwise the writer state is set to
/// <see cref="BatchWriterState.Error"/> and the exception rethrown.
/// </returns>
/// <remarks>
/// Make sure to only use anonymous functions that don't capture state from the enclosing context,
/// so the compiler optimizes the code to avoid delegate and closure allocations on every call to this method.
/// </remarks>
private TResult InterceptException<TResult, TArg>(Func<ODataBatchWriter, TArg, TResult> operation, TArg arg)
{
try
{
return operation(this, arg);
}
catch
{
if (!IsErrorState(this.state))
{
this.SetState(BatchWriterState.Error);
}

throw;
}
}

/// <summary>
/// Catch any exception thrown by the action passed in; in the exception case move the reader into
/// state Error and then rethrow the exception.
Expand Down Expand Up @@ -787,9 +821,9 @@ private ODataBatchOperationRequestMessage CreateOperationRequestMessageInternal(
this.payloadUriConverter.AddContentId(this.currentOperationContentId);
}

// This action delegate is capturing state from the enclosing context, to re-assign the 'uri', hence allocations are incurred here.
this.InterceptException((thisParam) =>
uri = ODataBatchUtils.CreateOperationRequestUri(uri, thisParam.outputContext.MessageWriterSettings.BaseUri, thisParam.payloadUriConverter));
uri = this.InterceptException((thisParam, uriParam) =>
gathogojr marked this conversation as resolved.
Show resolved Hide resolved
ODataBatchUtils.CreateOperationRequestUri(uriParam, thisParam.outputContext.MessageWriterSettings.BaseUri, thisParam.payloadUriConverter),
uri);

this.CurrentOperationRequestMessage = this.CreateOperationRequestMessageImplementation(
method, uri, contentId, payloadUriOption, dependsOnIds);
Expand Down Expand Up @@ -846,9 +880,9 @@ private async Task<ODataBatchOperationRequestMessage> CreateOperationRequestMess
this.payloadUriConverter.AddContentId(this.currentOperationContentId);
}

// This action delegate is capturing state from the enclosing context, to re-assign the 'uri', hence allocations are incurred here.
this.InterceptException((thisParam) =>
uri = ODataBatchUtils.CreateOperationRequestUri(uri, thisParam.outputContext.MessageWriterSettings.BaseUri, thisParam.payloadUriConverter));
uri = this.InterceptException((thisParam, uriParam) =>
ODataBatchUtils.CreateOperationRequestUri(uriParam, thisParam.outputContext.MessageWriterSettings.BaseUri, thisParam.payloadUriConverter),
uri);

this.CurrentOperationRequestMessage = await this.CreateOperationRequestMessageImplementationAsync(
method, uri, contentId, payloadUriOption, dependsOnIds).ConfigureAwait(false);
Expand Down
55 changes: 40 additions & 15 deletions src/Microsoft.OData.Core/Json/JsonLightInstanceAnnotationWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ internal void WriteInstanceAnnotation(
/// <param name="ignoreFilter">Whether to ignore the filter in settings.</param>
/// <param name="propertyName">The name of the property this instance annotation applies to</param>
/// <returns>A task that represents the asynchronous write operation.</returns>
internal async Task WriteInstanceAnnotationsAsync(
internal Task WriteInstanceAnnotationsAsync(
ICollection<ODataInstanceAnnotation> instanceAnnotations,
InstanceAnnotationWriteTracker tracker,
bool ignoreFilter = false,
Expand All @@ -279,7 +279,7 @@ internal async Task WriteInstanceAnnotationsAsync(

if (instanceAnnotations.Count == 0)
{
return;
return TaskUtils.CompletedTask;
}

HashSet<string> instanceAnnotationNames = new HashSet<string>(StringComparer.Ordinal);
Expand All @@ -289,19 +289,46 @@ internal async Task WriteInstanceAnnotationsAsync(
// but foreach against an IEnumerable does due to boxing
if (instanceAnnotations is List<ODataInstanceAnnotation> instanceAnnotationsList)
{
foreach (ODataInstanceAnnotation annotation in instanceAnnotationsList)
return WriteInstanceAnnotationsListAsync(instanceAnnotationsList, tracker, instanceAnnotationNames, ignoreFilter, propertyName);

async Task WriteInstanceAnnotationsListAsync(
List<ODataInstanceAnnotation> innerInstanceAnnotationsList,
InstanceAnnotationWriteTracker innerInstanceAnnotationWriteTracker,
HashSet<string> innerInstanceAnnotationNames,
bool innerIgnoreFilter,
string innerPropertyName)
{
await this.WriteAndTrackInstanceAnnotationAsync(annotation, tracker, instanceAnnotationNames, ignoreFilter, propertyName)
.ConfigureAwait(false);
foreach (ODataInstanceAnnotation annotation in innerInstanceAnnotationsList)
{
await this.WriteAndTrackInstanceAnnotationAsync(
annotation,
innerInstanceAnnotationWriteTracker,
innerInstanceAnnotationNames,
innerIgnoreFilter,
innerPropertyName).ConfigureAwait(false);
}
}

}
else
{
foreach (ODataInstanceAnnotation annotation in instanceAnnotations)
return WriteInstanceAnnotationsInnerAsync(instanceAnnotations, tracker, instanceAnnotationNames, ignoreFilter, propertyName);

async Task WriteInstanceAnnotationsInnerAsync(
ICollection<ODataInstanceAnnotation> innerInstanceAnnotations,
InstanceAnnotationWriteTracker innerInstanceAnnotationWriteTracker,
HashSet<string> innerInstanceAnnotationNames,
bool innerIgnoreFilter,
string innerPropertyName)
{
await this.WriteAndTrackInstanceAnnotationAsync(annotation, tracker, instanceAnnotationNames, ignoreFilter, propertyName)
.ConfigureAwait(false);
foreach (ODataInstanceAnnotation annotation in innerInstanceAnnotations)
{
await this.WriteAndTrackInstanceAnnotationAsync(
annotation,
innerInstanceAnnotationWriteTracker,
innerInstanceAnnotationNames,
innerIgnoreFilter,
innerPropertyName).ConfigureAwait(false);
}
}
}
}
Expand All @@ -313,7 +340,7 @@ await this.WriteAndTrackInstanceAnnotationAsync(annotation, tracker, instanceAnn
/// <param name="instanceAnnotations">Collection of instance annotations to write.</param>
/// <param name="propertyName">The name of the property this instance annotation applies to</param>
/// <param name="isUndeclaredProperty">If writing an undeclared property.</param>
internal async Task WriteInstanceAnnotationsAsync(
internal Task WriteInstanceAnnotationsAsync(
ICollection<ODataInstanceAnnotation> instanceAnnotations,
string propertyName = null,
bool isUndeclaredProperty = false)
Expand All @@ -322,13 +349,11 @@ internal async Task WriteInstanceAnnotationsAsync(
if (isUndeclaredProperty)
{
// write undeclared property's all annotations
await WriteInstanceAnnotationsForUndeclaredPropertyAsync(instanceAnnotations, propertyName)
.ConfigureAwait(false);
return WriteInstanceAnnotationsForUndeclaredPropertyAsync(instanceAnnotations, propertyName);
}
else
{
await this.WriteInstanceAnnotationsAsync(instanceAnnotations, new InstanceAnnotationWriteTracker(), false, propertyName)
.ConfigureAwait(false);
return this.WriteInstanceAnnotationsAsync(instanceAnnotations, new InstanceAnnotationWriteTracker(), false, propertyName);
}
}

Expand Down Expand Up @@ -542,7 +567,7 @@ private async Task WriteAndTrackInstanceAnnotationAsync(
}

if (!tracker.IsAnnotationWritten(annotation.Name)
&& (!ODataAnnotationNames.IsODataAnnotationName(annotation.Name) || ODataAnnotationNames.IsUnknownODataAnnotationName(annotation.Name)))
&& (!ODataAnnotationNames.IsODataAnnotationName(annotation.Name) || ODataAnnotationNames.IsUnknownODataAnnotationName(annotation.Name)))
{
await this.WriteInstanceAnnotationAsync(annotation, ignoreFilter, propertyName)
.ConfigureAwait(false);
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.OData.Core/Json/JsonReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2139,7 +2139,7 @@ private async Task<int> ReadCharsAsync(char[] chars, int offset, int maxLength)
/// The value of the TResult parameter contains true if a non-whitespace character was found,
/// in which case the <see cref="tokenStartIndex"/> is pointing at that character;
/// otherwise false if there are no non-whitespace characters left in the input.</returns>
#if NETSTANDARD2_0
#if NETSTANDARD2_0 || NETCOREAPP3_1_OR_GREATER
gathogojr marked this conversation as resolved.
Show resolved Hide resolved
private async ValueTask<bool> SkipWhitespacesAsync()
#else
private async Task<bool> SkipWhitespacesAsync()
Expand Down Expand Up @@ -2167,7 +2167,7 @@ private async Task<bool> SkipWhitespacesAsync()
/// <returns>A task that represents the asynchronous operation.
/// The value of the TResult parameter contains true if at least the required number of characters is available;
/// otherwise false if end of input was reached.</returns>
#if NETSTANDARD2_0
#if NETSTANDARD2_0 || NETCOREAPP3_1_OR_GREATER
private async ValueTask<bool> EnsureAvailableCharactersAsync(int characterCountAfterTokenStart)
#else
private async Task<bool> EnsureAvailableCharactersAsync(int characterCountAfterTokenStart)
Expand All @@ -2192,7 +2192,7 @@ private async Task<bool> EnsureAvailableCharactersAsync(int characterCountAfterT
/// otherwise false if end of input was reached.</returns>
/// <remarks>This may move characters in the <see cref="characterBuffer"/>, so after this is called
/// all indices to the <see cref="characterBuffer"/> are invalid except for <see cref="tokenStartIndex"/>.</remarks>
#if NETSTANDARD2_0
#if NETSTANDARD2_0 || NETCOREAPP3_1_OR_GREATER
private async ValueTask<bool> ReadInputAsync()
#else
private async Task<bool> ReadInputAsync()
Expand Down
5 changes: 3 additions & 2 deletions src/Microsoft.OData.Core/Json/JsonReaderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -808,9 +808,10 @@ internal static async Task<JsonNodeType> ReadNextAsync(this IJsonReaderAsync jso
#if DEBUG
bool result = await jsonReader.ReadAsync()
.ConfigureAwait(false);
Debug.Assert(result, "JsonReader.Read returned false in an unexpected place.");
Debug.Assert(result, "JsonReader.ReadAsync returned false in an unexpected place.");
#else
await jsonReader.ReadAsync();
await jsonReader.ReadAsync()
.ConfigureAwait(false);
gathogojr marked this conversation as resolved.
Show resolved Hide resolved
#endif
return jsonReader.NodeType;
}
Expand Down
Loading