Merge pull request #17571 from obiwantoby/perf/batch-people-dto

Batch people lookups when building item DTOs

6 files changed, 149 insertions, 5 deletions

diff --git a/Emby.Server.Implementations/Dto/DtoService.cs b/Emby.Server.Implementations/Dto/DtoService.cs
index 71c3b24907..da0c52df5b 100644
--- a/Emby.Server.Implementations/Dto/DtoService.cs
+++ b/Emby.Server.Implementations/Dto/DtoService.cs
@@ -242,6 +242,17 @@ namespace Emby.Server.Implementations.Dto
                 artistsBatch = _libraryManager.GetArtists(artistNames.ToArray());
             }
 
+            // Batch-fetch people across all items to avoid one GetPeople query per item.
+            IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>>? peopleBatch = null;
+            if (options.ContainsField(ItemFields.People))
+            {
+                var peopleItemIds = accessibleItems.Where(i => i.SupportsPeople).Select(i => i.Id).ToList();
+                if (peopleItemIds.Count > 0)
+                {
+                    peopleBatch = _libraryManager.GetPeopleByItems(peopleItemIds);
+                }
+            }
+
             for (int index = 0; index < accessibleItems.Count; index++)
             {
                 var item = accessibleItems[index];
@@ -255,7 +266,8 @@ namespace Emby.Server.Implementations.Dto
                     childCountBatch,
                     playedCountBatch,
                     artistsBatch,
-                    resumeDataBatch?.GetValueOrDefault(item.Id));
+                    resumeDataBatch?.GetValueOrDefault(item.Id),
+                    peopleBatch);
 
                 if (item is LiveTvChannel tvChannel)
                 {
@@ -317,7 +329,8 @@ namespace Emby.Server.Implementations.Dto
             Dictionary<Guid, int>? childCountBatch = null,
             Dictionary<Guid, (int Played, int Total)>? playedCountBatch = null,
             IReadOnlyDictionary<string, MusicArtist[]>? artistsBatch = null,
-            VersionResumeData? resumeData = null)
+            VersionResumeData? resumeData = null,
+            IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>>? peopleBatch = null)
         {
             var dto = new BaseItemDto
             {
@@ -331,7 +344,15 @@ namespace Emby.Server.Implementations.Dto
 
             if (options.ContainsField(ItemFields.People))
             {
-                AttachPeople(dto, item, user);
+                IReadOnlyList<PersonInfo>? prefetchedPeople = null;
+                if (peopleBatch is not null)
+                {
+                    // The batch omits items with no people, so a miss means "no people",
+                    // not "not fetched". Use an empty list to skip the per-item query.
+                    prefetchedPeople = peopleBatch.GetValueOrDefault(item.Id) ?? [];
+                }
+
+                AttachPeople(dto, item, user, prefetchedPeople);
             }
 
             if (options.ContainsField(ItemFields.PrimaryImageAspectRatio))
@@ -742,12 +763,18 @@ namespace Emby.Server.Implementations.Dto
         /// <param name="dto">The dto.</param>
         /// <param name="item">The item.</param>
         /// <param name="user">The requesting user.</param>
-        private void AttachPeople(BaseItemDto dto, BaseItem item, User? user = null)
+        /// <param name="prefetchedPeople">People fetched in batch by the caller; when null the people are queried per item.</param>
+        private void AttachPeople(BaseItemDto dto, BaseItem item, User? user = null, IReadOnlyList<PersonInfo>? prefetchedPeople = null)
         {
+            // When rendering a page of items the caller batch-fetches people for every item up
+            // front and passes them in, avoiding one GetPeople query per item. Fall back to the
+            // per-item query for the single item path where no batch is available.
+            var source = prefetchedPeople ?? _libraryManager.GetPeople(item);
+
             // Ordering by person type to ensure actors and artists are at the front.
             // This is taking advantage of the fact that they both begin with A
             // This should be improved in the future
-            var people = _libraryManager.GetPeople(item).OrderBy(i => i.SortOrder ?? int.MaxValue)
+            var people = source.OrderBy(i => i.SortOrder ?? int.MaxValue)
                 .ThenBy(i =>
                 {
                     if (i.IsType(PersonKind.Actor))
diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 5db3b80386..19371f68d7 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -3537,6 +3537,12 @@ namespace Emby.Server.Implementations.Library
             return _peopleRepository.GetPeopleNamesByItems(itemIds, personTypes);
         }
 
+        /// <inheritdoc/>
+        public IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>> GetPeopleByItems(IReadOnlyList<Guid> itemIds)
+        {
+            return _peopleRepository.GetPeopleByItems(itemIds);
+        }
+
         public void UpdatePeople(BaseItem item, List<PersonInfo> people)
         {
             UpdatePeopleAsync(item, people, CancellationToken.None).GetAwaiter().GetResult();
diff --git a/Jellyfin.Server.Implementations/Item/PeopleRepository.cs b/Jellyfin.Server.Implementations/Item/PeopleRepository.cs
index 05c8bffd66..a592d0e6e2 100644
--- a/Jellyfin.Server.Implementations/Item/PeopleRepository.cs
+++ b/Jellyfin.Server.Implementations/Item/PeopleRepository.cs
@@ -236,6 +236,53 @@ public class PeopleRepository(IDbContextFactory<JellyfinDbContext> dbProvider, I
         return result;
     }
 
+    /// <inheritdoc/>
+    public IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>> GetPeopleByItems(IReadOnlyList<Guid> itemIds)
+    {
+        using var context = _dbProvider.CreateDbContext();
+        var rows = context.PeopleBaseItemMap
+            .AsNoTracking()
+            .Where(m => itemIds.Contains(m.ItemId))
+            .OrderBy(m => m.ListOrder)
+            .Select(m => new
+            {
+                m.ItemId,
+                m.Role,
+                m.SortOrder,
+                m.People.Id,
+                m.People.Name,
+                m.People.PersonType
+            })
+            .ToList();
+
+        var result = new Dictionary<Guid, IReadOnlyList<PersonInfo>>();
+        foreach (var group in rows.GroupBy(r => r.ItemId))
+        {
+            var people = new List<PersonInfo>();
+            foreach (var row in group)
+            {
+                var personInfo = new PersonInfo
+                {
+                    ItemId = row.ItemId,
+                    Id = row.Id,
+                    Name = row.Name,
+                    Role = row.Role,
+                    SortOrder = row.SortOrder
+                };
+                if (Enum.TryParse<PersonKind>(row.PersonType, out var kind))
+                {
+                    personInfo.Type = kind;
+                }
+
+                people.Add(personInfo);
+            }
+
+            result[group.Key] = people;
+        }
+
+        return result;
+    }
+
     private IEnumerable<PersonInfo> MapCredits(People people)
     {
         var mappings = people.BaseItems;
diff --git a/MediaBrowser.Controller/Library/ILibraryManager.cs b/MediaBrowser.Controller/Library/ILibraryManager.cs
index 6d85a1e401..5eae6e103f 100644
--- a/MediaBrowser.Controller/Library/ILibraryManager.cs
+++ b/MediaBrowser.Controller/Library/ILibraryManager.cs
@@ -606,6 +606,13 @@ namespace MediaBrowser.Controller.Library
         IReadOnlyDictionary<Guid, IReadOnlyList<string>> GetPeopleNamesByItems(IReadOnlyList<Guid> itemIds, IReadOnlyList<string> personTypes);
 
         /// <summary>
+        /// Gets the people for multiple items in a single query, keyed by item id.
+        /// </summary>
+        /// <param name="itemIds">The item IDs.</param>
+        /// <returns>A dictionary mapping each item ID to its people. Items with no people are omitted.</returns>
+        IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>> GetPeopleByItems(IReadOnlyList<Guid> itemIds);
+
+        /// <summary>
         /// Queries the items.
         /// </summary>
         /// <param name="query">The query.</param>
diff --git a/MediaBrowser.Controller/Persistence/IPeopleRepository.cs b/MediaBrowser.Controller/Persistence/IPeopleRepository.cs
index e2833dc722..9811241d31 100644
--- a/MediaBrowser.Controller/Persistence/IPeopleRepository.cs
+++ b/MediaBrowser.Controller/Persistence/IPeopleRepository.cs
@@ -40,4 +40,11 @@ public interface IPeopleRepository
     /// <param name="personTypes">The person types to include (e.g. "Actor", "Director").</param>
     /// <returns>A dictionary mapping each item ID to its distinct people names, ordered by cast list order. Items with no matching people are omitted.</returns>
     IReadOnlyDictionary<Guid, IReadOnlyList<string>> GetPeopleNamesByItems(IReadOnlyList<Guid> itemIds, IReadOnlyList<string> personTypes);
+
+    /// <summary>
+    /// Gets the people for multiple items in a single query, keyed by item id.
+    /// </summary>
+    /// <param name="itemIds">The item IDs to get people for.</param>
+    /// <returns>A dictionary mapping each item ID to its people (with role, type and sort order), ordered by cast list order. Items with no people are omitted.</returns>
+    IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>> GetPeopleByItems(IReadOnlyList<Guid> itemIds);
 }
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs
index 6b6240e116..d18f8c6cff 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs
@@ -14,6 +14,7 @@ using MediaBrowser.Controller.LiveTv;
 using MediaBrowser.Controller.Providers;
 using MediaBrowser.Controller.Trickplay;
 using MediaBrowser.Model.Entities;
+using MediaBrowser.Model.Querying;
 using Moq;
 using Xunit;
 
@@ -155,6 +156,55 @@ public class DtoServiceImageInheritanceTests
         libraryManager.Verify(x => x.GetArtist(It.IsAny<string>(), It.IsAny<DtoOptions>()), Times.Never);
     }
 
+    [Fact]
+    public void GetBaseItemDtos_Items_ResolvePeopleFromBatch_WithoutPerItemLookup()
+    {
+        static MusicAlbum MakeAlbum() => new MusicAlbum
+        {
+            Id = Guid.NewGuid(),
+            Name = "Album",
+            ImageInfos = []
+        };
+
+        var albumOne = MakeAlbum();
+        var albumTwo = MakeAlbum();
+
+        var libraryManager = new Mock<ILibraryManager>();
+
+        // DtoService resolves people for every item in ONE batch (GetPeopleByItems) before the
+        // per-item loop. A regression to the per-item path would call GetPeople(BaseItem) once per
+        // item (the N+1); it is intentionally left unset so such a regression fails here.
+        libraryManager
+            .Setup(x => x.GetPeopleByItems(It.IsAny<IReadOnlyList<Guid>>()))
+            .Returns(new Dictionary<Guid, IReadOnlyList<PersonInfo>>
+            {
+                [albumOne.Id] = [new PersonInfo { ItemId = albumOne.Id, Name = "Some Actor", Type = PersonKind.Actor }],
+                [albumTwo.Id] = [new PersonInfo { ItemId = albumTwo.Id, Name = "Some Actor", Type = PersonKind.Actor }]
+            });
+
+        // AttachPeople still resolves each distinct name to its Person entity to attach images.
+        libraryManager
+            .Setup(x => x.GetPerson("Some Actor"))
+            .Returns(new Person { Id = Guid.NewGuid(), Name = "Some Actor" });
+
+        var dtoService = BuildDtoService(libraryManager);
+
+        var options = new DtoOptions(false) { Fields = [ItemFields.People] };
+        var dtos = dtoService.GetBaseItemDtos([albumOne, albumTwo], options);
+
+        Assert.Equal(2, dtos.Count);
+        foreach (var dto in dtos)
+        {
+            Assert.NotNull(dto.People);
+            Assert.Single(dto.People);
+            Assert.Equal("Some Actor", dto.People[0].Name);
+        }
+
+        // People are batched once for the whole set, never once per item.
+        libraryManager.Verify(x => x.GetPeopleByItems(It.IsAny<IReadOnlyList<Guid>>()), Times.Once);
+        libraryManager.Verify(x => x.GetPeople(It.IsAny<BaseItem>()), Times.Never);
+    }
+
     private static DtoService BuildDtoService(BaseItem displayParent)
     {
         var libraryManager = new Mock<ILibraryManager>();