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

optimise pin update command #4348

Merged
merged 3 commits into from
Oct 31, 2017
Merged

optimise pin update command #4348

merged 3 commits into from
Oct 31, 2017

Conversation

ianopolous
Copy link
Member

This handles merkle links that aren't named. And improves the
Peergos usage from worst case 30s to ~20ms

@ghost ghost assigned ianopolous Oct 28, 2017
@ghost ghost added the status/in-progress In progress label Oct 28, 2017
This handles merkle links that aren't named. And improves the
Peergos usage from worst case 30s to ~20ms

License: MIT
Signed-off-by: Ian Preston <ianopolous@protonmail.com>
var aonly []*cid.Cid
for _, l := range b.Links() {
inb[l.Cid.KeyString()] = l
}
for _, l := range a.Links() {
Copy link
Member

Choose a reason for hiding this comment

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

It will be faster to just call KeyString() once, bind it to a local variable here. (also easier on memory)

Copy link
Member Author

Choose a reason for hiding this comment

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

done

}

var out []diffpair
var aindex = 0
Copy link
Member

Choose a reason for hiding this comment

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

you can just say var aindex int

License: MIT
Signed-off-by: Ian Preston <ianopolous@protonmail.com>
@ianopolous ianopolous force-pushed the fix/pin_update_opt branch 2 times, most recently from 8e4d5e9 to 825add0 Compare October 28, 2017 13:02
@whyrusleeping
Copy link
Member

I think we should make sure that the name pairing still works properly with this change, so that links with the same name (or 'path' within the object) still create the same resulting 'change' object

@ianopolous
Copy link
Member Author

I believe that the name matching property will be maintained with this pr, as long as the Links() function is only a function of the ipld topology of the object. I will write a test to verify this.

License: MIT
Signed-off-by: Ian Preston <ianopolous@protonmail.com>
@@ -65,27 +65,35 @@ type diffpair struct {
// getLinkDiff returns a changeset between nodes 'a' and 'b'. Currently does
// not log deletions as our usecase doesnt call for this.
func getLinkDiff(a, b node.Node) []diffpair {
have := make(map[string]*node.Link)
names := make(map[string]*node.Link)
Copy link
Member

Choose a reason for hiding this comment

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

Looks right. The Name property of links is a protonode thing (that needs to die).

@whyrusleeping whyrusleeping merged commit 7c8da64 into master Oct 31, 2017
@ghost ghost removed the status/in-progress In progress label Oct 31, 2017
@whyrusleeping whyrusleeping deleted the fix/pin_update_opt branch October 31, 2017 12:51
@ianopolous
Copy link
Member Author

Hooray! My first commits to go-ipfs! :-)

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.

3 participants