Performance and Safety Improvements

[paulomorgado]

Performance and Safety Optimizations

Overview

This PR introduces a comprehensive set of memory, allocation, and safety optimizations across the codebase. The changes are designed to reduce unnecessary allocations and data copying, improve performance through the use of spans, and enforce immutability and safer exception handling patterns.

---

Memory and Allocation Improvements

Why These Changes?

• Zero-Allocation Slicing:
  Span<T> and ReadOnlySpan<T> enable lightweight, stack-only views over arrays and buffers, eliminating the need for new array allocations.
• No Data Copying:
  Spans provide direct access to subranges of data, avoiding the overhead and latency of copying.
• Reduced GC Pressure:
  Fewer allocations mean less work for the garbage collector, resulting in lower latency and higher throughput.
• Better JIT Optimization:
  Span operations are simple and predictable, allowing the JIT to optimize them aggressively.
• Improved Readability and Intent:
  Using spans clarifies that you are working with a view, not a copy, preventing accidental modifications.

What Was Changed?

• Minimized Temporary Arrays:
  Refactored code to avoid creating temporary arrays, especially in cryptographic and handshake routines.
• Replaced LINQ Take, Skip, and ToArray with Span Slicing:
  All usages of these LINQ methods for subarray extraction have been replaced with Span<T> or ReadOnlySpan<T> slicing, eliminating unnecessary heap allocations and data copying.
• Direct Span/Memory Usage:
  Where compatible, replaced array-based APIs and LINQ subsetting with Span<T>/ReadOnlySpan<T> slicing, enabling stack-based, allocation-free operations.

---

Immutability and API Safety

• Read-Only Parameters and Properties:
  Parameters and properties are now marked as readonly or exposed as ReadOnlySpan<T>/IReadOnlyList<T> where possible, preventing accidental modification of sensitive data.
• Safer API Contracts:
  Public properties that expose internal state are now read-only, ensuring consumers cannot mutate internal buffers or collections.

---

Exception Handling and Inlining

• Throwing in Non-Inlineable Methods:
  Exception throwing is centralized into dedicated helper methods (e.g., Throw.NotSupportedException(), Throw.TlsFatalAlert()), which are explicitly marked as non-inlineable.
  • Inlining Opportunity: Isolating exception logic allows the JIT to inline the calling method, improving hot-path performance.
  • Consistency: All exception throwing is now uniform and easier to audit or update.
• Fail-Fast on Invalid State:
  Defensive checks now throw immediately using these helpers, reducing the risk of undefined behavior.

---

Compatibility

• All changes are compatible with both .NET Standard 2.0 and .NET 8 targets.
• No breaking changes to public APIs, except for increased immutability (which is a safe, forward-compatible improvement).

---

Summary

This PR makes the library more robust, performant, and secure. By reducing allocations, enforcing immutability, using span slicing instead of LINQ, and optimizing exception handling, the codebase is better suited for high-performance and security-sensitive applications.

Please review the changes with a focus on:

• Correctness of the new read-only contracts
• Any edge cases in array handling or exception logic
• Potential for further allocation reduction

---

[paulomorgado]

@jimm98y,

I'd like to create tests for the untested parts. Any suggestions?

I also thing some properties, like keys, should be locked for access from external code and KeyParameter should be cached (because each time there is memory allocation and copy). What do you think?

[paulomorgado]

@jimm98y,

Why are SrtpContext.DeriveSessionKeys and SrtpContext.CalculateRequiredSrtpPayloadLength virtual. Is there a use case where the implementation would change?

[jimm98y]

@jimm98y,

Why are SrtpContext.DeriveSessionKeys and SrtpContext.CalculateRequiredSrtpPayloadLength virtual. Is there a use case where the implementation would change?

The idea was in case there is a newer crypto algorithm added in the future, people could simply add it by extending the class.

[jimm98y]

@jimm98y,

I'd like to create tests for the untested parts. Any suggestions?

I also thing some properties, like keys, should be locked for access from external code and KeyParameter should be cached (because each time there is memory allocation and copy). What do you think?

Caching KeyParameter is a good idea, that would reduce the number of allocations.

I don't see much benefit in "locking access" from external code, in the end you can always use reflection or dump it from the memory if you try really hard. See here https://github.com/jimm98y/AnsysDiscoveryD3DEnabler for instance. My most recent experience with "locked access" resulted in a few bad workarounds to accomplish my goal - see https://github.com/jimm98y/SharpOnvif/blob/32699624b9cff9801cc1b3eefad1741771859bdc/src/SharpOnvifClient/Security/HttpDigestProxy.cs#L115 and https://github.com/jimm98y/SharpOnvif/blob/32699624b9cff9801cc1b3eefad1741771859bdc/src/SharpOnvifClient/Security/HttpDigestHeaderInspector.cs#L141. I'd prefer to avoid putting developers in the same situations with my APIs...