Merge pull request #17576 from obiwantoby/perf/batch-mediasourcecount-dto
Bugfix: #17547 | Batching MediaSourceCount into one call
This commit is contained in:
@@ -253,6 +253,18 @@ namespace Emby.Server.Implementations.Dto
|
||||
}
|
||||
}
|
||||
|
||||
// Batch-detect which videos own alternate versions to avoid the per-item alternate-version
|
||||
// queries in MediaSourceCount. Videos absent from this set have a single media source.
|
||||
IReadOnlySet<Guid>? alternateVersionItemIds = null;
|
||||
if (options.ContainsField(ItemFields.MediaSourceCount))
|
||||
{
|
||||
var versionItemIds = accessibleItems.OfType<Video>().Select(i => i.Id).ToList();
|
||||
if (versionItemIds.Count > 0)
|
||||
{
|
||||
alternateVersionItemIds = _libraryManager.GetItemIdsWithAlternateVersions(versionItemIds);
|
||||
}
|
||||
}
|
||||
|
||||
for (int index = 0; index < accessibleItems.Count; index++)
|
||||
{
|
||||
var item = accessibleItems[index];
|
||||
@@ -267,7 +279,8 @@ namespace Emby.Server.Implementations.Dto
|
||||
playedCountBatch,
|
||||
artistsBatch,
|
||||
resumeDataBatch?.GetValueOrDefault(item.Id),
|
||||
peopleBatch);
|
||||
peopleBatch,
|
||||
alternateVersionItemIds);
|
||||
|
||||
if (item is LiveTvChannel tvChannel)
|
||||
{
|
||||
@@ -330,7 +343,8 @@ namespace Emby.Server.Implementations.Dto
|
||||
Dictionary<Guid, (int Played, int Total)>? playedCountBatch = null,
|
||||
IReadOnlyDictionary<string, MusicArtist[]>? artistsBatch = null,
|
||||
VersionResumeData? resumeData = null,
|
||||
IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>>? peopleBatch = null)
|
||||
IReadOnlyDictionary<Guid, IReadOnlyList<PersonInfo>>? peopleBatch = null,
|
||||
IReadOnlySet<Guid>? alternateVersionItemIds = null)
|
||||
{
|
||||
var dto = new BaseItemDto
|
||||
{
|
||||
@@ -399,7 +413,7 @@ namespace Emby.Server.Implementations.Dto
|
||||
AttachStudios(dto, item);
|
||||
}
|
||||
|
||||
AttachBasicFields(dto, item, owner, options, artistsBatch, user);
|
||||
AttachBasicFields(dto, item, owner, options, artistsBatch, user, alternateVersionItemIds);
|
||||
|
||||
if (options.ContainsField(ItemFields.CanDelete))
|
||||
{
|
||||
@@ -984,7 +998,8 @@ namespace Emby.Server.Implementations.Dto
|
||||
/// <param name="options">The options.</param>
|
||||
/// <param name="artistsBatch">Optional pre-fetched artist lookup shared across a batch of items.</param>
|
||||
/// <param name="user">The user, for per-user values such as the accessible media source count.</param>
|
||||
private void AttachBasicFields(BaseItemDto dto, BaseItem item, BaseItem? owner, DtoOptions options, IReadOnlyDictionary<string, MusicArtist[]>? artistsBatch = null, User? user = null)
|
||||
/// <param name="alternateVersionItemIds">Optional pre-fetched set of item IDs that own alternate versions, shared across a batch of items.</param>
|
||||
private void AttachBasicFields(BaseItemDto dto, BaseItem item, BaseItem? owner, DtoOptions options, IReadOnlyDictionary<string, MusicArtist[]>? artistsBatch = null, User? user = null, IReadOnlySet<Guid>? alternateVersionItemIds = null)
|
||||
{
|
||||
if (options.ContainsField(ItemFields.DateCreated))
|
||||
{
|
||||
@@ -1298,15 +1313,27 @@ namespace Emby.Server.Implementations.Dto
|
||||
|
||||
if (options.ContainsField(ItemFields.MediaSourceCount))
|
||||
{
|
||||
// Match the per-user filtering of the media sources: versions the user cannot
|
||||
// access are not selectable, so they must not count towards the badge either.
|
||||
var mediaSourceCount = user is null
|
||||
|| (!video.PrimaryVersionId.HasValue && video.LinkedAlternateVersions.Length == 0 && !video.HasLocalAlternateVersions)
|
||||
? video.MediaSourceCount
|
||||
: video.GetAllVersions().Count(v => v.Id.Equals(video.Id) || v.IsVisibleStandalone(user));
|
||||
if (mediaSourceCount != 1)
|
||||
// A video with no primary version and no alternate versions always has a single
|
||||
// media source. Only compute the count for videos that might have more: a primary
|
||||
// version, or membership in the batch's set of items that own alternate versions.
|
||||
// Without the batch we can't rule it out, so fall back to computing (the single-item
|
||||
// path). Everything else is the common case and keeps the default count of one.
|
||||
var mayHaveAlternateVersions = alternateVersionItemIds is null
|
||||
|| video.PrimaryVersionId.HasValue
|
||||
|| alternateVersionItemIds.Contains(video.Id);
|
||||
|
||||
if (mayHaveAlternateVersions)
|
||||
{
|
||||
dto.MediaSourceCount = mediaSourceCount;
|
||||
// Match the per-user filtering of the media sources: versions the user cannot
|
||||
// access are not selectable, so they must not count towards the badge either.
|
||||
var mediaSourceCount = user is null
|
||||
|| (!video.PrimaryVersionId.HasValue && video.LinkedAlternateVersions.Length == 0 && !video.HasLocalAlternateVersions)
|
||||
? video.MediaSourceCount
|
||||
: video.GetAllVersions().Count(v => v.Id.Equals(video.Id) || v.IsVisibleStandalone(user));
|
||||
if (mediaSourceCount != 1)
|
||||
{
|
||||
dto.MediaSourceCount = mediaSourceCount;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2234,6 +2234,12 @@ namespace Emby.Server.Implementations.Library
|
||||
return [];
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds)
|
||||
{
|
||||
return _linkedChildrenService.GetItemIdsWithAlternateVersions(itemIds);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public void UpsertLinkedChild(Guid parentId, Guid childId, MediaBrowser.Controller.Entities.LinkedChildType childType)
|
||||
{
|
||||
|
||||
@@ -59,6 +59,25 @@ public class LinkedChildrenService : ILinkedChildrenService
|
||||
.ToArray();
|
||||
}
|
||||
|
||||
/// <inheritdoc/>
|
||||
public IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds)
|
||||
{
|
||||
if (itemIds.Count == 0)
|
||||
{
|
||||
return new HashSet<Guid>();
|
||||
}
|
||||
|
||||
using var dbContext = _dbProvider.CreateDbContext();
|
||||
|
||||
return dbContext.LinkedChildren
|
||||
.Where(lc => lc.ChildType == DbLinkedChildType.LocalAlternateVersion
|
||||
|| lc.ChildType == DbLinkedChildType.LinkedAlternateVersion)
|
||||
.WhereOneOrMany(itemIds as IList<Guid> ?? itemIds.ToList(), lc => lc.ParentId)
|
||||
.Select(lc => lc.ParentId)
|
||||
.Distinct()
|
||||
.ToHashSet();
|
||||
}
|
||||
|
||||
/// <inheritdoc/>
|
||||
public IReadOnlyDictionary<string, MusicArtist[]> FindArtists(IReadOnlyList<string> artistNames)
|
||||
{
|
||||
|
||||
@@ -255,6 +255,14 @@ namespace MediaBrowser.Controller.Library
|
||||
/// <returns>Enumerable of linked Video items.</returns>
|
||||
IEnumerable<Video> GetLinkedAlternateVersions(Video video);
|
||||
|
||||
/// <summary>
|
||||
/// Gets, in a single query, the subset of the supplied items that own at least one alternate
|
||||
/// version (local or linked). Items absent from the result have no alternate versions.
|
||||
/// </summary>
|
||||
/// <param name="itemIds">The item IDs to check.</param>
|
||||
/// <returns>The set of item IDs that have alternate versions.</returns>
|
||||
IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds);
|
||||
|
||||
/// <summary>
|
||||
/// Creates or updates a LinkedChild entry linking a parent to a child item.
|
||||
/// </summary>
|
||||
|
||||
@@ -19,6 +19,15 @@ public interface ILinkedChildrenService
|
||||
/// <returns>List of child item IDs.</returns>
|
||||
IReadOnlyList<Guid> GetLinkedChildrenIds(Guid parentId, int? childType = null);
|
||||
|
||||
/// <summary>
|
||||
/// Gets, in a single query, the subset of the supplied items that own at least one alternate
|
||||
/// version (local or linked). Items absent from the result have no alternate versions, so their
|
||||
/// media source count is one.
|
||||
/// </summary>
|
||||
/// <param name="itemIds">The item IDs to check.</param>
|
||||
/// <returns>The set of item IDs that have alternate versions.</returns>
|
||||
IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds);
|
||||
|
||||
/// <summary>
|
||||
/// Gets all artist matches from the database.
|
||||
/// </summary>
|
||||
|
||||
@@ -9,6 +9,7 @@ using MediaBrowser.Controller.Drawing;
|
||||
using MediaBrowser.Controller.Dto;
|
||||
using MediaBrowser.Controller.Entities;
|
||||
using MediaBrowser.Controller.Entities.Audio;
|
||||
using MediaBrowser.Controller.Entities.Movies;
|
||||
using MediaBrowser.Controller.Library;
|
||||
using MediaBrowser.Controller.LiveTv;
|
||||
using MediaBrowser.Controller.Providers;
|
||||
@@ -205,6 +206,84 @@ public class DtoServiceImageInheritanceTests
|
||||
libraryManager.Verify(x => x.GetPeople(It.IsAny<BaseItem>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetBaseItemDtos_Videos_ResolveMediaSourceCountFromBatch_WithoutPerItemLookup()
|
||||
{
|
||||
static Movie MakeMovie() => new Movie
|
||||
{
|
||||
Id = Guid.NewGuid(),
|
||||
Name = "Movie",
|
||||
ImageInfos = []
|
||||
};
|
||||
|
||||
var movieOne = MakeMovie();
|
||||
var movieTwo = MakeMovie();
|
||||
|
||||
var libraryManager = new Mock<ILibraryManager>();
|
||||
|
||||
// DtoService detects which videos own alternate versions in ONE batch
|
||||
// (GetItemIdsWithAlternateVersions) before the per-item loop. Videos absent from that set have a
|
||||
// single media source, so the per-item GetLinkedAlternateVersions/GetLocalAlternateVersionIds
|
||||
// queries (the N+1) must be skipped entirely. Here neither movie has alternate versions.
|
||||
libraryManager
|
||||
.Setup(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()))
|
||||
.Returns(new HashSet<Guid>());
|
||||
|
||||
var dtoService = BuildDtoService(libraryManager);
|
||||
|
||||
var options = new DtoOptions(false) { Fields = [ItemFields.MediaSourceCount] };
|
||||
var dtos = dtoService.GetBaseItemDtos([movieOne, movieTwo], options);
|
||||
|
||||
Assert.Equal(2, dtos.Count);
|
||||
|
||||
// A single media source is the default, so the count is left unset (the client treats null as one).
|
||||
foreach (var dto in dtos)
|
||||
{
|
||||
Assert.Null(dto.MediaSourceCount);
|
||||
}
|
||||
|
||||
// The alternate-version check is batched once for the whole set, and the per-item lookups are
|
||||
// never reached because the batch already ruled out alternate versions.
|
||||
libraryManager.Verify(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()), Times.Once);
|
||||
libraryManager.Verify(x => x.GetLinkedAlternateVersions(It.IsAny<Video>()), Times.Never);
|
||||
libraryManager.Verify(x => x.GetLocalAlternateVersionIds(It.IsAny<Video>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetBaseItemDtos_VideoInAlternateVersionBatch_ResolvesRealCount()
|
||||
{
|
||||
var movie = new Movie
|
||||
{
|
||||
Id = Guid.NewGuid(),
|
||||
Name = "Movie",
|
||||
ImageInfos = []
|
||||
};
|
||||
|
||||
var libraryManager = new Mock<ILibraryManager>();
|
||||
|
||||
// This movie IS in the batch set, so the fast path must not short-circuit it: the per-item
|
||||
// lookups still run and the count is computed exactly as it was before batching. Two linked
|
||||
// alternate versions plus the movie itself is a count of three.
|
||||
libraryManager
|
||||
.Setup(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()))
|
||||
.Returns(new HashSet<Guid> { movie.Id });
|
||||
libraryManager
|
||||
.Setup(x => x.GetLinkedAlternateVersions(It.IsAny<Video>()))
|
||||
.Returns([new Movie { Id = Guid.NewGuid() }, new Movie { Id = Guid.NewGuid() }]);
|
||||
libraryManager
|
||||
.Setup(x => x.GetLocalAlternateVersionIds(It.IsAny<Video>()))
|
||||
.Returns([]);
|
||||
|
||||
var dtoService = BuildDtoService(libraryManager);
|
||||
|
||||
var options = new DtoOptions(false) { Fields = [ItemFields.MediaSourceCount] };
|
||||
var dtos = dtoService.GetBaseItemDtos([movie], options);
|
||||
|
||||
Assert.Single(dtos);
|
||||
Assert.Equal(3, dtos[0].MediaSourceCount);
|
||||
libraryManager.Verify(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()), Times.Once);
|
||||
}
|
||||
|
||||
private static DtoService BuildDtoService(BaseItem displayParent)
|
||||
{
|
||||
var libraryManager = new Mock<ILibraryManager>();
|
||||
@@ -231,6 +310,10 @@ public class DtoServiceImageInheritanceTests
|
||||
.Setup(x => x.GetImageCacheTag(It.IsAny<BaseItem>(), It.IsAny<ItemImageInfo>()))
|
||||
.Returns<BaseItem, ItemImageInfo>((_, image) => image.Path);
|
||||
|
||||
// Video.IsActiveRecording() dereferences this static during DTO building.
|
||||
Video.RecordingsManager = recordingsManager.Object;
|
||||
BaseItem.LibraryManager = libraryManager.Object;
|
||||
|
||||
return new DtoService(
|
||||
logger.Object,
|
||||
libraryManager.Object,
|
||||
|
||||
Reference in New Issue
Block a user