Merge pull request #10713 from peppy/efficient-user-retrieval

Add user caching / lookup component and consume in the currently playing display

osu.Game.Tests/Visual/Online/TestSceneCurrentlyPlayingDisplay.cs

@@ -2,12 +2,14 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
 using osu.Framework.Testing;
-using osu.Game.Online.API;
-using osu.Game.Online.API.Requests;
+using osu.Game.Database;
 using osu.Game.Online.Spectator;
 using osu.Game.Overlays.Dashboard;
 using osu.Game.Tests.Visual.Gameplay;
@@ -22,32 +24,34 @@ namespace osu.Game.Tests.Visual.Online
 
         private CurrentlyPlayingDisplay currentlyPlaying;
 
-        private DummyAPIAccess dummyAPI => (DummyAPIAccess)API;
+        [Cached(typeof(UserLookupCache))]
+        private UserLookupCache lookupCache = new TestUserLookupCache();
+
+        private Container nestedContainer;
 
         [SetUpSteps]
         public void SetUpSteps()
         {
-            AddStep("register request handling", () => dummyAPI.HandleRequest = req =>
-            {
-                switch (req)
-                {
-                    case GetUserRequest cRequest:
-                        cRequest.TriggerSuccess(new User { Username = "peppy", Id = 2 });
-                        break;
-                }
-            });
-
             AddStep("add streaming client", () =>
             {
-                Remove(testSpectatorStreamingClient);
+                nestedContainer?.Remove(testSpectatorStreamingClient);
+                Remove(lookupCache);
 
                 Children = new Drawable[]
                 {
-                    testSpectatorStreamingClient,
-                    currentlyPlaying = new CurrentlyPlayingDisplay
+                    lookupCache,
+                    nestedContainer = new Container
                     {
                         RelativeSizeAxes = Axes.Both,
-                    }
+                        Children = new Drawable[]
+                        {
+                            testSpectatorStreamingClient,
+                            currentlyPlaying = new CurrentlyPlayingDisplay
+                            {
+                                RelativeSizeAxes = Axes.Both,
+                            }
+                        }
+                    },
                 };
             });
 
@@ -62,5 +66,11 @@ namespace osu.Game.Tests.Visual.Online
             AddStep("Remove playing user", () => testSpectatorStreamingClient.PlayingUsers.Remove(2));
             AddUntilStep("Panel no longer present", () => !currentlyPlaying.ChildrenOfType<UserGridPanel>().Any());
         }
+
+        internal class TestUserLookupCache : UserLookupCache
+        {
+            protected override Task<User> ComputeValueAsync(int lookup, CancellationToken token = default)
+                => Task.FromResult(new User { Username = "peppy", Id = 2 });
+        }
     }
 }

osu.Game/Database/UserLookupCache.cs

