Skip to content

Reuse DatumVec allocation in metrics history truncation#36990

Open
antiguru wants to merge 1 commit into
mainfrom
claude/modest-knuth-hd5c3n
Open

Reuse DatumVec allocation in metrics history truncation#36990
antiguru wants to merge 1 commit into
mainfrom
claude/modest-knuth-hd5c3n

Conversation

@antiguru

Copy link
Copy Markdown
Member

Motivation

Optimize memory allocation in the partially_truncate_metrics_history function by reusing a single DatumVec allocation across multiple row unpacking operations instead of allocating a fresh vector for each row.

Description

This change introduces a performance optimization in the metrics history truncation logic. Previously, each call to row.unpack() would allocate a new vector to hold the unpacked datums. By reusing a single DatumVec allocation across the loop, we avoid repeated allocations and deallocations.

The change:

  1. Adds DatumVec to the imports from mz_repr
  2. Creates a single DatumVec instance before the loop
  3. Replaces row.unpack() with datum_vec.borrow_with(row) to reuse the allocation

This is a straightforward refactoring that maintains identical behavior while reducing memory pressure during metrics history truncation operations.

Verification

This is a performance optimization with no functional changes. The behavior remains identical — we're just reusing an allocation. Existing tests for metrics history truncation will continue to pass.

https://claude.ai/code/session_01Do1ud8hPYH5Pk3iM36AkEY

partially_truncate_metrics_history decoded each snapshot row with
Row::unpack, allocating a fresh Vec<Datum> per row in the hot
truncation loop. Use a re-used DatumVec and borrow_with instead, which
returns the allocation to the vector on drop and reuses it across rows.
@antiguru antiguru requested a review from DAlperin June 11, 2026 13:17
@antiguru antiguru marked this pull request as ready for review June 11, 2026 13:27
@antiguru antiguru requested a review from a team as a code owner June 11, 2026 13:27
antiguru added a commit that referenced this pull request Jun 11, 2026
…6991)

### Motivation

The `handle_row` closure in `partially_truncate_status_history` decoded
each snapshot row with `Row::unpack`, which allocates a fresh
`Vec<Datum>` on every call. This runs in the hot per-row truncation loop
over the status history shard.

This is a sibling change to the metrics-history truncation PR (#36990);
same pattern, different call site.

### Tips for reviewer

The closure now captures a re-used `DatumVec` and decodes rows via
`borrow_with`, which returns its allocation to the vector on drop and
reuses it across rows. `extract_key`/`extract_time` take `&[Datum]`, and
`&DatumVecBorrow` deref-coerces to `&[Datum]`, so the call sites are
unchanged. The closure was already `mut`/`FnMut` (it mutates
`latest_row_per_key`), so capturing the `DatumVec` by value and
borrowing it mutably is fine.

### Checklist

- [ ] This PR has adequate test coverage / QA involvement has been duly
considered. ([trigger-ci for additional test/nightly
runs](https://trigger-ci.dev.materialize.com/))
- [ ] This PR has an associated up-to-date [design
doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md),
is a design doc
([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)),
or is sufficiently small to not require a design.
- [ ] If this PR evolves [an existing `$T ⇔ Proto$T`
mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md)
(possibly in a backwards-incompatible way), then it is tagged with a
`T-proto` label.
- [ ] If this PR will require changes to cloud orchestration or tests,
there is a companion cloud PR to account for those changes that is
tagged with the release-blocker label
([example](MaterializeInc/cloud#5021)).
- [ ] If this PR includes major [user-facing behavior
changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note),
I have pinged the relevant PM to schedule a changelog entry.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01Do1ud8hPYH5Pk3iM36AkEY)_

Co-authored-by: Claude <noreply@anthropic.com>
@antiguru antiguru requested a review from martykulma June 12, 2026 14:55
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.

2 participants