search

multiformats / go-multihash

Multihash implementation in Go
MIT License
234 stars 61 forks source link

Make Multihash a string instead of a []byte? #29

Open kevina opened 7 years ago

kevina commented 7 years ago

Currently Multihash is defined as:

type Multihash []byte

A multihash may be binary, but it is small and (I hope) immutable. In my view a string is a better fit since it will ensure that it will not change. It will also take less memory to point too (two pointers, instead of three).

This relates to ipfs/go-cid#3.

whyrusleeping commented 7 years ago

The reason i vote in favor of using a []byte is because once we move to using a string, anything that needs to copy or write out a multihash needs to make a copy of it first, and those can get expensive.

kevina commented 7 years ago

I suppose that is true. It is really annoying that go does not have a concept of "const" as that can avoid most of these unnecessary copies, for example having to make a copy to pass a string into a function that expects a []byte even though there the function will not modify the data.

kevina commented 6 years ago

And the argument can also be that more things end up needing it as a string that a []byte. @Stebalien what are your thoughts on this manor.

Changing the type of Multihash will be a simple but API breaking change. This perhaps should be done at the same time as https://github.com/ipfs/go-cid/issues/3 (if we decide to do that) to minimize the pain.

Stebalien commented 6 years ago

I see CIDs used as keys in maps all the time and using a string would help there a lot. Honestly, I'm not sure (we'd have to test it).

kevina commented 6 years ago

@Stebalien I think you are you are confusing CID with mutlihashes, they are now two different things.

Stebalien commented 6 years ago

Ah. Sorry, too many issues open (I care about this because it's one route to make CID's usable as map keys).