Skip to content

Commit 612149b

Browse files
authored
Improve loading paginated items with parallelism (#922)
This PR updates `Client::all_paging_items` API to support parallelism, i.e. making multiple API requests in batch Benchmark loading `GetUserTopTracks` with 3k5 songs takes 4600ms, a big playlist with 1k7 songs takes 3600ms Resolves #732 Resolves #502 Resolves #782 Resolves #840
1 parent 242202a commit 612149b

File tree

3 files changed

+118
-106
lines changed

3 files changed

+118
-106
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

spotify_player/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ fuzzy-matcher = { version = "0.3.7", optional = true }
5858
html-escape = "0.2.13"
5959
rustls = { version = "0.23.35", default-features = false, features = ["ring"] }
6060
unicode-bidi = "0.3.18"
61+
futures = "0.3.31"
6162

6263
[target.'cfg(any(target_os = "windows", target_os = "macos"))'.dependencies.winit]
6364
version = "0.30.12"

spotify_player/src/client/mod.rs

Lines changed: 116 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@ impl Deref for AppClient {
6363
}
6464
}
6565

66-
fn market_query() -> Query<'static> {
67-
Query::from([("market", "from_token")])
68-
}
69-
7066
impl AppClient {
7167
/// Construct a new client
7268
pub async fn new() -> Result<Self> {
@@ -826,14 +822,13 @@ impl AppClient {
826822

827823
/// Get the saved (liked) tracks of the current user
828824
pub async fn current_user_saved_tracks(&self) -> Result<Vec<Track>> {
829-
let first_page = self
830-
.current_user_saved_tracks_manual(
831-
Some(rspotify::model::Market::FromToken),
832-
Some(50),
833-
None,
825+
let tracks = self
826+
.all_paging_items::<rspotify::model::SavedTrack>(
827+
&format!("{SPOTIFY_API_ENDPOINT}/me/tracks"),
828+
0, // we don't know the total number of saved tracks beforehand
834829
)
835830
.await?;
836-
let tracks = self.all_paging_items(first_page, &market_query()).await?;
831+
837832
Ok(tracks
838833
.into_iter()
839834
.filter_map(|t| Track::try_from_full_track(t.track))
@@ -860,11 +855,13 @@ impl AppClient {
860855

861856
/// Get the top tracks of the current user
862857
pub async fn current_user_top_tracks(&self) -> Result<Vec<Track>> {
863-
let first_page = self
864-
.current_user_top_tracks_manual(None, Some(50), None)
858+
let tracks = self
859+
.all_paging_items::<rspotify::model::FullTrack>(
860+
&format!("{SPOTIFY_API_ENDPOINT}/me/top/tracks"),
861+
0, // we don't know the total number of top tracks beforehand
862+
)
865863
.await?;
866864

867-
let tracks = self.all_paging_items(first_page, &Query::new()).await?;
868865
Ok(tracks
869866
.into_iter()
870867
.filter_map(Track::try_from_full_track)
@@ -873,10 +870,12 @@ impl AppClient {
873870

874871
/// Get all playlists of the current user
875872
pub async fn current_user_playlists(&self) -> Result<Vec<Playlist>> {
876-
let first_page = self.current_user_playlists_manual(Some(50), None).await?;
877-
878-
// Fetch all pages of playlists
879-
let playlists = self.all_paging_items(first_page, &Query::new()).await?;
873+
let playlists = self
874+
.all_paging_items::<rspotify::model::SimplifiedPlaylist>(
875+
&format!("{SPOTIFY_API_ENDPOINT}/me/playlists"),
876+
0, // we don't know the total number of playlists beforehand
877+
)
878+
.await?;
880879

881880
Ok(playlists
882881
.into_iter()
@@ -910,62 +909,44 @@ impl AppClient {
910909

911910
/// Get all saved albums of the current user
912911
pub async fn current_user_saved_albums(&self) -> Result<Vec<Album>> {
913-
let first_page = self
914-
.current_user_saved_albums_manual(
915-
Some(rspotify::model::Market::FromToken),
916-
Some(50),
917-
None,
912+
let albums = self
913+
.all_paging_items::<rspotify::model::SavedAlbum>(
914+
&format!("{SPOTIFY_API_ENDPOINT}/me/albums"),
915+
0, // we don't know the total number of saved albums beforehand
918916
)
919917
.await?;
920918

921-
let albums = self.all_paging_items(first_page, &Query::new()).await?;
922-
923919
// Converts `rspotify::model::SavedAlbum` into `state::Album`
924920
Ok(albums.into_iter().map(Album::from).collect())
925921
}
926922

927923
/// Get all saved shows of the current user
928924
pub async fn current_user_saved_shows(&self) -> Result<Vec<Show>> {
929-
let first_page = self.get_saved_show_manual(Some(50), None).await?;
930-
let shows = self.all_paging_items(first_page, &Query::new()).await?;
925+
let shows = self
926+
.all_paging_items::<rspotify::model::Show>(
927+
&format!("{SPOTIFY_API_ENDPOINT}/me/shows"),
928+
0, // we don't know the total number of saved shows beforehand
929+
)
930+
.await?;
931+
931932
Ok(shows.into_iter().map(|s| s.show.into()).collect())
932933
}
933934

934935
/// Get all albums of an artist
935936
pub async fn artist_albums(&self, artist_id: ArtistId<'_>) -> Result<Vec<Album>> {
936-
let payload = market_query();
937-
938-
let mut singles = {
939-
let first_page = self
940-
.artist_albums_manual(
941-
artist_id.as_ref(),
942-
Some(rspotify::model::AlbumType::Single),
943-
Some(rspotify::model::Market::FromToken),
944-
Some(50),
945-
None,
946-
)
947-
.await?;
948-
self.all_paging_items(first_page, &payload).await
949-
}?;
950-
let mut albums = {
951-
let first_page = self
952-
.artist_albums_manual(
953-
artist_id.as_ref(),
954-
Some(rspotify::model::AlbumType::Album),
955-
Some(rspotify::model::Market::FromToken),
956-
Some(50),
957-
None,
958-
)
959-
.await?;
960-
self.all_paging_items(first_page, &payload).await
961-
}?;
962-
albums.append(&mut singles);
963-
964-
// converts `rspotify::model::SimplifiedAlbum` into `state::Album`
965-
let albums = albums
937+
let albums = self
938+
.all_paging_items::<rspotify::model::SimplifiedAlbum>(
939+
&format!(
940+
"{SPOTIFY_API_ENDPOINT}/artists/{}/albums?include_groups=album,single",
941+
artist_id.id()
942+
),
943+
0, // we don't know the total number of artist albums beforehand
944+
)
945+
.await?
966946
.into_iter()
967947
.filter_map(Album::try_from_simplified_album)
968948
.collect();
949+
969950
Ok(AppClient::process_artist_albums(albums))
970951
}
971952

@@ -1388,13 +1369,21 @@ impl AppClient {
13881369
tracing::info!("Get playlist context: {}", playlist_uri);
13891370

13901371
let playlist = self
1391-
.playlist(playlist_id, None, Some(rspotify::model::Market::FromToken))
1372+
.playlist(
1373+
playlist_id.clone(),
1374+
None,
1375+
Some(rspotify::model::Market::FromToken),
1376+
)
13921377
.await?;
13931378

1394-
// get the playlist's tracks
1395-
let first_page = playlist.tracks.clone();
13961379
let tracks = self
1397-
.all_paging_items(first_page, &market_query())
1380+
.all_paging_items(
1381+
&format!(
1382+
"{SPOTIFY_API_ENDPOINT}/playlists/{}/tracks",
1383+
playlist_id.id(),
1384+
),
1385+
playlist.tracks.total as usize,
1386+
)
13981387
.await?
13991388
.into_iter()
14001389
.filter_map(Track::try_from_playlist_item)
@@ -1412,16 +1401,20 @@ impl AppClient {
14121401
tracing::info!("Get album context: {}", album_uri);
14131402

14141403
let album = self
1415-
.album(album_id, Some(rspotify::model::Market::FromToken))
1404+
.album(album_id.clone(), Some(rspotify::model::Market::FromToken))
14161405
.await?;
1417-
let first_page = album.tracks.clone();
1406+
1407+
let total_tracks = album.tracks.total as usize;
14181408

14191409
// converts `rspotify::model::FullAlbum` into `state::Album`
14201410
let album: Album = album.into();
14211411

14221412
// get the album's tracks
14231413
let tracks = self
1424-
.all_paging_items(first_page, &Query::new())
1414+
.all_paging_items(
1415+
&format!("{SPOTIFY_API_ENDPOINT}/albums/{}/tracks", album_id.id()),
1416+
total_tracks,
1417+
)
14251418
.await?
14261419
.into_iter()
14271420
.filter_map(|t| {
@@ -1454,8 +1447,7 @@ impl AppClient {
14541447
let top_tracks = self
14551448
.artist_top_tracks(artist_id.as_ref(), Some(rspotify::model::Market::FromToken))
14561449
.await
1457-
.context("get artist's top tracks")?;
1458-
let top_tracks = top_tracks
1450+
.context("get artist's top tracks")?
14591451
.into_iter()
14601452
.filter_map(Track::try_from_full_track)
14611453
.collect::<Vec<_>>();
@@ -1465,8 +1457,7 @@ impl AppClient {
14651457
.artist_related_artists(artist_id.as_ref())
14661458
.await
14671459
.ok()
1468-
.unwrap_or_default();
1469-
let related_artists = related_artists
1460+
.unwrap_or_default()
14701461
.into_iter()
14711462
.map(std::convert::Into::into)
14721463
.collect::<Vec<_>>();
@@ -1489,33 +1480,22 @@ impl AppClient {
14891480
let show_uri = show_id.uri();
14901481
tracing::info!("Get show context: {}", show_uri);
14911482

1492-
let show = self.get_a_show(show_id, None).await?;
1493-
let first_page = show.episodes.clone();
1494-
1495-
// Copy first_page but use Page<Option<SimplifiedEpisode>> instead of Page<SimplifiedEpisode>
1496-
// This is a temporary fix for https://github.com/aome510/spotify-player/issues/663
1497-
let first_page = rspotify::model::Page {
1498-
items: first_page.items.into_iter().map(Some).collect(),
1499-
href: first_page.href,
1500-
limit: first_page.limit,
1501-
next: first_page.next,
1502-
offset: first_page.offset,
1503-
previous: first_page.previous,
1504-
total: first_page.total,
1505-
};
1506-
1507-
// converts `rspotify::model::FullShow` into `state::Show`
1508-
let show: Show = show.into();
1483+
let show = self.get_a_show(show_id.clone(), None).await?;
15091484

15101485
// get the show's episodes
15111486
let episodes = self
1512-
.all_paging_items(first_page, &Query::new())
1487+
.all_paging_items::<rspotify::model::SimplifiedEpisode>(
1488+
&format!("{SPOTIFY_API_ENDPOINT}/shows/{}/episodes", show_id.id()),
1489+
show.episodes.total as usize,
1490+
)
15131491
.await?
15141492
.into_iter()
1515-
.flatten()
15161493
.map(std::convert::Into::into)
15171494
.collect::<Vec<_>>();
15181495

1496+
// converts `rspotify::model::FullShow` into `state::Show`
1497+
let show: Show = show.into();
1498+
15191499
Ok(Context::Show { show, episodes })
15201500
}
15211501

@@ -1557,29 +1537,61 @@ impl AppClient {
15571537
Ok(serde_json::from_str(&text)?)
15581538
}
15591539

1560-
/// Get all paging items starting from a pagination object of the first page
1561-
async fn all_paging_items<T>(
1562-
&self,
1563-
first_page: rspotify::model::Page<T>,
1564-
payload: &Query<'_>,
1565-
) -> Result<Vec<T>>
1540+
async fn all_paging_items<T>(&self, base_url: &str, mut count: usize) -> Result<Vec<T>>
15661541
where
1567-
T: serde::de::DeserializeOwned,
1542+
T: serde::de::DeserializeOwned + std::fmt::Debug,
15681543
{
1569-
let mut items = first_page.items;
1570-
let mut maybe_next = first_page.next;
1544+
const PAGE_LIMIT: usize = 50;
1545+
const MAX_PARALLEL: usize = 8;
15711546

1572-
while let Some(url) = maybe_next {
1573-
let mut next_page = self
1574-
.http_get::<rspotify::model::Page<T>>(&url, payload)
1575-
.await?;
1576-
if next_page.items.is_empty() {
1547+
let mut all_items = Vec::new();
1548+
let mut offset = 0;
1549+
1550+
// if count is 0 (i.e., unknown), set it to usize::MAX to fetch until no more items
1551+
if count == 0 {
1552+
count = usize::MAX;
1553+
}
1554+
1555+
while offset < count {
1556+
let n_jobs = std::cmp::min(MAX_PARALLEL, (count - offset).div_ceil(PAGE_LIMIT));
1557+
1558+
let mut futures = Vec::with_capacity(n_jobs);
1559+
1560+
for i in 0..n_jobs {
1561+
let current_offset = offset + i * PAGE_LIMIT;
1562+
let limit_str = PAGE_LIMIT.to_string();
1563+
let offset_str = current_offset.to_string();
1564+
1565+
futures.push(async move {
1566+
let params = Query::from([
1567+
("market", "from_token"),
1568+
("limit", &limit_str),
1569+
("offset", &offset_str),
1570+
]);
1571+
self.http_get::<rspotify::model::Page<T>>(base_url, &params)
1572+
.await
1573+
});
1574+
}
1575+
1576+
let results = futures::future::try_join_all(futures).await?;
1577+
1578+
let mut found_empty = false;
1579+
for mut page in results {
1580+
if page.items.is_empty() {
1581+
found_empty = true;
1582+
break;
1583+
}
1584+
all_items.append(&mut page.items);
1585+
}
1586+
1587+
if found_empty {
15771588
break;
15781589
}
1579-
items.append(&mut next_page.items);
1580-
maybe_next = next_page.next;
1590+
1591+
offset += n_jobs * PAGE_LIMIT;
15811592
}
1582-
Ok(items)
1593+
1594+
Ok(all_items)
15831595
}
15841596

15851597
/// Get all cursor-based paging items starting from a pagination object of the first page
@@ -1963,9 +1975,7 @@ impl AppClient {
19631975
/// Process a list of albums, which includes
19641976
/// - sort albums by the release date
19651977
/// - sort albums by the type if `sort_artist_albums_by_type` config is enabled
1966-
fn process_artist_albums(albums: Vec<Album>) -> Vec<Album> {
1967-
let mut albums = albums.into_iter().collect::<Vec<_>>();
1968-
1978+
fn process_artist_albums(mut albums: Vec<Album>) -> Vec<Album> {
19691979
albums.sort_by(|x, y| y.release_date.partial_cmp(&x.release_date).unwrap());
19701980

19711981
if config::get_config().app_config.sort_artist_albums_by_type {

0 commit comments

Comments
 (0)