From 8c02e08cd3b6219a2aeb0604c1de89243f1a0ffd Mon Sep 17 00:00:00 2001 From: Maxim Merzhanov Date: Sun, 12 Apr 2020 22:12:55 +0300 Subject: [PATCH 1/2] reduce complexity in update addr book method --- pstoreds/addr_book.go | 53 +++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/pstoreds/addr_book.go b/pstoreds/addr_book.go index d0998de..f57c9a0 100644 --- a/pstoreds/addr_book.go +++ b/pstoreds/addr_book.go @@ -15,6 +15,7 @@ import ( "github.com/libp2p/go-libp2p-core/peer" pstore "github.com/libp2p/go-libp2p-core/peerstore" + "github.com/libp2p/go-libp2p-core/record" pb "github.com/libp2p/go-libp2p-peerstore/pb" "github.com/libp2p/go-libp2p-peerstore/pstoremem" @@ -467,36 +468,40 @@ func (ab *dsAddrBook) setAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Duratio // } newExp := time.Now().Add(ttl).Unix() - // TODO this is very inefficient O(m*n); we could build a map to use as an - // index, and test against it. That would turn it into O(m+n). This code - // will be refactored entirely anyway, and it's not being used by users - // (that we know of); so OK to keep it for now. - updateExisting := func(entryList []*pb.AddrBookRecord_AddrEntry, incoming ma.Multiaddr) *pb.AddrBookRecord_AddrEntry { - for _, have := range entryList { - if incoming.Equal(have.Addr) { - switch mode { - case ttlOverride: - have.Ttl = int64(ttl) - have.Expiry = newExp - case ttlExtend: - if int64(ttl) > have.Ttl { - have.Ttl = int64(ttl) - } - if newExp > have.Expiry { - have.Expiry = newExp - } - default: - panic("BUG: unimplemented ttl mode") - } - return have + var addrsMap map[string]*pb.AddrBookRecord_AddrEntry + if len(addrs) > 0 { + addrsMap = make(map[string]*pb.AddrBookRecord_AddrEntry, len(pr.Addrs)) + for _, addr := range pr.Addrs { + addrsMap[addr.Addr.String()] = addr + } + } + + updateExisting := func(incoming ma.Multiaddr) *pb.AddrBookRecord_AddrEntry { + existingEntry := addrsMap[incoming.String()] + if existingEntry == nil { + return nil + } + + switch mode { + case ttlOverride: + existingEntry.Ttl = int64(ttl) + existingEntry.Expiry = newExp + case ttlExtend: + if int64(ttl) > existingEntry.Ttl { + existingEntry.Ttl = int64(ttl) } + if newExp > existingEntry.Expiry { + existingEntry.Expiry = newExp + } + default: + panic("BUG: unimplemented ttl mode") } - return nil + return existingEntry } var entries []*pb.AddrBookRecord_AddrEntry for _, incoming := range addrs { - existingEntry := updateExisting(pr.Addrs, incoming) + existingEntry := updateExisting(incoming) if existingEntry == nil { // if signed { From 517a48433f8f2750111ceba8dfb6631d52fd6379 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 20 Jul 2021 15:21:36 -0700 Subject: [PATCH 2/2] chore: cleanup addr book patch 1. Remove duplicate import. 2. Lift empty check to simplify code. 3. Use a more efficient map key (go will optimize this to not allocate). --- pstoreds/addr_book.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pstoreds/addr_book.go b/pstoreds/addr_book.go index f57c9a0..e7c1e0a 100644 --- a/pstoreds/addr_book.go +++ b/pstoreds/addr_book.go @@ -7,8 +7,6 @@ import ( "sync" "time" - "github.com/libp2p/go-libp2p-core/record" - ds "github.com/ipfs/go-datastore" "github.com/ipfs/go-datastore/query" logging "github.com/ipfs/go-log" @@ -454,6 +452,10 @@ func (ab *dsAddrBook) ClearAddrs(p peer.ID) { } func (ab *dsAddrBook) setAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Duration, mode ttlWriteMode, signed bool) (err error) { + if len(addrs) == 0 { + return nil + } + pr, err := ab.loadRecord(p, true, false) if err != nil { return fmt.Errorf("failed to load peerstore entry for peer %v while setting addrs, err: %v", p, err) @@ -468,16 +470,13 @@ func (ab *dsAddrBook) setAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Duratio // } newExp := time.Now().Add(ttl).Unix() - var addrsMap map[string]*pb.AddrBookRecord_AddrEntry - if len(addrs) > 0 { - addrsMap = make(map[string]*pb.AddrBookRecord_AddrEntry, len(pr.Addrs)) - for _, addr := range pr.Addrs { - addrsMap[addr.Addr.String()] = addr - } + addrsMap := make(map[string]*pb.AddrBookRecord_AddrEntry, len(pr.Addrs)) + for _, addr := range pr.Addrs { + addrsMap[string(addr.Addr.Bytes())] = addr } updateExisting := func(incoming ma.Multiaddr) *pb.AddrBookRecord_AddrEntry { - existingEntry := addrsMap[incoming.String()] + existingEntry := addrsMap[string(incoming.Bytes())] if existingEntry == nil { return nil }