peerstore: don't intern protocols (#2860)

* peerstore: don't intern protocols * peerstore: reduce default protocol limit to 128 * Remove unused mutex --------- Co-authored-by: Marco Munizaga <[email protected]>
libp2p · Jul 4, 2024 · 50a070d · 50a070d
1 parent e433d07
commit 50a070d
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 36 deletions.
diff --git a/p2p/host/peerstore/pstoreds/protobook.go b/p2p/host/peerstore/pstoreds/protobook.go
@@ -48,7 +48,7 @@ func NewProtoBook(meta pstore.PeerMetadata, opts ...ProtoBookOption) (*dsProtoBo
 			}
 			return ret
 		}(),
-		maxProtos: 1024,
+		maxProtos: 128,
 	}
 
 	for _, opt := range opts {

diff --git a/p2p/host/peerstore/pstoremem/protobook.go b/p2p/host/peerstore/pstoremem/protobook.go
@@ -26,9 +26,6 @@ type memoryProtoBook struct {
 	segments protoSegments
 
 	maxProtos int
-
-	lk       sync.RWMutex
-	interned map[protocol.ID]protocol.ID
 }
 
 var _ pstore.ProtoBook = (*memoryProtoBook)(nil)
@@ -44,7 +41,6 @@ func WithMaxProtocols(num int) ProtoBookOption {
 
 func NewProtoBook(opts ...ProtoBookOption) (*memoryProtoBook, error) {
 	pb := &memoryProtoBook{
-		interned: make(map[protocol.ID]protocol.ID, 256),
 		segments: func() (ret protoSegments) {
 			for i := range ret {
 				ret[i] = &protoSegment{
@@ -53,7 +49,7 @@ func NewProtoBook(opts ...ProtoBookOption) (*memoryProtoBook, error) {
 			}
 			return ret
 		}(),
-		maxProtos: 1024,
+		maxProtos: 128,
 	}
 
 	for _, opt := range opts {
@@ -64,38 +60,14 @@ func NewProtoBook(opts ...ProtoBookOption) (*memoryProtoBook, error) {
 	return pb, nil
 }
 
-func (pb *memoryProtoBook) internProtocol(proto protocol.ID) protocol.ID {
-	// check if it is interned with the read lock
-	pb.lk.RLock()
-	interned, ok := pb.interned[proto]
-	pb.lk.RUnlock()
-
-	if ok {
-		return interned
-	}
-
-	// intern with the write lock
-	pb.lk.Lock()
-	defer pb.lk.Unlock()
-
-	// check again in case it got interned in between locks
-	interned, ok = pb.interned[proto]
-	if ok {
-		return interned
-	}
-
-	pb.interned[proto] = proto
-	return proto
-}
-
 func (pb *memoryProtoBook) SetProtocols(p peer.ID, protos ...protocol.ID) error {
 	if len(protos) > pb.maxProtos {
 		return errTooManyProtocols
 	}
 
 	newprotos := make(map[protocol.ID]struct{}, len(protos))
 	for _, proto := range protos {
-		newprotos[pb.internProtocol(proto)] = struct{}{}
+		newprotos[proto] = struct{}{}
 	}
 
 	s := pb.segments.get(p)
@@ -121,7 +93,7 @@ func (pb *memoryProtoBook) AddProtocols(p peer.ID, protos ...protocol.ID) error
 	}
 
 	for _, proto := range protos {
-		protomap[pb.internProtocol(proto)] = struct{}{}
+		protomap[proto] = struct{}{}
 	}
 	return nil
 }
@@ -151,7 +123,10 @@ func (pb *memoryProtoBook) RemoveProtocols(p peer.ID, protos ...protocol.ID) err
 	}
 
 	for _, proto := range protos {
-		delete(protomap, pb.internProtocol(proto))
+		delete(protomap, proto)
+	}
+	if len(protomap) == 0 {
+		delete(s.protocols, p)
 	}
 	return nil
 }

diff --git a/p2p/protocol/identify/id_test.go b/p2p/protocol/identify/id_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math/rand"
 	"slices"
 	"sync"
 	"testing"
@@ -730,6 +731,15 @@ func TestLargeIdentifyMessage(t *testing.T) {
 	}
 }
 
+func randString(n int) string {
+	chars := "abcdefghijklmnopqrstuvwxyz"
+	buf := make([]byte, n)
+	for i := 0; i < n; i++ {
+		buf[i] = chars[rand.Intn(len(chars))]
+	}
+	return string(buf)
+}
+
 func TestLargePushMessage(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -738,9 +748,9 @@ func TestLargePushMessage(t *testing.T) {
 	h2 := blhost.NewBlankHost(swarmt.GenSwarm(t))
 
 	// add protocol strings to make the message larger
-	// about 2K of protocol strings
-	for i := 0; i < 500; i++ {
-		r := protocol.ID(fmt.Sprintf("rand%d", i))
+	// about 3K of protocol strings
+	for i := 0; i < 100; i++ {
+		r := protocol.ID(fmt.Sprintf("%s-%d", randString(30), i))
 		h1.SetStreamHandler(r, func(network.Stream) {})
 		h2.SetStreamHandler(r, func(network.Stream) {})
 	}