Skip to content
This repository has been archived by the owner on Feb 1, 2023. It is now read-only.

More specific wantlists #74

Merged
merged 2 commits into from
Feb 20, 2019
Merged

More specific wantlists #74

merged 2 commits into from
Feb 20, 2019

Conversation

hannahhoward
Copy link
Contributor

Goals

Make handling of wantlists more clear, avoid memory copying with session tracking, remove unneeded mutexes

Implementation

Currently, the wantlist code obscures multiple different concerns:

  • tracking of sessions (only needed for our own wantlists and our own outgoing wantlists for individual peers)
  • thread safety
  • bookkeeping fields only needed for incoming peer wantlists

All of this is made worse by the fact that every field on a wantlist entry becomes a field on a Bitswap message entry, even though they are unused

This PR takes the following approach:

  • cut the wantlist public fields down to the ones that are used universally - CID & Priority
  • handle thread safety outside the wantlist itself, since it's already largely handled that way
  • move session tracking to private fields in a session tracked want list
  • move the trash bookkeeping field into a private field only inside peer-request-queues

For Discussion

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM. I'm starting to think that the MessageQueue should be session-ignorant but I'm not sure how to do that and we can tackle it later.

@Stebalien
Copy link
Member

At this point, we should be able to pass Entrys by value (saving an allocation and avoiding mutable shared state). However, we can fix that in a followup PR.

@hannahhoward hannahhoward force-pushed the feat/differentiate_wantList branch 2 times, most recently from 1a12448 to eb192a0 Compare February 20, 2019 23:00
@hannahhoward hannahhoward changed the base branch from bugs/racy-wantlist-handling to master February 20, 2019 23:10
@ghost ghost assigned Stebalien Feb 20, 2019
@Stebalien
Copy link
Member

Stebalien commented Feb 20, 2019

(fixed the rebase) (note to self: don't touch someone else's PR while they're working on it)

network MessageNetwork
wl *wantlist.ThreadSafe
refcnt int
Copy link
Member

Choose a reason for hiding this comment

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

Extra field.

Seperate want list into differentiated types - session tracking and regular

fix #13
put trash field only where it is needed, in peer request queues
@Stebalien Stebalien merged commit ab7ddf0 into master Feb 20, 2019
@ghost ghost removed the status/in-progress In progress label Feb 20, 2019
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
…ntList

More specific wantlists

This commit was moved from ipfs/go-bitswap@ab7ddf0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concurrent map access in Entry.SesTrk Use different types for our wantlists and peer wantlists
2 participants