Skip to content

Commit

Permalink
[Hotfix] Binary Encoding: Fixes Performance Regression on CI Pipeline (
Browse files Browse the repository at this point in the history
…#4953)

# Pull Request Template

## Description

Recently during our v3 sdk CI rolling runs, we observed some performance
regressions on the `ItemStreamAsync()` APIs. They regressed beyond 5%.



![image](https://github.com/user-attachments/assets/66cc4f01-2ec6-47e0-b885-5ad74e02bb63)

Upon doing further investigation, we figured out that during the
non-binary flow, we end up converting the incoming stream into
`CloneableStream` which might be the reason for this regression. Please
note that the reason this was not caught during the [original version of
the binary encoding
PR](#4652) was that
the performance test used to capture the benchmark for the original PR,
was targeted a real cosmos container, where for the CI runs, we use our
mocked containers.

This PR skips `CloneableStream` conversation for non-binary encoding
flow.

With the above change in place, our CI builds started passing:



![image](https://github.com/user-attachments/assets/8293a6e5-6fbc-4953-9de0-37162a081194)

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)

## Closing issues

To automatically close an issue: closes #IssueNumber

# Pull Request Template

## Description

Please include a summary of the change and which issue is fixed. Include
samples if adding new API, and include relevant motivation and context.
List any dependencies that are required for this change.

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)

## Closing issues

To automatically close an issue: closes #IssueNumber
  • Loading branch information
kundadebdatta authored Jan 8, 2025
1 parent 38ccda1 commit e9fa019
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
4 changes: 3 additions & 1 deletion Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ public override async Task<ResponseMessage> SendAsync(
&& response.Content != null
&& response.Content is not CloneableStream)
{
response.Content = await StreamExtension.AsClonableStreamAsync(response.Content, default);
response.Content = await StreamExtension.AsClonableStreamAsync(
mediaStream: response.Content,
allowUnsafeDataAccess: true);
}

return response;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -924,9 +924,14 @@ private async Task<ResponseMessage> ProcessItemStreamAsync(
string resourceUri = this.GetResourceUri(requestOptions, operationType, itemId);

// Convert Text to Binary Stream.
streamPayload = CosmosSerializationUtil.TrySerializeStreamToTargetFormat(
targetSerializationFormat: ContainerCore.GetTargetRequestSerializationFormat(),
inputStream: streamPayload == null ? null : await StreamExtension.AsClonableStreamAsync(streamPayload));
if (ConfigurationManager.IsBinaryEncodingEnabled())
{
streamPayload = CosmosSerializationUtil.TrySerializeStreamToTargetFormat(
targetSerializationFormat: ContainerCore.GetTargetRequestSerializationFormat(),
inputStream: streamPayload == null ? null : await StreamExtension.AsClonableStreamAsync(
mediaStream: streamPayload,
allowUnsafeDataAccess: true));
}

ResponseMessage responseMessage = await this.ClientContext.ProcessResourceOperationStreamAsync(
resourceUri: resourceUri,
Expand Down

0 comments on commit e9fa019

Please sign in to comment.