@@ -0,0 +1,117 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using osu.Framework.Allocation;
+using osu.Game.Online.API;
+using osu.Game.Online.API.Requests;
+using osu.Game.Users;
+
+namespace osu.Game.Database
+{
+    public class UserLookupCache : MemoryCachingComponent<int, User>
+    {
+        private readonly HashSet<int> nextTaskIDs = new HashSet<int>();
+
+        [Resolved]
+        private IAPIProvider api { get; set; }
+
+        private readonly object taskAssignmentLock = new object();
+
+        private Task<List<User>> pendingRequest;
+
+        /// <summary>
+        /// Whether <see cref="pendingRequest"/> has already grabbed its IDs.
+        /// </summary>
+        private bool pendingRequestConsumedIDs;
+
+        public Task<User> GetUserAsync(int userId, CancellationToken token = default) => GetAsync(userId, token);
+
+        protected override async Task<User> ComputeValueAsync(int lookup, CancellationToken token = default)
+        {
+            var users = await getQueryTaskForUser(lookup);
+            return users.FirstOrDefault(u => u.Id == lookup);
+        }
+
+        /// <summary>
+        /// Return the task responsible for fetching the provided user.
+        /// This may be part of a larger batch lookup to reduce web requests.
+        /// </summary>
+        /// <param name="userId">The user to lookup.</param>
+        /// <returns>The task responsible for the lookup.</returns>
+        private Task<List<User>> getQueryTaskForUser(int userId)
+        {
+            lock (taskAssignmentLock)
+            {
+                nextTaskIDs.Add(userId);
+
+                // if there's a pending request which hasn't been started yet (and is not yet full), we can wait on it.
+                if (pendingRequest != null && !pendingRequestConsumedIDs && nextTaskIDs.Count < 50)
+                    return pendingRequest;
+
+                return queueNextTask(nextLookup);
+            }
+
+            List<User> nextLookup()
+            {
+                int[] lookupItems;
+
+                lock (taskAssignmentLock)
+                {
+                    pendingRequestConsumedIDs = true;
+                    lookupItems = nextTaskIDs.ToArray();
+                    nextTaskIDs.Clear();
+
+                    if (lookupItems.Length == 0)
+                    {
+                        queueNextTask(null);
+                        return new List<User>();
+                    }
+                }
+
+                var request = new GetUsersRequest(lookupItems);
+
+                // rather than queueing, we maintain our own single-threaded request stream.
+                api.Perform(request);
+
+                return request.Result?.Users;
+            }
+        }
+
+        /// <summary>
+        /// Queues new work at the end of the current work tasks.
+        /// Ensures the provided work is eventually run.
+        /// </summary>
+        /// <param name="work">The work to run. Can be null to signify the end of available work.</param>
+        /// <returns>The task tracking this work.</returns>
+        private Task<List<User>> queueNextTask(Func<List<User>> work)
+        {
+            lock (taskAssignmentLock)
+            {
+                if (work == null)
+                {
+                    pendingRequest = null;
+                    pendingRequestConsumedIDs = false;
+                }
+                else if (pendingRequest == null)
+                {
+                    // special case for the first request ever.
+                    pendingRequest = Task.Run(work);
+                    pendingRequestConsumedIDs = false;
+                }
+                else
+                {
+                    // append the new request on to the last to be executed.
+                    pendingRequest = pendingRequest.ContinueWith(_ => work());
+                    pendingRequestConsumedIDs = false;
+                }
+
+                return pendingRequest;
+            }
+        }
+    }
+}

osu.Game/Online/API/Requests/GetTopUsersRequest.cs

@@ -0,0 +1,10 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+namespace osu.Game.Online.API.Requests
+{
+    public class GetTopUsersRequest : APIRequest<GetTopUsersResponse>
+    {
+        protected override string Target => @"rankings/osu/performance";
+    }
+}

osu.Game/Online/API/Requests/GetTopUsersResponse.cs

@@ -0,0 +1,15 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Collections.Generic;
+using Newtonsoft.Json;
+using osu.Game.Users;
+
+namespace osu.Game.Online.API.Requests
+{
+    public class GetTopUsersResponse : ResponseWithCursor
+    {
+        [JsonProperty("ranking")]
+        public List<UserStatistics> Users;
+    }
+}

osu.Game/Online/API/Requests/GetUserRankingsRequest.cs

