Skip to content

tp: have the executor end every step's run when its stream ends - #8332

Open
LalitMaganti wants to merge 1 commit into
mainfrom
dev/lalitm/step-state-lifetimes
Open

LalitMaganti wants to merge 1 commit into
mainfrom
dev/lalitm/step-state-lifetimes

Conversation

@LalitMaganti

@LalitMaganti LalitMaganti commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Why

A step let go of what it held only if it remembered to clear itself,
and almost none did. GraphTree, the interval operators and the tree
transforms kept everything they built until the next run. One
OperatorState mixed per-call scratch, rows held for the whole input
and the step's status, so nothing said what a step may keep or for how
long.

Changes

  • operator.h states the rules every step keeps to. Each field of a
    state declares its lifetime: Held<T> or RunValue<T> for the run,
    CallScratch<T> for one call, and anything else is set when the
    state is made.
  • Held<T> and RunValue<T> register with their state. The pipeline
    ends their run as soon as the step's stream ends, and again before a
    source is restarted. Reset() and ResetEachRun are gone.
  • States are made once and reused, because the same pipeline is often
    rerun for small queries. Ending a run keeps up to 64 KiB of each
    container's storage and releases anything larger.
  • The pipeline merges its states' registries by segment, so ending a
    run is one pass with no call per state.
  • A batch a step outputs never points into its state, so a run can end
    while its batches are still read. GroupedSort copies its group
    numbers into a buffer instead of lending them.
  • Each state's fields are grouped by what they do.

Results

INCLUDE of heap_graph_dominator_tree on
test/data/heap_graph_object_for_benchmarks.pftrace, release build:

Peak RSS Buffers
#8331 305 MB 57 MB
This PR 270 MB 53 MB

Every heap graph table is byte-identical. Rerunning a one-row pipeline
costs about 2-3 ns more per step that holds state. Large runs are
unchanged.

Testing

  • New StateTest.AStepsRunEndsWithItsStream.
  • The affected unit test suites pass.

@LalitMaganti
LalitMaganti added this pull request to stack #8328 October 28, 2026 06:10
@LalitMaganti
LalitMaganti force-pushed the dev/lalitm/step-state-lifetimes branch 3 times, most recently from aae62b5 to 4295b58 Compare October 28, 2026 07:13
@LalitMaganti
LalitMaganti force-pushed the dev/lalitm/step-state-lifetimes branch 2 times, most recently from 84c0e83 to 0487efd Compare October 29, 2026 18:17
@LalitMaganti
LalitMaganti marked this pull request as ready for review October 29, 2026 18:17
@LalitMaganti
LalitMaganti requested a review from a team as a code owner October 29, 2026 18:17
@LalitMaganti
LalitMaganti force-pushed the dev/lalitm/step-state-lifetimes branch from 0487efd to 4eec006 Compare October 29, 2026 19:13
@LalitMaganti
LalitMaganti removed this pull request from stack #8328 October 29, 2026 20:34
@LalitMaganti
LalitMaganti added this pull request to stack #8386 October 29, 2026 20:35
@LalitMaganti
LalitMaganti force-pushed the dev/lalitm/step-state-lifetimes branch from 4eec006 to 0ccac95 Compare October 29, 2026 20:56
@LalitMaganti
LalitMaganti force-pushed the dev/lalitm/step-state-lifetimes branch 2 times, most recently from 5eb9bc3 to baca647 Compare October 30, 2026 20:20
@LalitMaganti
LalitMaganti force-pushed the dev/lalitm/step-state-lifetimes branch from baca647 to 422e2cb Compare October 30, 2026 21:20
Base automatically changed from dev/lalitm/exec-run-allocs to main October 30, 2026 21:37
**Why**

A step let go of what it held only if it remembered to clear itself,
and almost none did. GraphTree, the interval operators and the tree
transforms kept everything they built until the next run. One
`OperatorState` mixed per-call scratch, rows held for the whole input
and the step's status, so nothing said what a step may keep or for how
long.

**Changes**

- `operator.h` states the rules every step keeps to. Each field of a
  state declares its lifetime: `Held<T>` or `RunValue<T>` for the run,
  `CallScratch<T>` for one call, and anything else is set when the
  state is made.
- `Held<T>` and `RunValue<T>` register with their state. The pipeline
  ends their run as soon as the step's stream ends, and again before a
  source is restarted. `Reset()` and `ResetEachRun` are gone.
- States are made once and reused, because the same pipeline is often
  rerun for small queries. Ending a run keeps up to 64 KiB of each
  container's storage and releases anything larger.
- The pipeline merges its states' registries by segment, so ending a
  run is one pass with no call per state.
- A batch a step outputs never points into its state, so a run can end
  while its batches are still read. `GroupedSort` copies its group
  numbers into a buffer instead of lending them.
- Each state's fields are grouped by what they do.

**Results**

`INCLUDE` of `heap_graph_dominator_tree` on
`test/data/heap_graph_object_for_benchmarks.pftrace`, release build:

| | Peak RSS | Buffers |
|---|---:|---:|
| #8331 | 305 MB | 57 MB |
| This PR | 270 MB | 53 MB |

Every heap graph table is byte-identical. Rerunning a one-row pipeline
costs about 2-3 ns more per step that holds state. Large runs are
unchanged.

**Testing**

- New `StateTest.AStepsRunEndsWithItsStream`.
- The affected unit test suites pass.
@LalitMaganti
LalitMaganti force-pushed the dev/lalitm/step-state-lifetimes branch from 422e2cb to e89c486 Compare October 30, 2026 21:37

This branch has not been deployed

No deployments
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.

1 participant