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.

Sign up for GitHub

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

Jump to bottom

[Client encryption]: Optimize encryption/decryption flows #4678

Open
juraj-blazek opened this issue Sep 12, 2024 · 0 comments
Open

[Client encryption]: Optimize encryption/decryption flows #4678

juraj-blazek opened this issue Sep 12, 2024 · 0 comments
Labels
customer-reported Issue created by a customer needs-investigation

Comments

@juraj-blazek
Copy link
Contributor

juraj-blazek commented Sep 12, 2024 •
edited
Loading

Is your feature request related to a problem? Please describe.
When using the Microsoft.Azure.Cosmos.Encryption.Custom encryption package, there is a big jump in CPU and memory consumption, leading to higher compute cost, higher GC pressure, and lower throughput. This is especially sensitive in the large-scale services.

See sample benchmark results of the EncryptionProcessor (PR adding the performance tests):

Method DocumentSizeInKb Mean Error StdDev Gen0 Gen1 Gen2 Allocated
Encrypt 1 60.05 μs 1.537 μs 2.300 μs 5.0659 1.2817 - 62.65 KB
Decrypt 1 70.76 μs 0.812 μs 1.164 μs 5.7373 1.4648 - 71.22 KB
Encrypt 10 165.23 μs 3.741 μs 5.365 μs 21.2402 3.6621 - 262.38 KB
Decrypt 10 231.32 μs 4.627 μs 6.635 μs 29.5410 3.4180 - 363.84 KB
Encrypt 100 2,572.40 μs 242.163 μs 362.458 μs 201.1719 126.9531 125.0000 2466.27 KB
Decrypt 100 2,952.48 μs 397.387 μs 557.081 μs 255.8594 210.9375 160.1563 3412.88 KB

Describe the solution you'd like
As a first step, we need to bring the allocations down as much as possible. We already started updating the core Microsoft.Data.Encryption.Cryptography dependency to add allocation-free APIs. We'll follow with contributions to Cosmos DB SDK to have them utilized.

Describe alternatives you've considered
Not using encryption. This is not a viable alternative.

