optimize peerstore memory #71
Conversation
Ideally, we'd store a bitmap. But we'll have to think through how we want that to play out with the datastore-backed peerstore (maybe move the `*Protocols` methods down to the backend implementation?).
ghost
assigned 
| @@ -75,13 +80,30 @@ func (ps *peerstore) PeerInfo(p peer.ID) PeerInfo { | |||
| } | |||
| } | |||
|
|
|||
| func (ps *peerstore) internProtocol(s string) string { | |||
| if len(s) > maxInternedProtocolSize { | |||
There was a problem hiding this comment.
These checks are in case we're using, e.g., the datastore backed peerstore.
|
Ideally, we'd handle this in the actual peerstore implementation but that would require pushing these functions down into the implementation. If we do that, we can, e.g., use a bitfield in the memory version and some kind of id system in the datastore backed version. |
peerstore.go
Outdated
| @@ -10,6 +10,9 @@ import ( | |||
|
|
|||
| var _ Peerstore = (*peerstore)(nil) | |||
|
|
|||
| const maxInternedProtocols = 256 | |||
| const maxInternedProtocolSize = 96 | |||
There was a problem hiding this comment.
let's make these a little larger.
There was a problem hiding this comment.
I think the sizing is fine; can't really see protocol strings being longer than 96 chars nor us handling 256 unique proto IDs over a long period. (Opinion weakly held).
There was a problem hiding this comment.
Upped. Note: I consider this to be a temporary fix. We should probably specialize protocol handling and push it down into the peerstore implementations.
There was a problem hiding this comment.
I doubt this'll be an issue but 512/256 is probably fine anyways (128KiB).
512 * 256 (max path length on many systems) = 128KiB
fixes #68