Perf | Reduce allocations in Always Encrypted key handling, reorganise#4256
Open
edwardneal wants to merge 24 commits into
Open
Perf | Reduce allocations in Always Encrypted key handling, reorganise#4256edwardneal wants to merge 24 commits into
edwardneal wants to merge 24 commits into
Conversation
The underlying MemoryCache is thread-safe at the point of retrieval. Add a check outside of the lock to avoid contending it where possible.
The encryption key and the algorithm are tightly coupled; we don't need to specify this explicitly.
Salt strings are always constant.
Lazy-load these - these values will not change.
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4256 +/- ##
==========================================
- Coverage 66.02% 64.30% -1.72%
==========================================
Files 277 272 -5
Lines 42988 65786 +22798
==========================================
+ Hits 28382 42304 +13922
- Misses 14606 23482 +8876
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cheenamalhotra
added
the
Performance 📈
apoorvdeshmukh
approved these changes
May 12, 2026
Contributor
apoorvdeshmukh
left a comment
apoorvdeshmukh
left a comment
There was a problem hiding this comment.
Changes look good to me overall.
@edwardneal Would it be possible to add the benchmarking code in the repo? The one with which you measured the performance.
Maybe under src/Microsoft.Data.SqlClient/tests/PerformanceTests?
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Always Encrypted internals to reduce allocations and contention during CEK handling, including reorganizing internal types into the Microsoft.Data.SqlClient.AlwaysEncrypted namespace and adding targeted nullability annotations.
Changes:
- Replaces/relocates internal Always Encrypted primitives (
SymmetricKey,EncryptionType, CEK cache) and updates call sites + tests accordingly. - Optimizes CEK caching behavior (fast-path cache lookup; uses
TimeSpan.Zerofor provider cache TTL). - Reduces allocations in key derivation/HMAC paths (span-based HMAC on .NET; avoids extra wrapper allocations for derived keys).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/AlwaysEncrypted/NativeAeadBaseline.cs | Updates unit tests to new SymmetricKey/EncryptionType types. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/Setup/CertificateUtility.cs | Updates reflection hooks to renamed/moved CEK cache type and singleton accessor. |
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/AlwaysEncryptedTests/Utility.cs | Updates reflection references for internal key types/constructors. |
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/AlwaysEncryptedTests/ExceptionsAlgorithmErrors.cs | Updates reflection references for internal key types/constructors. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlUtil.cs | Switches InvalidEncryptionType helper to new EncryptionType enum. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs | Adds span-based HMAC path on .NET; updates AE key decryption flow to new key/cache types. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs | Updates encryption-type comparisons to new EncryptionType. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlAeadAes256CbcHmac256Algorithm.cs | Switches to new derived-key container and updated key properties. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/EnclaveDelegate.cs | Updates enclave-related encryption/decryption to new key/encryption type types. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/SymmetricKeyCache.cs | Introduces new CEK cache implementation in AlwaysEncrypted namespace (includes concurrency changes). |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/SymmetricKey.cs | Renames/moves symmetric key wrapper to AlwaysEncrypted namespace. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/EncryptionType.cs | Introduces new internal enum aligned to TDS encryption type values. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/EncryptionAlgorithmFactoryList.cs | Updates algorithm factory dispatch to new key/encryption type types. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/EncryptionAlgorithmFactory.cs | Updates factory interface to new key/encryption type types. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/AeadAes256CbcHmac256Factory.cs | Updates AEAD factory to new key/encryption type types and derived-key construction. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/AeadAes256CbcHmac256EncryptionKey.cs | Adds new derived-key container that caches salts/derived keys. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSymmetricKeyCache.cs | Removes legacy CEK cache type from root namespace (replaced by SymmetricKeyCache). |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEncryptionType.cs | Removes legacy enum (replaced by AlwaysEncrypted.EncryptionType). |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlAeadAes256CbcHmac256EncryptionKey.cs | Removes legacy derived-key wrapper (replaced by AlwaysEncrypted.AeadAes256CbcHmac256EncryptionKey). |
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AlwaysEncrypted/SymmetricKey.cs:35
- The XML doc says
RootKey"Returns a copy of the plain text key", but the property returns the underlying array reference. Either update the documentation to reflect the actual behavior or return a defensive copy to match the doc (be mindful of the perf/security tradeoff).
Comment on lines
+28
to
+30
| public static SymmetricKeyCache Instance => | ||
| field ??= new(); | ||
|
|
Comment on lines
+53
to
+58
| private static byte[] EncryptionKeySalt => | ||
| field ??= Encoding.Unicode.GetBytes(EncryptionKeySaltString); | ||
| private static byte[] MacKeySalt => | ||
| field ??= Encoding.Unicode.GetBytes(MacKeySaltString); | ||
| private static byte[] IvKeySalt => | ||
| field ??= Encoding.Unicode.GetBytes(IvKeySaltString); |
Comment on lines
41
to
45
| /// <param name="encryptionType">Encryption type. Expected values are either Deterministic or Randomized.</param> | ||
| /// <param name="encryptionAlgorithm">Cryptographic algorithm.</param> | ||
| /// <returns>An implementation of the AEAD_AES_256_CBC_HMAC_SHA256 cryptographic algorithm.</returns> | ||
| internal override SqlClientEncryptionAlgorithm Create(SqlClientSymmetricKey encryptionKey, SqlClientEncryptionType encryptionType, string encryptionAlgorithm) | ||
| internal override SqlClientEncryptionAlgorithm Create(SymmetricKey encryptionKey, EncryptionType encryptionType, string encryptionAlgorithm) | ||
| { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This reduces the number of memory allocations performed by Always Encrypted functionality, focusing primarily upon the encryption keys.
It also introduces nullability annotations to the touched files, makes some minor documentation improvements, removes the redundant
SqlClientorSqlprefixes ofinternaltypes and moves them into the AlwaysEncrypted namespace. These don't result in any public API changes or any changes to the calling contract forSqlColumnEncryptionKeyStoreProvider.The PR's diff appears larger than it truly is; it may be easier to review commit-by-commit.
In sequence:
ColumnEncryptionKeyCacheTtlwas being set to a newTimeSpaninstance rather thanTimeSpan.Zero.SqlColumnEncryptionKeyStoreProvider.DecryptColumnEncryptionKey. We retain the lock in order to preserve the guarantee that this method call will always be serialized.SqlAeadAes256CbcHmac256Algorithmwas being passed the constant algorithm name in the constructor. These two components are tightly coupled, so I removed the redundant parameter name. This had knock-on effects - it meant that thestring.Formatcould be replaced with a constant string, and that the Unicode encoding of this could be cached.SymmetricKeyallocations were used to store the IV, MAC and cryptographic keys. These acted as a lightweight wrapper, providing no defensive copies or immutability guarantees. We now just store the byte arrays directly.SqlSecurityUtility.GetHMACWithSHA256used the netfx cryptographic methods, then allocated and copied. On netcore, we can use the oneshots and eliminate all allocations (and in some circumstances, the copy.)The microbenchmark highlights the impact when instantiating a
AeadAes256CbcHmac256EncryptionKeyunder cold cache conditions: a measurable reduction in GC pressure and a modest throughput improvement.Security considerations
This makes changes which are very close to the core
SqlAeadAes256CbcHmac256Algorithmimplementation. I've deliberately scoped this PR to exclude any changes to this. While I think there are other performance improvements to be made in that area, they don't stand alone in the same way that these do and need their own discussion/review.The CEK derivation logic remains unchanged, as does the key validation logic and key usage ordering.
Issues
None.
Testing
Existing automated tests continue to pass. This doesn't introduce any new functionality or make any behavioural changes, so I've not added any new tests.