Additional context
This is a placeholder issue where we'll link any optimizations PR to.
@juraj-blazek juraj-blazek mentioned this issue Sep 12, 2024
Merged
1 task
kirankumarkolli added a commit that referenced this issue Sep 27, 2024
@juraj-blazek @kr-santosh @kirankumarkolli
[Client encryption]: Add baseline benchmarks for `Microsoft.Azure.Cos…
a883414 
…mos.Encryption.Custom` (#4679)

# Pull Request Template

## Description

Adding a `Microsoft.Azure.Cosmos.Encryption.Custom.Performance.Tests`
with `Encrypt`/`Decrypt` benchmarks for different document size.
Attaching also initial baseline report with the current memory
allocations.

## Type of change

Please delete options that are not relevant.

- [x] New feature (non-breaking change which adds functionality)

## Closing issues

Contributes to #4678

---------

Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
@JanHyka JanHyka mentioned this issue Sep 27, 2024
Merged
kirankumarkolli added a commit that referenced this issue Sep 27, 2024
@JanHyka @kirankumarkolli
[Client encryption]: Switch Newtonsoft.Json to use ArrayPool (#4726)
a4242a3 
# Pull Request Template

## Description

Switches usages of `JsonTextReader` and `JsonTextWriter` to use
`ArrayPool<char>.Shared` for better performance and reduced memory
allocations. Arrays returned to pool are zeroed for increased data
security (at benchmarked cost of ~3.5% extra mean compared to non-zeroed
returns).

## Type of change

Please delete options that are not relevant.

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

## Closing issues

Contributes to #4678

Co-authored-by: Kiran Kumar Kolli <[email protected]>
This was referenced Oct 1, 2024
Merged
Merged
kirankumarkolli pushed a commit that referenced this issue Oct 1, 2024
@JanHyka
[Client encryption]: Fixes Faster validations and string comparisons (#…
e2d2ea3 
…4739)

# Pull Request Template

## Description

1. Switch frequent constants string comparison mode to `ordinal`
2. Reduce allocations and validations complexity

## Type of change

Please delete options that are not relevant.

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

## Closing issues

Contributes to #4678
@JanHyka JanHyka mentioned this issue Oct 4, 2024
Merged
kirankumarkolli added a commit that referenced this issue Oct 6, 2024
@JanHyka @juraj-blazek
@kr-santosh @kirankumarkolli
[Client encryption]: Drop allocations on MDE Encryption path (#4740)
50c401e 
# Pull Request Template

## Description

On Encryption path we are switching to MDE API with offsets. This allows
reducing of allocations.
To sustain compatibility, enhanced API is backward implemented to
obsoleted local copy of encryption algorithm. No performance
optimizations on obsoleted code paths.

## Type of change

Please delete options that are not relevant.

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

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
kirankumarkolli added a commit that referenced this issue Oct 6, 2024
@JanHyka @juraj-blazek
@kr-santosh @kirankumarkolli
[Client encryption]: Address code styling errors and warnings (#4747)
814e33b 
# Pull Request Template

## Description

Cleanup of code styling issues in Cosmos.Encryption.Custom.* projects as
set up by editorconfig.

## Type of change

Please delete options that are not relevant.

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

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
This was referenced Oct 6, 2024
Merged
Merged
kirankumarkolli pushed a commit that referenced this issue Oct 7, 2024
@JanHyka @juraj-blazek @kr-santosh
[Client encryption]: Switch to non-allocating MDE 2.0 api (#4753)
43c14a3 
# Pull Request Template

## Description

Switch both MDE encryption and encryption paths to use MDE2.0 api

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
This was referenced Oct 8, 2024
Merged
Merged
Merged
Merged
kirankumarkolli added a commit that referenced this issue Oct 9, 2024
@JanHyka @juraj-blazek
@kr-santosh @kirankumarkolli
[Client encryption]: Refactor EncryptionProcessor (#4757)
ac78e58 
# Pull Request Template

## Description

Refactor EncryptionProcessor - split code paths for deprecated local
implementation of Aes, split individual transformers to improve
maintainability and testability, split preview/stable

To be processed after #4753 

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
kirankumarkolli pushed a commit that referenced this issue Oct 9, 2024
@JanHyka @juraj-blazek @kr-santosh
[Client encryption]: Add .NET8.0 target (#4766)
01c6218 
# Pull Request Template

## Description

- This is preliminary step to enable Brotli compression on
Cosmos.Encryption.Custom.
- .NET8.0 target was added to the project
- New compiler complaints were addressed

To be processed after #4757 

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
@JanHyka JanHyka mentioned this issue Oct 10, 2024
Merged
kirankumarkolli pushed a commit that referenced this issue Oct 10, 2024
@JanHyka @juraj-blazek @kr-santosh
[Client encryption]: Add JsonNodeSqlSerializer (#4779)
4f515bb 
# Pull Request Template

## Description

Initial commit for JsonNode on Encryption path. This is currently not
executed on any production/preview path. Depends on JsonNode features
available from System.Text.Json 8.0+.

- Adds JsonNodeSqlSerializer
- JObjectSqlSerializer now doesn't format inner serialized
JArrays/JObjects (with line breaks/indentations)

To be processed after #4766 

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
kirankumarkolli added a commit that referenced this issue Oct 11, 2024
@JanHyka @juraj-blazek
@kr-santosh @kirankumarkolli
[Client encryption]: Adds support for Brotli compression (#4769)
8644014 
# Pull Request Template

## Description

- Add Brotli compression for .NET8.0 target
- To stabilize performance tests duration results, GC was reconfigured
to server mode (reduces StdDev 42% -> 5%)

Brotli compression is now available for .NET8.0 target. All encrypted
fields matching criteria are compressed before encryption.

The option is configurable via
`Microsoft.Azure.Cosmos.Encryption.Custom.EncryptionOptions` property
`CompressionOptions`

`CompressionOptions` structure:
- `Algorithm` : `CompressionOptions.CompressionAlgoritm` suports `None`
and `Brotli` (for .NET8.0+), defaults to None.
- `CompressionLevel` : `System.IO.Compression.CompressionLevel` -
defaults to CompressionLevel.Fastest
- `MinimalCompressedLength` : determines minimal size of encrypted
serialized Json text for compression. Defaults to 128 bytes.

It is suggested to benchmark your use cases, different settings can lead
to different ratio of compute vs storage costs. Default settings in
general should save both cpu time and memory consumption.

**Warn: make sure all your instances are upgraded to version supporting
compression BEFORE enabling it.**

To be processed after #4766 

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
kirankumarkolli pushed a commit that referenced this issue Oct 15, 2024
@JanHyka @juraj-blazek @kr-santosh
[Client encryption]: Add JsonNode Encryption processor (#4780)
138601f 
# Pull Request Template

## Description

- Added support for System.Text.JsonNode DOM on encryption path
- Configuration option to select Json Processor (defaults to Newtonsoft)
- Custom byte[] coverter supporting offset+length for JsonNode
- Perf tests expanded to cover both json processors (JsonNode decryption
still uses Newtonsoft, this will be upgraded with further PRs)

To be processed after #4779 

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)
- [] This change requires a documentation update

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
@JanHyka JanHyka mentioned this issue Oct 15, 2024
Merged
kirankumarkolli added a commit that referenced this issue Oct 16, 2024
@JanHyka @juraj-blazek
@kr-santosh @kirankumarkolli
[Client encryption]: Adds JsonNode support on deserialization path (#…
f88605c 
…4787)

# Pull Request Template

## Description

- Added support for System.Text.JsonNode DOM on deserializer and
decryption path
- Drops custom byte[] converter as JsonNode can take Memory<byte>
directly

To be processed after #4780 

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)
- [] This change requires a documentation update

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
kirankumarkolli added a commit that referenced this issue Oct 23, 2024
@JanHyka @juraj-blazek
@kr-santosh @kirankumarkolli
[Client encryption]: Adds Stream based processor (#4806)
f1e5c2f 
# Pull Request Template

## Description

- JsonProcessor.Stream was added to drop DOM overhead.
- EncryptionProcessor has added overloads allowing consumer provision of
output stream
- tests were extended to new processor
- performance tests were extended by provided stream overloads.
RecyclableMemoryStream 3.0.1 is being used.

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)
- [] This change might require a documentation update

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
This was referenced Oct 24, 2024
Merged
Open
@JanHyka JanHyka mentioned this issue Oct 24, 2024
Draft
kirankumarkolli added a commit that referenced this issue Oct 25, 2024
@JanHyka @juraj-blazek
@kr-santosh @kirankumarkolli
[Client encryption]: Drops JsonNode based processor (#4846)
4a70bc3 
# Pull Request Template

## Description

With new Stream based processor the JsonNode option is excessive and not
really needed. We are dropping all of its code.

## Type of change

Please delete options that are not relevant.

- [] New feature (non-breaking change which adds functionality)

## Closing issues

Contributes to #4678

---------

Co-authored-by: Juraj Blazek <[email protected]>
Co-authored-by: juraj-blazek <[email protected]>
Co-authored-by: Santosh Kulkarni <[email protected]>
Co-authored-by: Kiran Kumar Kolli <[email protected]>
@JanHyka JanHyka mentioned this issue Nov 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
customer-reported Issue created by a customer needs-investigation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@juraj-blazek