Skip to content

refactor: Use dispatch instead of post#7438

Open
bthomee wants to merge 3 commits into
bthomee/peer_chargefrom
bthomee/dispatch
Open

refactor: Use dispatch instead of post#7438
bthomee wants to merge 3 commits into
bthomee/peer_chargefrom
bthomee/dispatch

Conversation

@bthomee

@bthomee bthomee commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

This refactor changes the post calls to dispatch calls.

Context of Change

This PR follows up on #7422. Dispatching the work to the strand makes the code a lot easier to read and maintain than posting the work.

Copilot AI review requested due to automatic review settings June 9, 2026 17:30

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

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.

No issues.

Review by Claude Sonnet 4.6 · Prompt: V15

Copilot AI left a comment

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.

Pull request overview

This PR refactors PeerImp’s strand scheduling to use boost::asio::dispatch in place of post + running_in_this_thread() checks, aligning more operations to consistently execute on the peer’s strand (following the direction from #7422).

Changes:

  • Replaced multiple post(strand_, ...) call sites with dispatch(strand_, ...) lambdas for peer lifecycle and message-handling entry points.
  • Centralized strand-hopping behavior by wrapping methods like run, stop, send, and tx-queue helpers in dispatch.
  • Removed boost::asio/post.hpp include and eliminated remaining post(...) usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/xrpld/overlay/detail/PeerImp.cpp Outdated
Comment thread src/xrpld/overlay/detail/PeerImp.cpp Outdated
Comment thread src/xrpld/overlay/detail/PeerImp.cpp
Comment thread src/xrpld/overlay/detail/PeerImp.cpp
Comment thread src/xrpld/overlay/detail/PeerImp.cpp

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

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.

No issues.

Review by Claude Sonnet 4.6 · Prompt: V15

Copilot AI left a comment

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.

Pull request overview

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

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 120 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.3%. Comparing base (d076174) to head (e9c21a8).

Files with missing lines Patch % Lines
src/xrpld/overlay/detail/PeerImp.cpp 0.0% 120 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##           bthomee/peer_charge   #7438   +/-   ##
===================================================
  Coverage                 82.3%   82.3%           
===================================================
  Files                     1010    1010           
  Lines                    76965   76956    -9     
  Branches                  8984    8976    -8     
===================================================
- Hits                     63368   63367    -1     
+ Misses                   13588   13580    -8     
  Partials                     9       9           
Files with missing lines Coverage Δ
src/xrpld/overlay/detail/PeerImp.cpp 5.7% <0.0%> (+<0.1%) ⬆️

... and 1 file with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bthomee bthomee requested review from godexsoft and vlntb June 10, 2026 12:29

@godexsoft godexsoft left a comment

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.

Surprised captured self is not flagged by clang-tidy for places it is unused.. but it seems to work so all good 👍

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

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.

No issues.

Review by Claude Sonnet 4.6 · Prompt: V15

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

post(strand_, std::bind(&PeerImp::run, shared_from_this()));
return;
}
dispatch(strand_, [self = shared_from_this()]() {
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