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.

Sign up for GitHub

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

Jump to bottom

Use a map container to store peers in tr_swarm #5645

Merged
merged 12 commits into from Jun 22, 2023
Merged

Use a map container to store peers in tr_swarm #5645

merged 12 commits into from Jun 22, 2023

Conversation

tearfur
Copy link
Member

@tearfur tearfur commented Jun 20, 2023 •
edited by ckerr

(Partially? Hopefully?) fixes #5610.

  1. To address the high CPU usage of tr_swarm::get_existing_atom(), the member pool of tr_swarm was changed from a dequeue to a map.
  2. Refactored related functions to streamline function calls.

Notes: Improved libtransmission code to use less CPU.

@tearfur tearfur mentioned this pull request Jun 20, 2023
Merged
@tearfur tearfur marked this pull request as ready for review June 20, 2023 15:04
@tearfur tearfur mentioned this pull request Jun 20, 2023
Merged
ckerr
ckerr previously requested changes Jun 20, 2023
View reviewed changes
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

@tearfur looks like this PR is coupling two unrelated topics, the std::map use here and the followup to the atom+port PR. Could you move the second category into a standalone PRs?

@tearfur
Copy link
Member Author

tearfur commented Jun 20, 2023

@ckerr Re-opened #5644 (comment)

@tearfur tearfur mentioned this pull request Jun 20, 2023
Closed
@ckerr ckerr added the needs update The PR has needs to be updated by the submitter label Jun 20, 2023
@ckerr ckerr self-requested a review June 20, 2023 18:11
@ckerr ckerr dismissed their stale review June 20, 2023 18:11

addressed

@tearfur
Merge branch 'main' into atoms-pool-map
10fbc8c 
# Conflicts:
#	libtransmission/peer-mgr.cc
@tearfur tearfur mentioned this pull request Jun 21, 2023
Closed
@tearfur
Copy link
Member Author

tearfur commented Jun 21, 2023

@ckerr PR's ready.

@ckerr ckerr added scope:core type:perf A code change that improves performance semver:patch make backwards compatible bug fixes and removed needs update The PR has needs to be updated by the submitter labels Jun 21, 2023
@ckerr ckerr added this to the 4.0.x milestone Jun 21, 2023
@ckerr ckerr modified the milestones: 4.0.x, 4.1.0 Jun 21, 2023
@ckerr ckerr added semver:minor adds functionality in a backwards compatible manner and removed semver:patch make backwards compatible bug fixes labels Jun 21, 2023
@blind-oracle
Copy link

Works as expected I guess

ckerr and others added 2 commits June 22, 2023 08:56
@ckerr
Merge branch 'main' into atoms-pool-map
4f3bea8
@tearfur
prevent accessing tr_swarm.pool twice in `tr_swarm::ensure_atom_exi…
617c17b 
…sts()`

Before this commit, `tr_swarm.pool` is traversed twice:
1. Inside `tr_swarm::get_existing_atom()`
2. By `std::map::emplace()`
@tearfur
Copy link
Member Author

tearfur commented Jun 22, 2023

@ckerr Just letting you know I did a small refactor to prevent traversing the tr_swarm.swarm twice in tr_swarm::ensure_atom_exists().

@ckerr ckerr merged commit 699b3d8 into transmission:main Jun 22, 2023
21 checks passed
@tearfur tearfur deleted the atoms-pool-map branch June 22, 2023 20:29
@tearfur tearfur restored the atoms-pool-map branch June 26, 2023 16:49
@tearfur tearfur deleted the atoms-pool-map branch June 26, 2023 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ckerr ckerr ckerr approved these changes

Assignees
No one assigned
Labels
scope:core semver:minor adds functionality in a backwards compatible manner type:perf A code change that improves performance
Milestone
4.1.0
Development

Successfully merging this pull request may close these issues.

transmission-daemon goes 100% cpu after some time
3 participants
@tearfur @blind-oracle @ckerr