-
Notifications
You must be signed in to change notification settings - Fork 491
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
Query and change feed: Add serialization optimization by using array instead of by item #1516
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the required format: "[Internal] Category: (Add|Fix|Refactor) Description"
Examples:
Diagnostics: Add GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fix null reference when using default(PartitionKey)
[v4] Client Encryption: Refactor code to external project
[Internal] Query: Add code generator for CosmosNumbers for easy additions in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the required format: "[Internal] Category: (Add|Fix|Refactor) Description"
Examples:
Diagnostics: Add GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fix null reference when using default(PartitionKey)
[v4] Client Encryption: Refactor code to external project
[Internal] Query: Add code generator for CosmosNumbers for easy additions in the future.
This reverts commit 01e39d4.
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Performance.Tests/Program.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
90% memory improvement and even greater CPU improvement
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
...re.Cosmos/tests/Microsoft.Azure.Cosmos.Performance.Tests/CosmosElements/ReadFeedBenchmark.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for the few comments above
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/src/Serializer/CosmosFeedResponseSerializer.cs
Outdated
Show resolved
Hide resolved
…nstead of by item (#1516) * ReadFeed and ChangeFeed optimization. # Conflicts: # Microsoft.Azure.Cosmos/src/Serializer/CosmosElementSerializer.cs
Pull Request Template
Description
This optimizes the query, read feed, and change feed serialization logic. This now sends an array to the serializer instead of serializing each item individually.
Read feed and change feed instead of using cosmos elements which has a DOM this now just searches the binary array for the start and end of the outer documents array.
This being done under the assumption that any new array being added to the service envelope will be a breaking change and will require an API version bump to correctly handle it.
Type of change
Please delete options that are not relevant.
This only includes the cost of unwrapping the service envelope and getting the array of documents that serializer can easily understand.