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
Remove *Serial types. Use pointers for all types. #688
Conversation
This takes advantange of the latest features in go-cid, peer.ID and go-multiaddr and makes the Go types serializable by default. This means we no longer need to copy between Pin <-> PinSerial, or ID <-> IDSerial etc. We can now efficiently binary-encode these types using short field keys and without parsing/stringifying (in many cases it just a cast). We still get the same json output as before (with minor modifications for Cids). This should greatly improve Cluster performance and memory usage when dealing with large collections of items. License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
It has been HORRIBLE to do this, but I think we had to do it sooner or later. I'm super glad we have so many tests. The main things here are in |
api/rest/restapi_test.go
Outdated
makeGet(t, rest, url(rest)+"/id", &id) | ||
if id.ID != test.TestPeerID1.Pretty() { | ||
if id.ID.Pretty() != test.TestPeerID1.Pretty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could do id.ID == test.TestPeerID1
but that's depending on the fact that peer IDs are strings, which may change. Ideailly, IDs should provide an Equals method, like Cids do.
ifaces := make([]interface{}, len(in), len(in)) | ||
for i := range in { | ||
ifaces[i] = &in[i] | ||
in[i] = &api.ID{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not doing this causes very bad errors
License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
Since now we can work with |
License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
Fixes #654 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the debug init, LGTM.
License: MIT Signed-off-by: Hector Sanjuan <hector@protocol.ai>
This takes advantange of the latest features in go-cid, peer.ID and
go-multiaddr and makes the Go types serializable by default.
This means we no longer need to copy between Pin <-> PinSerial, or ID <->
IDSerial etc. We can now efficiently binary-encode these types using short
field keys and without parsing/stringifying (in many cases it just a cast).
We still get the same json output as before (with minor modifications for
Cids).
This should greatly improve Cluster performance and memory usage when dealing
with large collections of items.
License: MIT
Signed-off-by: Hector Sanjuan hector@protocol.ai