Remove a lot of allocations, and fix some ambiguous naming #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned 
|
@monotone any thoughts? |
Stebalien
approved these changes
Apr 9, 2019
Additionally, there it may require less allocations to pack the interface value to sort.Sort now, and lint warning about modifying a receiver in peerDistanceSorter.appendPeersFromList goes away.
|
I think it’s pretty good now, it’s much clearer than before. thanks @anacrolix |
Stebalien
approved these changes
Apr 10, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I tidied up some ambiguous behaviours after #29. In particular there's some manual copying that isn't necessary, and the user of
[]*peerDistancewhen[]peerDistancewill suffice (and the last change here was an attempt to reduce allocations). Copying is replaced with append to make it clear about how to preallocate, and that the slice can/will be modified in place per normal append semantics.I see that with unusual
countvalues given toNearestPeers, behaviour isn't optimal, but I'm guessing that's not come up as count is likelyKValuewhich is approximately the bucket size most of the time.SortClosestPeersisn't used anywhere that I can see. I wonder if it should even be in this repo.