@@ -6,7 +6,7 @@ using osu.Game.Rulesets;
 
 namespace osu.Game.Online.API.Requests
 {
-    public class GetUserRankingsRequest : GetRankingsRequest<GetUsersResponse>
+    public class GetUserRankingsRequest : GetRankingsRequest<GetTopUsersResponse>
     {
         public readonly UserRankingsType Type;
 

osu.Game/Online/API/Requests/GetUsersRequest.cs

@@ -1,10 +1,24 @@
-// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
+
 namespace osu.Game.Online.API.Requests
 {
     public class GetUsersRequest : APIRequest<GetUsersResponse>
     {
-        protected override string Target => @"rankings/osu/performance";
+        private readonly int[] userIds;
+
+        private const int max_ids_per_request = 50;
+
+        public GetUsersRequest(int[] userIds)
+        {
+            if (userIds.Length > max_ids_per_request)
+                throw new ArgumentException($"{nameof(GetUsersRequest)} calls only support up to {max_ids_per_request} IDs at once");
+
+            this.userIds = userIds;
+        }
+
+        protected override string Target => "users/?ids[]=" + string.Join("&ids[]=", userIds);
     }
 }

osu.Game/Online/API/Requests/GetUsersResponse.cs

@@ -9,7 +9,7 @@ namespace osu.Game.Online.API.Requests
 {
     public class GetUsersResponse : ResponseWithCursor
     {
-        [JsonProperty("ranking")]
-        public List<UserStatistics> Users;
+        [JsonProperty("users")]
+        public List<User> Users;
     }
 }

osu.Game/OsuGameBase.cs

@@ -61,6 +61,8 @@ namespace osu.Game
 
         protected BeatmapDifficultyCache DifficultyCache;
 
+        protected UserLookupCache UserCache;
+
         protected SkinManager SkinManager;
 
         protected RulesetStore RulesetStore;
@@ -229,6 +231,9 @@ namespace osu.Game
             dependencies.Cache(DifficultyCache = new BeatmapDifficultyCache());
             AddInternal(DifficultyCache);
 
+            dependencies.Cache(UserCache = new UserLookupCache());
+            AddInternal(UserCache);
+
             var scorePerformanceManager = new ScorePerformanceCache();
             dependencies.Cache(scorePerformanceManager);
             AddInternal(scorePerformanceManager);

osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs

@@ -8,8 +8,8 @@ using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Screens;
+using osu.Game.Database;
 using osu.Game.Online.API;
-using osu.Game.Online.API.Requests;
 using osu.Game.Online.Spectator;
 using osu.Game.Screens.Multi.Match.Components;
 using osu.Game.Screens.Play;
@@ -37,6 +37,7 @@ namespace osu.Game.Overlays.Dashboard
             {
                 RelativeSizeAxes = Axes.X,
                 AutoSizeAxes = Axes.Y,
+                Padding = new MarginPadding(10),
                 Spacing = new Vector2(10),
             };
         }
@@ -44,41 +45,52 @@ namespace osu.Game.Overlays.Dashboard
         [Resolved]
         private IAPIProvider api { get; set; }
 
+        [Resolved]
+        private UserLookupCache users { get; set; }
+
         protected override void LoadComplete()
         {
             base.LoadComplete();
 
             playingUsers.BindTo(spectatorStreaming.PlayingUsers);
-            playingUsers.BindCollectionChanged((sender, e) => Schedule(() =>
-            {
-                switch (e.Action)
-                {
-                    case NotifyCollectionChangedAction.Add:
-                        foreach (var u in e.NewItems.OfType<int>())
-                        {
-                            var request = new GetUserRequest(u);
-                            request.Success += user => Schedule(() =>
-                            {
-                                if (playingUsers.Contains(user.Id))
-                                    userFlow.Add(createUserPanel(user));
-                            });
-                            api.Queue(request);
-                        }
-
-                        break;
-
-                    case NotifyCollectionChangedAction.Remove:
-                        foreach (var u in e.OldItems.OfType<int>())
-                            userFlow.FirstOrDefault(card => card.User.Id == u)?.Expire();
-                        break;
-
-                    case NotifyCollectionChangedAction.Reset:
-                        userFlow.Clear();
-                        break;
-                }
-            }), true);
+            playingUsers.BindCollectionChanged(onUsersChanged, true);
         }
 
+        private void onUsersChanged(object sender, NotifyCollectionChangedEventArgs e) => Schedule(() =>
+        {
+            switch (e.Action)
+            {
+                case NotifyCollectionChangedAction.Add:
+                    foreach (var id in e.NewItems.OfType<int>().ToArray())
+                    {
+                        users.GetUserAsync(id).ContinueWith(u =>
+                        {
+                            if (u.Result == null) return;
+
+                            Schedule(() =>
+                            {
+                                // user may no longer be playing.
+                                if (!playingUsers.Contains(u.Result.Id))
+                                    return;
+
+                                userFlow.Add(createUserPanel(u.Result));
+                            });
+                        });
+                    }
+
+                    break;
+
+                case NotifyCollectionChangedAction.Remove:
+                    foreach (var u in e.OldItems.OfType<int>())
+                        userFlow.FirstOrDefault(card => card.User.Id == u)?.Expire();
+                    break;
+
+                case NotifyCollectionChangedAction.Reset:
+                    userFlow.Clear();
+                    break;
+            }
+        });
+
         private PlayingUserPanel createUserPanel(User user) =>
             new PlayingUserPanel(user).With(panel =>
             {

osu.Game/Overlays/DashboardOverlay.cs

@@ -131,8 +131,7 @@ namespace osu.Game.Overlays
                     break;
 
                 case DashboardOverlayTabs.CurrentlyPlaying:
-                    //todo: enable once caching logic is better
-                    //loadDisplay(new CurrentlyPlayingDisplay());
+                    loadDisplay(new CurrentlyPlayingDisplay());
                     break;
 
                 default: