store Multiaddrs as strings and don't use interfaces

[Stebalien]

1. This saves one level of indirection, one distinct allocation, and a few bytes.
2. This allows Multiaddrs to be used as map keys (we can use this to make our Peerstore more efficient).
3. Removing the interface allows us to serialize these without copying.

This commit isn't as efficient as it could be. We could improve it slightly by using unsafe casts to temporarily treat strings as bytes. However I'm not sure this will improve performance much and, in general, this change will lead to fewer allocations overall.

[Stebalien]

The tests are failing because this change breaks GX. We'd have to fix that concurrently with this PR.

@whyrusleeping thoughts?

This will be a real pain (maddr == nil is broken) but, it will save us a ton of byte slice allocations in the DHT (we send out a lot of multiaddrs and copy them every time).

An alternative is to define multiaddrs to be type Multiaddr []byte (allowing a cheep cast via []byte(Multiaddr)) but that wouldn't allow us to use them in map keys. On the other hand maddr == nil will still work.

[whyrusleeping]

@Stebalien why not keep the interface, and just do:

type multiaddr string

That way we get all the same benefits, plus the ability to have nil multiaddrs (making the upgrade path easier)

[Stebalien]

Note: I'm pretty sure that'll still require an additional allocation/indirection but it should be fairly cheep.

[Kubuxu]

I think Go is smart enough and allocates the type header in the same call as the string.

[Stebalien]

I think Go is smart enough and allocates the type header in the same call as the string.

It's the string header I'm worried about. Strings are usually:

• data ->
  • "data"
• length
• capacity

And interfaces are usually:

• vtable ->
  • ...
• data ->
  • ...

So, a string wrapped in an interface should be:

• vtable ->
  • ...
• data ->
  • data ->
    • "data"
  • length
  • capacity

From a bit of quick testing, it looks like this is correct.

[Kubuxu]

From a bit of quick testing, it looks like this is correct.

correct as in "it adds allocation"?

[Stebalien]

Correct as in it adds a level of indirection. Go may do something sneaky with the allocation but I kind of doubt it. Go could do something to avoid an extra object to GC (e.g., have some concept of ownership) but the complexity probably isn't worth it.