Skip to content

Avoid heap-allocating background processor futures#4733

Open
Abeeujah wants to merge 1 commit into
lightningdevkit:mainfrom
Abeeujah:drop-box
Open

Avoid heap-allocating background processor futures#4733
Abeeujah wants to merge 1 commit into
lightningdevkit:mainfrom
Abeeujah:drop-box

Conversation

@Abeeujah

Copy link
Copy Markdown
Contributor
The persistence loop in process_events_async boxed each task future
(ChannelManager, network graph, scorer, sweeper, LiquidityManager)
before handing it to the Joiner, incurring a heap allocation per task
on every iteration. Box::pin was only needed to satisfy the Joiner's
Unpin bound when our MSRV predated core::pin::pin!.

Now that the MSRV is 1.75, replace Box::pin with core::pin::pin! so the
futures live on the stack. Pin<&mut F> is Unpin, so the Joiner bounds
still hold. To pin every future before the Joiner borrows it, build all
five uniformly at the end of the iteration: run the synchronous timer
checks and side effects first, capture their outcomes in flags, and
gate each future's work behind those flags. Futures whose work is
skipped resolve to Ok(()) immediately, preserving the previous behavior
where an unset Joiner slot was treated as Ready(Ok(())).

The eager single poll of the ChannelManager future and the early
loop-exit (break) semantics on the scorer/sweeper timers are unchanged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 20, 2026

Copy link
Copy Markdown

I've assigned @valentinewallace as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-claude-review-bot

ldk-claude-review-bot commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

I've re-verified the full refactored block against the original behavior. The code matches my prior analysis: side-effect ordering, skipped-work semantics, eager CM poll, break semantics, and drop order are all preserved.

No new issues found beyond what I noted previously.

The scorer_fut formatting (lib.rs:1294-1295 — the let scorer_fut = split before core::pin::pin!) is still inconsistent with the sibling network_graph_fut/sweeper_fut bindings and would likely be reformatted by cargo fmt; this was already raised in the prior pass and remains a non-blocking style note, not a new finding.

No issues found.

Comment thread lightning-background-processor/src/lib.rs Outdated
Replace Box::pin with core::pin::pin! in process_events_async now that
MSRV is 1.75. This eliminates a heap allocation per task on every
loop iteration by pinning the futures directly to the stack.

To satisfy lifetime and Joiner bounds, the loop logic was refactored
to run synchronous timer checks first, using flags to conditionally
execute the stack-pinned futures. Existing eager polling and early-break
semantics are preserved.
@ldk-reviews-bot

Copy link
Copy Markdown

🔔 1st Reminder

Hey @valentinewallace! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

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.

4 participants