From ccbbb31e4b3a82270d325ae70bb1c40deebec204 Mon Sep 17 00:00:00 2001
From: Debdatta Kunda <87335885+kundadebdatta@users.noreply.github.com>
Date: Mon, 25 Nov 2024 10:57:12 -0800
Subject: [PATCH] [Internal] Binary Encoding: Fixes Performance Regression
 (#4901)

# 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](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/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
---
 .../src/Handler/RequestInvokerHandler.cs              |  4 +++-
 .../src/Resource/Container/ContainerCore.Items.cs     | 11 ++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs b/Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs
index 3cb3e6c748..a1d4846fb8 100644
--- a/Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs
+++ b/Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs
@@ -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;
diff --git a/Microsoft.Azure.Cosmos/src/Resource/Container/ContainerCore.Items.cs b/Microsoft.Azure.Cosmos/src/Resource/Container/ContainerCore.Items.cs
index ca7565cd2f..6e23513ff6 100644
--- a/Microsoft.Azure.Cosmos/src/Resource/Container/ContainerCore.Items.cs
+++ b/Microsoft.Azure.Cosmos/src/Resource/Container/ContainerCore.Items.cs
@@ -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,