Merge pull request #17571 from obiwantoby/perf/batch-people-dto
Batch people lookups when building item DTOs
This commit is contained in:
@@ -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))
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -605,6 +605,13 @@ namespace MediaBrowser.Controller.Library
|
||||
/// <returns>A dictionary mapping each item ID to its distinct people names. 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.</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>
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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>();
|
||||
|
||||
Reference in New Issue
Block a user