From d6da6906a4b3426f05c5bf4ffe8bab4b2bc78ce8 Mon Sep 17 00:00:00 2001 From: brandon Date: Fri, 7 Aug 2026 11:59:17 -0400 Subject: Batch people lookups when building item DTOs GetBaseItemDtos already batch fetches user data, child counts, played counts and artists before its per item loop, but AttachPeople still ran one GetPeople query per item. Rendering a page of items (for example a large playlist) fired one extra query per row. Add GetPeopleByItems to IPeopleRepository, which reads every requested item in a single query over the people mapping table and returns full PersonInfo (role, type and sort order) grouped by item id. GetBaseItemDtos prefetches this once when the People field is requested and passes it into AttachPeople, which reads from the batch instead of querying per item. The single item GetBaseItemDto path keeps its existing per item behaviour when no batch is supplied. Adds a DtoService test asserting people resolve from the batch and the per item GetPeople is never called. --- Emby.Server.Implementations/Dto/DtoService.cs | 37 +++++++++++++--- .../Library/LibraryManager.cs | 6 +++ .../Item/PeopleRepository.cs | 47 ++++++++++++++++++++ MediaBrowser.Controller/Library/ILibraryManager.cs | 7 +++ .../Persistence/IPeopleRepository.cs | 7 +++ .../Dto/DtoServiceImageInheritanceTests.cs | 50 ++++++++++++++++++++++ 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>? 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? childCountBatch = null, Dictionary? playedCountBatch = null, IReadOnlyDictionary? artistsBatch = null, - VersionResumeData? resumeData = null) + VersionResumeData? resumeData = null, + IReadOnlyDictionary>? 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? 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 /// The dto. /// The item. /// The requesting user. - private void AttachPeople(BaseItemDto dto, BaseItem item, User? user = null) + /// People fetched in batch by the caller; when null the people are queried per item. + private void AttachPeople(BaseItemDto dto, BaseItem item, User? user = null, IReadOnlyList? 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); } + /// + public IReadOnlyDictionary> GetPeopleByItems(IReadOnlyList itemIds) + { + return _peopleRepository.GetPeopleByItems(itemIds); + } + public void UpdatePeople(BaseItem item, List 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 dbProvider, I return result; } + /// + public IReadOnlyDictionary> GetPeopleByItems(IReadOnlyList 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>(); + foreach (var group in rows.GroupBy(r => r.ItemId)) + { + var people = new List(); + 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(row.PersonType, out var kind)) + { + personInfo.Type = kind; + } + + people.Add(personInfo); + } + + result[group.Key] = people; + } + + return result; + } + private IEnumerable 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 @@ -605,6 +605,13 @@ namespace MediaBrowser.Controller.Library /// A dictionary mapping each item ID to its distinct people names. Items with no matching people are omitted. IReadOnlyDictionary> GetPeopleNamesByItems(IReadOnlyList itemIds, IReadOnlyList personTypes); + /// + /// Gets the people for multiple items in a single query, keyed by item id. + /// + /// The item IDs. + /// A dictionary mapping each item ID to its people. Items with no people are omitted. + IReadOnlyDictionary> GetPeopleByItems(IReadOnlyList itemIds); + /// /// Queries the items. /// 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 /// The person types to include (e.g. "Actor", "Director"). /// A dictionary mapping each item ID to its distinct people names, ordered by cast list order. Items with no matching people are omitted. IReadOnlyDictionary> GetPeopleNamesByItems(IReadOnlyList itemIds, IReadOnlyList personTypes); + + /// + /// Gets the people for multiple items in a single query, keyed by item id. + /// + /// The item IDs to get people for. + /// 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. + IReadOnlyDictionary> GetPeopleByItems(IReadOnlyList 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(), It.IsAny()), 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(); + + // 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>())) + .Returns(new Dictionary> + { + [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>()), Times.Once); + libraryManager.Verify(x => x.GetPeople(It.IsAny()), Times.Never); + } + private static DtoService BuildDtoService(BaseItem displayParent) { var libraryManager = new Mock(); -- cgit v1.2.3