Skip to content

Conversation

@imdominicreed
Copy link
Contributor

@imdominicreed imdominicreed commented Nov 13, 2025

Proposed Changes

  • Better locking to allow parallel encryption. Less memory allocations, causing reuse.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

pr benchmark:

endpoint: http://localhost:8080
# Benchmark Experimental TDF Writer Results:
| Metric             | Value         |
|--------------------|--------------|
| Payload Size (B)   | 1048576999 |
| Output Size (B)    | 6716081 |
| Total Time         | 222.619708ms |

main benchmark:

endpoint: http://localhost:8080
# Benchmark Experimental TDF Writer Results:
| Metric             | Value         |
|--------------------|--------------|
| Payload Size (B)   | 1048576999 |
| Output Size (B)    | 6721718 |
| Total Time         | 1.37738475s |

@imdominicreed imdominicreed requested review from a team as code owners November 13, 2025 21:04
@github-actions github-actions bot added comp:sdk A software development kit, including library, for client applications and inter-service communicati comp:lib:ocrypto size/m labels Nov 13, 2025
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @imdominicreed, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces significant performance and memory efficiency improvements to the experimental TDF Writer by optimizing the encryption process and streamlining how encrypted data segments are handled. The changes reduce memory copying, enable more flexible streaming of TDF segments, and enhance concurrency during segment writing operations.

Highlights

  • Efficient In-Place Encryption: Introduced a new EncryptInPlace function in aes_gcm.go that encrypts data directly into the provided buffer, returning the ciphertext and nonce. This avoids unnecessary memory allocations and copying during encryption.
  • Refactored TDF Segment Writing: The WriteSegment method in the experimental TDF Writer (writer.go) has been significantly updated to leverage the new in-place encryption. It now calculates the CRC32 checksum over the nonce and ciphertext, and returns an io.Reader for the full TDF segment (including nonce, encrypted data, and any ZIP structures), enabling more efficient streaming.
  • Updated SegmentResult Structure: The SegmentResult struct in writer.go now includes a TDFData io.Reader field to provide a streamable representation of the encrypted segment, and the CRC32 field has been removed as CRC calculation is now handled differently.
  • Optimized ZipStream Segment Handling: The WriteSegment method in sdk/internal/zipstream/segment_writer.go no longer accepts the raw data bytes. Instead, it takes the size and crc32 of the encrypted data. This change shifts the responsibility of data handling to the caller, allowing the zipstream package to focus solely on writing ZIP headers and metadata, further reducing memory overhead.
  • Mutex Unlock Optimization: The mutex unlock in WriteSegment in writer.go has been moved earlier to release the lock before performing potentially long-running encryption and archive writing operations, improving concurrency.
  • Segment Map Stores Pointers: The segments map in writer.go now stores pointers to Segment structs (map[int]*Segment) instead of values, allowing for in-place modification of segment metadata.
  • Test Updates: All relevant tests and benchmarks have been updated to reflect the new WriteSegment signatures and the revised data flow, ensuring correctness and performance gains.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Data streams flow, a silent, swift design, Encrypted fast, no copy, just inline. With nonce and cipher, secrets now entwine, Efficiency's dance, a code so divine.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively improves encryption efficiency in the TDF writer by introducing in-place encryption and optimizing the locking strategy. These changes reduce memory allocations and improve concurrency.

My review focuses on a critical data race introduced by the new locking mechanism and an area of potential confusion in the SegmentResult API. Addressing these points will help ensure the correctness and usability of these improvements.

@imdominicreed imdominicreed requested review from a team as code owners November 13, 2025 22:16
@imdominicreed imdominicreed added this pull request to the merge queue Nov 14, 2025
Merged via the queue into opentdf:main with commit 3ec0518 Nov 14, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:lib:ocrypto comp:sdk A software development kit, including library, for client applications and inter-service communicati size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants