From 42a2cc1747c7859c63334a7a45792e0af1410e1a Mon Sep 17 00:00:00 2001 From: cvium Date: Mon, 24 May 2021 00:30:41 +0200 Subject: Remove some unnecessary allocations --- .../Providers/DirectoryService.cs | 26 +++++++++++++++------- .../Providers/IDirectoryService.cs | 4 +++- .../Providers/MetadataResult.cs | 18 +++++++++++---- 3 files changed, 35 insertions(+), 13 deletions(-) (limited to 'MediaBrowser.Controller/Providers') diff --git a/MediaBrowser.Controller/Providers/DirectoryService.cs b/MediaBrowser.Controller/Providers/DirectoryService.cs index 291a268830..ba52907633 100644 --- a/MediaBrowser.Controller/Providers/DirectoryService.cs +++ b/MediaBrowser.Controller/Providers/DirectoryService.cs @@ -25,15 +25,16 @@ namespace MediaBrowser.Controller.Providers public FileSystemMetadata[] GetFileSystemEntries(string path) { - return _cache.GetOrAdd(path, p => _fileSystem.GetFileSystemEntries(p).ToArray()); + return _cache.GetOrAdd(path, (p, fileSystem) => fileSystem.GetFileSystemEntries(p).ToArray(), _fileSystem); } public List GetFiles(string path) { var list = new List(); var items = GetFileSystemEntries(path); - foreach (var item in items) + for (var i = 0; i < items.Length; i++) { + var item = items[i]; if (!item.IsDirectory) { list.Add(item); @@ -48,10 +49,9 @@ namespace MediaBrowser.Controller.Providers if (!_fileCache.TryGetValue(path, out var result)) { var file = _fileSystem.GetFileInfo(path); - var res = file != null && file.Exists ? file : null; - if (res != null) + if (file.Exists) { - result = res; + result = file; _fileCache.TryAdd(path, result); } } @@ -60,16 +60,26 @@ namespace MediaBrowser.Controller.Providers } public IReadOnlyList GetFilePaths(string path) - => GetFilePaths(path, false); + => GetFilePaths(path, false, false); - public IReadOnlyList GetFilePaths(string path, bool clearCache) + public IReadOnlyList GetSortedFilePaths(string path, bool clearCache) + => GetFilePaths(path, clearCache, true); + + public IReadOnlyList GetFilePaths(string path, bool clearCache, bool sort = false) { if (clearCache) { _filePathCache.TryRemove(path, out _); } - return _filePathCache.GetOrAdd(path, p => _fileSystem.GetFilePaths(p).ToList()); + var filePaths = _filePathCache.GetOrAdd(path, (p, fileSystem) => fileSystem.GetFilePaths(p).ToList(), _fileSystem); + + if (sort) + { + filePaths.Sort(); + } + + return filePaths; } } } diff --git a/MediaBrowser.Controller/Providers/IDirectoryService.cs b/MediaBrowser.Controller/Providers/IDirectoryService.cs index 9cee06a4c6..beb685b6d8 100644 --- a/MediaBrowser.Controller/Providers/IDirectoryService.cs +++ b/MediaBrowser.Controller/Providers/IDirectoryService.cs @@ -15,6 +15,8 @@ namespace MediaBrowser.Controller.Providers IReadOnlyList GetFilePaths(string path); - IReadOnlyList GetFilePaths(string path, bool clearCache); + IReadOnlyList GetSortedFilePaths(string path, bool clearCache); + + IReadOnlyList GetFilePaths(string path, bool clearCache, bool sort = false); } } diff --git a/MediaBrowser.Controller/Providers/MetadataResult.cs b/MediaBrowser.Controller/Providers/MetadataResult.cs index 8b0967a6e6..7ec1eefcd6 100644 --- a/MediaBrowser.Controller/Providers/MetadataResult.cs +++ b/MediaBrowser.Controller/Providers/MetadataResult.cs @@ -12,16 +12,26 @@ namespace MediaBrowser.Controller.Providers { public class MetadataResult { + // Images aren't always used so the allocation is a waste a lot of the time + private List _images; + private List<(string url, ImageType type)> _remoteImages; + public MetadataResult() { - Images = new List(); - RemoteImages = new List<(string url, ImageType type)>(); ResultLanguage = "en"; } - public List Images { get; set; } + public List Images + { + get => _images ??= new List(); + set => _images = value; + } - public List<(string url, ImageType type)> RemoteImages { get; set; } + public List<(string url, ImageType type)> RemoteImages + { + get => _remoteImages ??= new List<(string url, ImageType type)>(); + set => _remoteImages = value; + } public List UserDataList { get; set; } -- cgit v1.2.3 From 0835b26889e1b26d3bd00abfb076da3b6eb4424e Mon Sep 17 00:00:00 2001 From: cvium Date: Mon, 31 May 2021 13:55:54 +0200 Subject: review --- .../Data/SqliteItemRepository.cs | 97 +++++++++++++--------- .../Extensions/StringBuilderExtensions.cs | 34 ++++++++ .../Providers/DirectoryService.cs | 5 +- .../Providers/IDirectoryService.cs | 2 - .../MediaInfo/SubtitleResolver.cs | 2 +- .../Data/SqliteItemRepositoryTests.cs | 20 ----- 6 files changed, 95 insertions(+), 65 deletions(-) create mode 100644 MediaBrowser.Common/Extensions/StringBuilderExtensions.cs (limited to 'MediaBrowser.Controller/Providers') diff --git a/Emby.Server.Implementations/Data/SqliteItemRepository.cs b/Emby.Server.Implementations/Data/SqliteItemRepository.cs index 1480600cfb..15a4ab3180 100644 --- a/Emby.Server.Implementations/Data/SqliteItemRepository.cs +++ b/Emby.Server.Implementations/Data/SqliteItemRepository.cs @@ -2270,7 +2270,7 @@ namespace Emby.Server.Implementations.Data return query.IncludeItemTypes.Any(x => _seriesTypes.Contains(x)); } - private List GetFinalColumnsToSelect(InternalItemsQuery query, List columns) + private void SetFinalColumnsToSelect(InternalItemsQuery query, List columns) { foreach (var field in _allFields) { @@ -2438,8 +2438,6 @@ namespace Emby.Server.Implementations.Data columns.Add(builder.ToString()); } - - return columns; } private void BindSearchParams(InternalItemsQuery query, IStatement statement) @@ -2541,8 +2539,10 @@ namespace Emby.Server.Implementations.Data query.Limit = query.Limit.Value + 4; } - var commandTextBuilder = new StringBuilder("select ") - .AppendJoin(',', GetFinalColumnsToSelect(query, new List { "count(distinct PresentationUniqueKey)" })) + var columns = new List { "count(distinct PresentationUniqueKey)" }; + SetFinalColumnsToSelect(query, columns); + var commandTextBuilder = new StringBuilder("select ", 256) + .AppendJoin(',', columns) .Append(FromText) .Append(GetJoinUserDataText(query)); @@ -2595,8 +2595,10 @@ namespace Emby.Server.Implementations.Data query.Limit = query.Limit.Value + 4; } - var commandTextBuilder = new StringBuilder("select ") - .AppendJoin(',', GetFinalColumnsToSelect(query, _retriveItemColumns.ToList())) + var columns = _retriveItemColumns.ToList(); + SetFinalColumnsToSelect(query, columns); + var commandTextBuilder = new StringBuilder("select ", 1024) + .AppendJoin(',', columns) .Append(FromText) .Append(GetJoinUserDataText(query)); @@ -2618,13 +2620,13 @@ namespace Emby.Server.Implementations.Data if (query.Limit.HasValue || offset > 0) { commandTextBuilder.Append(" LIMIT ") - .Append((query.Limit ?? int.MaxValue).ToString(CultureInfo.InvariantCulture)); + .Append(query.Limit ?? int.MaxValue); } if (offset > 0) { commandTextBuilder.Append(" OFFSET ") - .Append(offset.ToString(CultureInfo.InvariantCulture)); + .Append(offset); } } @@ -2784,8 +2786,10 @@ namespace Emby.Server.Implementations.Data query.Limit = query.Limit.Value + 4; } - var commandTextBuilder = new StringBuilder("select ") - .AppendJoin(',', GetFinalColumnsToSelect(query, _retriveItemColumns.ToList())) + var columns = _retriveItemColumns.ToList(); + SetFinalColumnsToSelect(query, columns); + var commandTextBuilder = new StringBuilder("select ", 512) + .AppendJoin(',', columns) .Append(FromText) .Append(GetJoinUserDataText(query)); @@ -2811,13 +2815,13 @@ namespace Emby.Server.Implementations.Data if (query.Limit.HasValue || offset > 0) { commandTextBuilder.Append(" LIMIT ") - .Append((query.Limit ?? int.MaxValue).ToString(CultureInfo.InvariantCulture)); + .Append(query.Limit ?? int.MaxValue); } if (offset > 0) { commandTextBuilder.Append(" OFFSET ") - .Append(offset.ToString(CultureInfo.InvariantCulture)); + .Append(offset); } } @@ -2836,20 +2840,24 @@ namespace Emby.Server.Implementations.Data commandTextBuilder.Append(" select "); + List columnsToSelect; if (EnableGroupByPresentationUniqueKey(query)) { - commandTextBuilder.AppendJoin(',', GetFinalColumnsToSelect(query, new List { "count (distinct PresentationUniqueKey)" })); + columnsToSelect = new List { "count (distinct PresentationUniqueKey)" }; } else if (query.GroupBySeriesPresentationUniqueKey) { - commandTextBuilder.AppendJoin(',', GetFinalColumnsToSelect(query, new List { "count (distinct SeriesPresentationUniqueKey)" })); + columnsToSelect = new List { "count (distinct SeriesPresentationUniqueKey)" }; } else { - commandTextBuilder.AppendJoin(',', GetFinalColumnsToSelect(query, new List { "count (guid)" })); + columnsToSelect = new List { "count (guid)" }; } - commandTextBuilder.Append(FromText) + SetFinalColumnsToSelect(query, columnsToSelect); + + commandTextBuilder.AppendJoin(',', columnsToSelect) + .Append(FromText) .Append(GetJoinUserDataText(query)); if (!string.IsNullOrEmpty(whereText)) { @@ -3062,8 +3070,10 @@ namespace Emby.Server.Implementations.Data var now = DateTime.UtcNow; - var commandTextBuilder = new StringBuilder("select ") - .AppendJoin(',', GetFinalColumnsToSelect(query, new List { "guid" })) + var columns = new List { "guid" }; + SetFinalColumnsToSelect(query, columns); + var commandTextBuilder = new StringBuilder("select ", 256) + .AppendJoin(',', columns) .Append(FromText) .Append(GetJoinUserDataText(query)); @@ -3084,13 +3094,13 @@ namespace Emby.Server.Implementations.Data if (query.Limit.HasValue || offset > 0) { commandTextBuilder.Append(" LIMIT ") - .Append((query.Limit ?? int.MaxValue).ToString(CultureInfo.InvariantCulture)); + .Append(query.Limit ?? int.MaxValue); } if (offset > 0) { commandTextBuilder.Append(" OFFSET ") - .Append(offset.ToString(CultureInfo.InvariantCulture)); + .Append(offset); } } @@ -3133,7 +3143,9 @@ namespace Emby.Server.Implementations.Data var now = DateTime.UtcNow; - var commandText = "select " + string.Join(',', GetFinalColumnsToSelect(query, new List { "guid", "path" })) + FromText; + var columns = new List { "guid", "path" }; + SetFinalColumnsToSelect(query, columns); + var commandText = "select " + string.Join(',', columns) + FromText; var whereClauses = GetWhereClauses(query, null); if (whereClauses.Count != 0) @@ -3209,8 +3221,10 @@ namespace Emby.Server.Implementations.Data var now = DateTime.UtcNow; + var columns = new List { "guid" }; + SetFinalColumnsToSelect(query, columns); var commandText = "select " - + string.Join(',', GetFinalColumnsToSelect(query, new List { "guid" })) + + string.Join(',', columns) + FromText + GetJoinUserDataText(query); @@ -3251,19 +3265,23 @@ namespace Emby.Server.Implementations.Data { commandText = string.Empty; + List columnsToSelect; if (EnableGroupByPresentationUniqueKey(query)) { - commandText += " select " + string.Join(',', GetFinalColumnsToSelect(query, new List { "count (distinct PresentationUniqueKey)" })) + FromText; + columnsToSelect = new List { "count (distinct PresentationUniqueKey)" }; } else if (query.GroupBySeriesPresentationUniqueKey) { - commandText += " select " + string.Join(',', GetFinalColumnsToSelect(query, new List { "count (distinct SeriesPresentationUniqueKey)" })) + FromText; + columnsToSelect = new List { "count (distinct SeriesPresentationUniqueKey)" }; } else { - commandText += " select " + string.Join(',', GetFinalColumnsToSelect(query, new List { "count (guid)" })) + FromText; + columnsToSelect = new List { "count (guid)" }; } + SetFinalColumnsToSelect(query, columnsToSelect); + commandText += " select " + string.Join(',', columnsToSelect) + FromText; + commandText += GetJoinUserDataText(query) + whereText; statementTexts.Add(commandText); @@ -5232,30 +5250,30 @@ AND Type = @InternalPersonType)"); var now = DateTime.UtcNow; - var stringBuilder = new StringBuilder("Select Value From ItemValues where Type"); + var stringBuilder = new StringBuilder("Select Value From ItemValues where Type", 128); if (itemValueTypes.Length == 1) { stringBuilder.Append('=') - .Append(itemValueTypes[0].ToString(CultureInfo.InvariantCulture)); + .Append(itemValueTypes[0]); } else { stringBuilder.Append(" in (") - .AppendJoin(',', itemValueTypes.Select(i => i.ToString(CultureInfo.InvariantCulture))) + .AppendJoin(',', itemValueTypes) .Append(')'); } if (withItemTypes.Count > 0) { stringBuilder.Append(" AND ItemId In (select guid from typedbaseitems where type in (") - .AppendJoin(',', withItemTypes.Select(i => "'" + i + "'")) + .AppendJoinInSingleQuotes(',', withItemTypes) .Append("))"); } if (excludeItemTypes.Count > 0) { stringBuilder.Append(" AND ItemId not In (select guid from typedbaseitems where type in (") - .AppendJoin(',', excludeItemTypes.Select(i => "'" + i + "'")) + .AppendJoinInSingleQuotes(',', excludeItemTypes) .Append("))"); } @@ -5296,14 +5314,14 @@ AND Type = @InternalPersonType)"); var now = DateTime.UtcNow; var typeClause = itemValueTypes.Length == 1 ? - ("Type=" + itemValueTypes[0].ToString(CultureInfo.InvariantCulture)) : - ("Type in (" + string.Join(',', itemValueTypes.Select(i => i.ToString(CultureInfo.InvariantCulture))) + ")"); + ("Type=" + itemValueTypes[0]) : + ("Type in (" + string.Join(',', itemValueTypes) + ")"); InternalItemsQuery typeSubQuery = null; string itemCountColumns = null; - var stringBuilder = new StringBuilder(); + var stringBuilder = new StringBuilder(1024); var typesToCount = query.IncludeItemTypes; if (typesToCount.Length > 0) @@ -5326,6 +5344,7 @@ AND Type = @InternalPersonType)"); stringBuilder.Append(" where ") .AppendJoin(" AND ", whereClauses) + .Append(" AND ") .Append("guid in (select ItemId from ItemValues where ItemValues.CleanValue=A.CleanName AND ") .Append(typeClause) .Append(")) as itemTypes"); @@ -5359,7 +5378,7 @@ AND Type = @InternalPersonType)"); IsSeries = query.IsSeries }; - columns = GetFinalColumnsToSelect(query, columns); + SetFinalColumnsToSelect(query, columns); var innerWhereClauses = GetWhereClauses(innerQuery, null); @@ -5431,13 +5450,13 @@ AND Type = @InternalPersonType)"); if (query.Limit.HasValue || offset > 0) { stringBuilder.Append(" LIMIT ") - .Append((query.Limit ?? int.MaxValue).ToString(CultureInfo.InvariantCulture)); + .Append(query.Limit ?? int.MaxValue); } if (offset > 0) { stringBuilder.Append(" OFFSET ") - .Append(offset.ToString(CultureInfo.InvariantCulture)); + .Append(offset); } } @@ -5454,8 +5473,10 @@ AND Type = @InternalPersonType)"); if (query.EnableTotalRecordCount) { stringBuilder.Clear(); + var columnsToSelect = new List { "count (distinct PresentationUniqueKey)" }; + SetFinalColumnsToSelect(query, columnsToSelect); stringBuilder.Append("select ") - .AppendJoin(',', GetFinalColumnsToSelect(query, new List { "count (distinct PresentationUniqueKey)" })) + .AppendJoin(',', columnsToSelect) .Append(FromText) .Append(GetJoinUserDataText(query)) .Append(whereText); diff --git a/MediaBrowser.Common/Extensions/StringBuilderExtensions.cs b/MediaBrowser.Common/Extensions/StringBuilderExtensions.cs new file mode 100644 index 0000000000..0e0bc4282e --- /dev/null +++ b/MediaBrowser.Common/Extensions/StringBuilderExtensions.cs @@ -0,0 +1,34 @@ +using System.Collections.Generic; +using System.Text; + +namespace MediaBrowser.Common.Extensions +{ + /// + /// Extension methods for the class. + /// + public static class StringBuilderExtensions + { + /// + /// Concatenates and appends the members of a collection in single quotes using the specified delimiter. + /// + /// The string builder. + /// The character delimiter. + /// The collection of strings to concatenate. + /// The updated string builder. + public static StringBuilder AppendJoinInSingleQuotes(this StringBuilder builder, char delimiter, IReadOnlyList values) + { + for (var i = 0; i < values.Count; i++) + { + builder.Append('\'') + .Append(values[i]) + .Append('\'') + .Append(delimiter); + } + + // remove last , + builder.Length--; + + return builder; + } + } +} diff --git a/MediaBrowser.Controller/Providers/DirectoryService.cs b/MediaBrowser.Controller/Providers/DirectoryService.cs index ba52907633..b312702704 100644 --- a/MediaBrowser.Controller/Providers/DirectoryService.cs +++ b/MediaBrowser.Controller/Providers/DirectoryService.cs @@ -60,10 +60,7 @@ namespace MediaBrowser.Controller.Providers } public IReadOnlyList GetFilePaths(string path) - => GetFilePaths(path, false, false); - - public IReadOnlyList GetSortedFilePaths(string path, bool clearCache) - => GetFilePaths(path, clearCache, true); + => GetFilePaths(path, false); public IReadOnlyList GetFilePaths(string path, bool clearCache, bool sort = false) { diff --git a/MediaBrowser.Controller/Providers/IDirectoryService.cs b/MediaBrowser.Controller/Providers/IDirectoryService.cs index beb685b6d8..b1a36e1024 100644 --- a/MediaBrowser.Controller/Providers/IDirectoryService.cs +++ b/MediaBrowser.Controller/Providers/IDirectoryService.cs @@ -15,8 +15,6 @@ namespace MediaBrowser.Controller.Providers IReadOnlyList GetFilePaths(string path); - IReadOnlyList GetSortedFilePaths(string path, bool clearCache); - IReadOnlyList GetFilePaths(string path, bool clearCache, bool sort = false); } } diff --git a/MediaBrowser.Providers/MediaInfo/SubtitleResolver.cs b/MediaBrowser.Providers/MediaInfo/SubtitleResolver.cs index cb8ae0c510..3cd7ec7728 100644 --- a/MediaBrowser.Providers/MediaInfo/SubtitleResolver.cs +++ b/MediaBrowser.Providers/MediaInfo/SubtitleResolver.cs @@ -187,7 +187,7 @@ namespace MediaBrowser.Providers.MediaInfo IDirectoryService directoryService, bool clearCache) { - var files = directoryService.GetSortedFilePaths(folder, clearCache); + var files = directoryService.GetFilePaths(folder, clearCache, true); AddExternalSubtitleStreams(streams, videoPath, startIndex, files); } diff --git a/tests/Jellyfin.Server.Implementations.Tests/Data/SqliteItemRepositoryTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Data/SqliteItemRepositoryTests.cs index f312933fbf..ba67e55f3d 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/Data/SqliteItemRepositoryTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/Data/SqliteItemRepositoryTests.cs @@ -164,10 +164,7 @@ namespace Jellyfin.Server.Implementations.Tests.Data } } }; - } - public static IEnumerable DeserializeImages_ValidAndInvalid_TestData() - { yield return new object[] { string.Empty, @@ -215,23 +212,6 @@ namespace Jellyfin.Server.Implementations.Tests.Data } } - [Theory] - [MemberData(nameof(DeserializeImages_ValidAndInvalid_TestData))] - public void DeserializeImages_ValidAndInvalid_Success(string value, ItemImageInfo[] expected) - { - var result = _sqliteItemRepository.DeserializeImages(value); - Assert.Equal(expected.Length, result.Length); - for (int i = 0; i < expected.Length; i++) - { - Assert.Equal(expected[i].Path, result[i].Path); - Assert.Equal(expected[i].Type, result[i].Type); - Assert.Equal(expected[i].DateModified, result[i].DateModified); - Assert.Equal(expected[i].Width, result[i].Width); - Assert.Equal(expected[i].Height, result[i].Height); - Assert.Equal(expected[i].BlurHash, result[i].BlurHash); - } - } - [Theory] [MemberData(nameof(DeserializeImages_Valid_TestData))] public void SerializeImages_Valid_Success(string expected, ItemImageInfo[] value) -- cgit v1.2.3