From 80d7cf0b657751391f5d1db97d73d1aa826e0ce7 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Sun, 17 Mar 2024 13:21:51 -0300 Subject: [PATCH] fix: Avoid slices.Contains within tight loops Having a `slices.Contains` check within a `for` loop has a complexity of `O(n*m)`, with a worst case scenario of `O(n^2)` for when both collections have the same size. This is because `slices.Contains` internally runs just a regular loop, checking for equality element by element. Instead, this diff changes every occurence of this pattern to converting the collection we compare against to a set, and then running a faster lookup against it. --- sharedutil/sharedutil.go | 6 ++++-- ui/browsing/nowplayingpage.go | 3 ++- ui/browsing/playlistpage.go | 9 ++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/sharedutil/sharedutil.go b/sharedutil/sharedutil.go index 4e6e4e8..0759af4 100644 --- a/sharedutil/sharedutil.go +++ b/sharedutil/sharedutil.go @@ -98,8 +98,9 @@ func ReorderTracks(tracks []*mediaprovider.Track, idxToMove []int, op TrackReord case MoveToTop: topIdx := 0 botIdx := len(idxToMove) + idxToMoveSet := ToSet(idxToMove) for i, t := range tracks { - if slices.Contains(idxToMove, i) { + if _, ok := idxToMoveSet[i]; ok { newTracks[topIdx] = t topIdx++ } else { @@ -110,8 +111,9 @@ func ReorderTracks(tracks []*mediaprovider.Track, idxToMove []int, op TrackReord case MoveToBottom: topIdx := 0 botIdx := len(tracks) - len(idxToMove) + idxToMoveSet := ToSet(idxToMove) for i, t := range tracks { - if slices.Contains(idxToMove, i) { + if _, ok := idxToMoveSet[i]; ok { newTracks[botIdx] = t botIdx++ } else { diff --git a/ui/browsing/nowplayingpage.go b/ui/browsing/nowplayingpage.go index 980fffa..3ec00ab 100644 --- a/ui/browsing/nowplayingpage.go +++ b/ui/browsing/nowplayingpage.go @@ -257,9 +257,10 @@ func (a *NowPlayingPage) Tapped(*fyne.PointEvent) { } func (a *NowPlayingPage) doSetNewTrackOrder(trackIDs []string, op sharedutil.TrackReorderOp) { + trackIDSet := sharedutil.ToSet(trackIDs) idxs := make([]int, 0, len(trackIDs)) for i, tr := range a.queue { - if slices.Contains(trackIDs, tr.ID) { + if _, ok := trackIDSet[tr.ID]; ok { idxs = append(idxs, i) } } diff --git a/ui/browsing/playlistpage.go b/ui/browsing/playlistpage.go index 4d3f732..a4d6adc 100644 --- a/ui/browsing/playlistpage.go +++ b/ui/browsing/playlistpage.go @@ -3,7 +3,6 @@ package browsing import ( "fmt" "log" - "slices" "github.com/dweymouth/supersonic/backend" "github.com/dweymouth/supersonic/backend/mediaprovider" @@ -181,15 +180,15 @@ func (a *PlaylistPage) doSetNewTrackOrder(op sharedutil.TrackReorderOp) { // Since the tracklist view may be sorted in a different order than the // actual running order, we need to get the IDs of the selected tracks // from the tracklist and convert them to indices in the *original* run order - ids := a.tracklist.SelectedTrackIDs() - idxs := make([]int, 0, len(ids)) + idSet := sharedutil.ToSet(a.tracklist.SelectedTrackIDs()) + idxs := make([]int, 0, len(idSet)) for i, tr := range a.tracks { - if slices.Contains(ids, tr.ID) { + if _, ok := idSet[tr.ID]; ok { idxs = append(idxs, i) } } newTracks := sharedutil.ReorderTracks(a.tracks, idxs, op) - ids = sharedutil.TracksToIDs(newTracks) + ids := sharedutil.TracksToIDs(newTracks) if err := a.sm.Server.ReplacePlaylistTracks(a.playlistID, ids); err != nil { log.Printf("error updating playlist: %s", err.Error()) } else {