Skip to content

api: cap Get result capacity to lower retained heap#834

Open
snissn wants to merge 5 commits intopr/valuelog-decode-scratch-holder-poolfrom
pr/get-cap-trim-retention
Open

api: cap Get result capacity to lower retained heap#834
snissn wants to merge 5 commits intopr/valuelog-decode-scratch-holder-poolfrom
pr/get-cap-trim-retention

Conversation

@snissn
Copy link
Copy Markdown
Owner

@snissn snissn commented Mar 15, 2026

Summary

  • cap Tree.Get return slice capacity to len(out) so API callers don't retain append-growth headroom
  • apply the same cap behavior in Snapshot.Get for consistency
  • strengthen TestGet_SafeCopy to assert cap(result)==len(result) for Get

Why

Get is an ownership boundary. Returning extra capacity can accidentally pin oversized backing arrays and inflate retained heap/RSS under heavy read/reuse patterns.

Validation

  • go test ./TreeDB ./TreeDB/caching ./TreeDB/tree -count=1
  • go vet ./TreeDB ./TreeDB/caching ./TreeDB/tree

Copilot AI review requested due to automatic review settings March 15, 2026 12:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens TreeDB’s Get ownership boundary by ensuring returned []byte slices are capacity-cipped to their length, preventing callers from accidentally retaining append-growth headroom and inflating retained heap.

Changes:

  • Cap tree.Tree.Get return slice capacity to len(out) via full-slice expression.
  • Apply the same capacity capping behavior to cached caching.Snapshot.Get.
  • Strengthen TestGet_SafeCopy to assert cap(result) == len(result) for Get.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
TreeDB/tree/tree.go Caps Tree.Get return capacity to prevent retaining excess backing array capacity.
TreeDB/caching/snapshot.go Caps Snapshot.Get return capacity for consistent safe-copy semantics in cached snapshots.
TreeDB/api_alloc_test.go Adds assertions that Get returns a slice with cap == len.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to prevent Get APIs from returning []byte values with extra capacity, so callers don’t accidentally retain or reuse append headroom across an ownership boundary.

Changes:

  • Cap the returned slice capacity in tree.Tree.Get to len(out) via a full-slice expression.
  • Apply the same capacity-capping behavior in caching.Snapshot.Get.
  • Strengthen TestGet_SafeCopy to assert cap(result) == len(result) for DB.Get.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
TreeDB/tree/tree.go Caps Tree.Get return slice capacity to length and documents rationale.
TreeDB/caching/snapshot.go Wraps Snapshot.Get to cap capacity after calling GetAppend.
TreeDB/api_alloc_test.go Adds assertions that Get returns a slice with cap==len.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot AI review requested due to automatic review settings March 18, 2026 00:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Caps Get return slice capacity to its length so callers don’t retain append-growth headroom and unintentionally pin large backing arrays, reducing retained heap/RSS.

Changes:

  • Cap Tree.Get’s returned []byte capacity to len(out) using full slice expressions.
  • Apply the same capacity-capping behavior to Snapshot.Get.
  • Strengthen TestGet_SafeCopy to assert cap(result) == len(result) for Get.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
TreeDB/tree/tree.go Caps Tree.Get return slice capacity to length to avoid retaining excess backing array capacity.
TreeDB/caching/snapshot.go Updates Snapshot.Get to cap returned slice capacity similarly (with explicit error handling).
TreeDB/api_alloc_test.go Adds assertions ensuring Get results are capacity-capped to length.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 606d9954b9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants