basichost: don't allocate when deduplicating multiaddrs (#2206)

* basichost: don't allocate when deduplicating multiaddrs * fix sort.Slice less comparison Co-authored-by: Marco Munizaga <[email protected]> * interop: don't send loopback addrs --------- Co-authored-by: Marco Munizaga <[email protected]>
libp2p · Apr 12, 2023 · 7dc3b81 · 7dc3b81
1 parent ba328ae
commit 7dc3b81
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 11 deletions.
diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go
@@ -1,11 +1,13 @@
 package basichost
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
 	"io"
 	"net"
+	"sort"
 	"sync"
 	"time"
 
@@ -809,18 +811,24 @@ func (h *BasicHost) NormalizeMultiaddr(addr ma.Multiaddr) ma.Multiaddr {
 	return addr
 }
 
-// mergeAddrs merges input address lists, leave only unique addresses
-func dedupAddrs(addrs []ma.Multiaddr) (uniqueAddrs []ma.Multiaddr) {
-	exists := make(map[string]bool)
-	for _, addr := range addrs {
-		k := string(addr.Bytes())
-		if exists[k] {
-			continue
+// dedupAddrs deduplicates addresses in place, leave only unique addresses.
+// It doesn't allocate.
+func dedupAddrs(addrs []ma.Multiaddr) []ma.Multiaddr {
+	if len(addrs) == 0 {
+		return addrs
+	}
+	sort.Slice(addrs, func(i, j int) bool { return bytes.Compare(addrs[i].Bytes(), addrs[j].Bytes()) < 0 })
+	idx := 1
+	for i := 1; i < len(addrs); i++ {
+		if !addrs[i-1].Equal(addrs[i]) {
+			addrs[idx] = addrs[i]
+			idx++
 		}
-		exists[k] = true
-		uniqueAddrs = append(uniqueAddrs, addr)
 	}
-	return uniqueAddrs
+	for i := idx; i < len(addrs); i++ {
+		addrs[i] = nil
+	}
+	return addrs[:idx]
 }
 
 // AllAddrs returns all the addresses of BasicHost at this moment in time.

diff --git a/p2p/host/basic/basic_host_test.go b/p2p/host/basic/basic_host_test.go
@@ -823,3 +823,29 @@ func TestNormalizeMultiaddr(t *testing.T) {
 
 	require.Equal(t, "/ip4/1.2.3.4/udp/9999/quic-v1/webtransport", h1.NormalizeMultiaddr(ma.StringCast("/ip4/1.2.3.4/udp/9999/quic-v1/webtransport/certhash/uEgNmb28")).String())
 }
+
+func TestDedupAddrs(t *testing.T) {
+	tcpAddr := ma.StringCast("/ip4/127.0.0.1/tcp/1234")
+	quicAddr := ma.StringCast("/ip4/127.0.0.1/udp/1234/quic-v1")
+	wsAddr := ma.StringCast("/ip4/127.0.0.1/tcp/1234/ws")
+
+	type testcase struct {
+		in, out []ma.Multiaddr
+	}
+
+	for i, tc := range []testcase{
+		{in: nil, out: nil},
+		{in: []ma.Multiaddr{tcpAddr}, out: []ma.Multiaddr{tcpAddr}},
+		{in: []ma.Multiaddr{tcpAddr, tcpAddr, tcpAddr}, out: []ma.Multiaddr{tcpAddr}},
+		{in: []ma.Multiaddr{tcpAddr, quicAddr, tcpAddr}, out: []ma.Multiaddr{tcpAddr, quicAddr}},
+		{in: []ma.Multiaddr{tcpAddr, quicAddr, wsAddr}, out: []ma.Multiaddr{tcpAddr, quicAddr, wsAddr}},
+	} {
+		tc := tc
+		t.Run(fmt.Sprintf("test %d", i), func(t *testing.T) {
+			deduped := dedupAddrs(tc.in)
+			for _, a := range tc.out {
+				require.Contains(t, deduped, a)
+			}
+		})
+	}
+}
diff --git a/test-plans/cmd/ping/main.go b/test-plans/cmd/ping/main.go
@@ -28,6 +28,7 @@ import (
 	"github.com/libp2p/go-libp2p/p2p/transport/websocket"
 	libp2pwebtransport "github.com/libp2p/go-libp2p/p2p/transport/webtransport"
 	ma "github.com/multiformats/go-multiaddr"
+	manet "github.com/multiformats/go-multiaddr/net"
 )
 
 func main() {
@@ -201,7 +202,14 @@ func main() {
 		}
 		fmt.Println(string(testResultJSON))
 	} else {
-		_, err := rClient.RPush(ctx, "listenerAddr", host.Addrs()[0].Encapsulate(ma.StringCast("/p2p/"+host.ID().String())).String()).Result()
+		var listenAddr ma.Multiaddr
+		for _, addr := range host.Addrs() {
+			if !manet.IsIPLoopback(addr) {
+				listenAddr = addr
+				break
+			}
+		}
+		_, err := rClient.RPush(ctx, "listenerAddr", listenAddr.Encapsulate(ma.StringCast("/p2p/"+host.ID().String())).String()).Result()
 		if err != nil {
 			log.Fatal("Failed to send listener address")
 		}