Skip to content

stem: do not track credits for unreliable outs#8642

Merged
mmcgee-jump merged 1 commit intomainfrom
mmcgee/gossbound1
Mar 3, 2026
Merged

stem: do not track credits for unreliable outs#8642
mmcgee-jump merged 1 commit intomainfrom
mmcgee/gossbound1

Conversation

@mmcgee-jump
Copy link
Contributor

No description provided.

@mmcgee-jump mmcgee-jump added this to the v1.0 milestone Mar 2, 2026
@mmcgee-jump mmcgee-jump enabled auto-merge (rebase) March 2, 2026 23:39
Copy link
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 updates fd_stem credit accounting so that output links without reliable consumers do not participate in flow-control credit tracking, and adjusts the gossip tile’s STEM_BURST sizing to reflect per-output worst cases.

Changes:

  • Refines STEM_BURST in the gossip tile to bound per-output publishes between credit checks (reliable links only).
  • Adds an out_reliable mask to fd_stem_context_t and uses it to skip credit decrement/min tracking for unreliable outputs.
  • Extends fd_stem scratch layout/allocation to include the out_reliable array and marks outputs as reliable based on consumer wiring.

Reviewed changes

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

File Description
src/discof/gossip/fd_gossip_tile.c Reworks STEM_BURST definition/documentation to reflect per-link worst-case bursts for reliable outputs.
src/disco/stem/fd_stem.h Adds out_reliable to the stem context and gates credit decrement/min tracking on it.
src/disco/stem/fd_stem.c Allocates/initializes out_reliable in scratch and sets it based on consumer wiring; adjusts cr_max initialization.

out_reliable = (int *)FD_SCRATCH_ALLOC_APPEND( l, alignof(int), out_cnt*sizeof(int) );

ulong cr_max = fd_ulong_if( !out_cnt, 128UL, ULONG_MAX );
ulong cr_max = fd_ulong_if( !out_cnt || !cons_cnt, 128UL, ULONG_MAX );
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

Setting cr_max to 128 when cons_cnt==0 makes stem treat the tile as permanently/backward-pressure limited (min_cr_avail will never reach burst if burst>128) and can also trigger the "insufficient depth for STEM_BURST" fatal error even though there are no reliable consumers. For the no-consumer case, cr_max/min_cr_avail should be set so that the run loop never backpressures (e.g., keep the previous ULONG_MAX behavior or explicitly bypass burst/credit checks when cons_cnt==0).

Suggested change
ulong cr_max = fd_ulong_if( !out_cnt || !cons_cnt, 128UL, ULONG_MAX );
ulong cr_max = (!cons_cnt)
? ULONG_MAX
: fd_ulong_if( !out_cnt, 128UL, ULONG_MAX );

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Mar 2, 2026

Performance Measurements ⏳

Suite Baseline New Change
backtest mainnet-368528500-perf per slot 0.071413 s 0.071719 s 0.428%
backtest mainnet-368528500-perf snapshot load 3.135 s 2.751 s -12.249%
backtest mainnet-368528500-perf total elapsed 71.413282 s 71.718913 s 0.428%
firedancer mem usage with mainnet.toml 964.37 GiB 964.37 GiB 0.000%

Copilot AI review requested due to automatic review settings March 3, 2026 00:19
@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Performance Measurements ⏳

Suite Baseline New Change
backtest mainnet-368528500-perf per slot 0.074931 s 0.075573 s 0.857%
backtest mainnet-368528500-perf snapshot load 4.308 s 3.087 s -28.343%
backtest mainnet-368528500-perf total elapsed 74.930722 s 75.57284 s 0.857%
firedancer mem usage with mainnet.toml 964.37 GiB 964.37 GiB 0.000%

Copy link
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

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

@mmcgee-jump mmcgee-jump merged commit 9214aad into main Mar 3, 2026
21 checks passed
@mmcgee-jump mmcgee-jump deleted the mmcgee/gossbound1 branch March 3, 2026 01:54
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