Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(swarm): Show task spawn paths in tokio-console #5465

Merged
merged 7 commits into from
Jul 4, 2024

Conversation

oblique
Copy link
Contributor

@oblique oblique commented Jun 11, 2024

Description

This PR adds #[track_caller] on all spawn wrappers.

Notes & open questions

#[track_caller] on a trait does not affect or break its definition.

Before
screenshot_2024-06-11_15-45-16

After
screenshot_2024-06-11_16-00-54

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@dhuseby
Copy link
Contributor

dhuseby commented Jul 2, 2024

@oblique we have a rust-libp2p maintainer's call every two weeks that I think you should attend. The next one is starting right now: https://lu.ma/2024-07-02-rust-libp2p

The one after that is in two weeks: https://lu.ma/2024-07-16-rust-libp2p

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

Hi Yianis, thanks for this. Can you explain the difference in the two pictures that putting #[track_caller] addresses? And shouldn't this be covered in tokio instead, like this PR addresses? Thanks!

@oblique
Copy link
Contributor Author

oblique commented Jul 2, 2024

Can you explain the difference in the two pictures that putting #[track_caller] addresses?

In the before picture, all libp2p's tasks show that they started from executor.rs:41, which actually is a wrapper function to tokio::spawn.

In the after picture you can see pool.rs:538 and pool.rs:437, which are correct place that the tasks were spawned by the wrapper.

And shouldn't this be covered in tokio instead, like tokio-rs/tokio#4483 PR addresses?

Tokio covers this. However, because we define a wrapper to tokio::spawn we are responsible to propagate this tracking mechanism.

jxs
jxs previously approved these changes Jul 3, 2024
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

Ok thanks LGTM!

@jxs jxs added the send-it label Jul 3, 2024
@mergify mergify bot dismissed jxs’s stale review July 3, 2024 16:29

Approvals have been dismissed because the PR was updated after the send-it label was applied.

@mergify mergify bot merged commit 693d7b7 into libp2p:master Jul 4, 2024
72 checks passed
TimTinkers pushed a commit to unattended-backpack/rust-libp2p that referenced this pull request Sep 14, 2024
This PR adds `#[track_caller]` on all `spawn` wrappers.

Pull-Request: libp2p#5465.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants