Skip to content

Add physical parameters to OutputSink - #9585

Open
connortsui20 wants to merge 1 commit into
claude/generic-output-sink-etzm2lfrom
ct/sink-params
Open

Add physical parameters to OutputSink#9585
connortsui20 wants to merge 1 commit into
claude/generic-output-sink-etzm2lfrom
ct/sink-params

Conversation

@connortsui20

@connortsui20 connortsui20 commented Aug 24, 2026

Copy link
Copy Markdown
Member

Summary

This adds a physical-parameters channel to OutputSink and stacks on #9581, completing runtime logical and physical output selection separately.

Changes

OutputSink::Params now flows through sink visits, validation, and allocation. For example, FixedSizeListSink (new) uses an options-derived runtime width with safe complete-row initialization.

API Changes

The unstable row-function API now requires sink parameters for visit_into and visit_prepared_into; sinks that need none just use ().

@codspeed-hq

codspeed-hq Bot commented Aug 24, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 13.04%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 1 improved benchmark
✅ 1980 untouched benchmarks
⏩ 54 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime words_gather_scalar_avx2[65536] 9.3 µs 8.3 µs +13.04%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/sink-params (f50516b) with claude/generic-output-sink-etzm2l (f3ca34d)

Open in CodSpeed

Footnotes

  1. 54 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 marked this pull request as ready for review August 24, 2026 16:06
@connortsui20 connortsui20 added the changelog/break A breaking API change label Aug 24, 2026
@connortsui20
connortsui20 marked this pull request as draft August 24, 2026 16:07
@connortsui20
connortsui20 marked this pull request as ready for review August 24, 2026 16:07
// Dense execution leaves output uninitialized. Nullable branches fill placeholders only when
// they need to skip invalid rows.
visitor.visit_into::<(T, T), UninitElementSink<T>, _>(|(lhs, rhs), output| {
visitor.visit_into::<(T, T), UninitElementSink<T>, _>((), |(lhs, rhs), output| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would be nice to not have new ()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah it would be nice, but don't really want to add more API surface just for this (and it depends on the output sink you choose at the end so not all of them are like this)

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

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants