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

bitswap nits #4458

Merged
merged 8 commits into from
Dec 8, 2017
Merged

bitswap nits #4458

merged 8 commits into from
Dec 8, 2017

Conversation

Stebalien
Copy link
Member

No description provided.

@ghost ghost assigned Stebalien Dec 5, 2017
@ghost ghost added the status/in-progress In progress label Dec 5, 2017
@magik6k
Copy link
Member

magik6k commented Dec 5, 2017

Test fail on Jenkins looks unrelated:

ok 11 - 'ipfs daemon' succeeds

expecting success: 
    test_wait_for_file 20 100ms "$IPFS_PATH/api"
  
Error: timed out waiting for file: /go/src/github.com/ipfs/go-ipfs/test/sharness/trash directory.t0045-ls.sh/.ipfs/api
not ok 12 - api file shows up
#	
#	    test_wait_for_file 20 100ms "$IPFS_PATH/api"
#	

@@ -372,16 +372,6 @@ func (bs *Bitswap) ReceiveMessage(ctx context.Context, p peer.ID, incoming bsmsg
return
}

// quickly send out cancels, reduces chances of duplicate block receives
var keys []*cid.Cid
Copy link
Member Author

Choose a reason for hiding this comment

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

@whyrusleeping this code was dead. We're currently sending out cancels from within sessions. However, this does mean that the above comment is no longer correct (it'll likely take a bit longer to send out cancels).

@Stebalien
Copy link
Member Author

This should hopefully make gc a bit happier when bitswapping.

Copy link
Member

@frrist frrist left a comment

Choose a reason for hiding this comment

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

LGTM

Avoids lots of reallocations under a lock.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
Also, don't call time.Now in a loop.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@@ -336,12 +337,12 @@ func (e *Engine) numBytesReceivedFrom(p peer.ID) uint64 {
// ledger lazily instantiates a ledger
func (e *Engine) findOrCreate(p peer.ID) *ledger {
e.lock.Lock()
defer e.lock.Unlock()
Copy link
Member

Choose a reason for hiding this comment

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

I do want to note that this may cause a perf degradation

@whyrusleeping whyrusleeping merged commit 8b90b70 into master Dec 8, 2017
@ghost ghost removed the status/in-progress In progress label Dec 8, 2017
@whyrusleeping whyrusleeping deleted the fix/bitswap-nits branch December 8, 2017 22:00
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