Minor changes to reduce allocations #848

Bond-009 · 2019-02-09T15:04:24Z

Reduces stress on the GC

JustAMan · 2019-02-11T12:12:15Z

Emby.Server.Implementations/Data/BaseSqliteRepository.cs

        public static IDisposable Read(this ReaderWriterLockSlim obj)
        {
            //if (BaseSqliteRepository.ThreadSafeMode > 0)
            //{
            //    return new DummyToken();
            //}
-            return new WriteLockToken(obj);
+            return new WriteLockToken(obj); // TODO: fix segfault


this message should be expanded, something like "fix segfault if using Read/Write lock"

I took a closer look at the db code and figured out the reason, I'll try and restructure all databases when my current big PRs are in.

Still this TODO should be either reformulated to be clear or removed altogether.

Emby.Server.Implementations/Data/SqliteItemRepository.cs

Emby.Server.Implementations/HttpServer/HttpResultFactory.cs

Emby.Server.Implementations/Library/LibraryManager.cs

Emby.Server.Implementations/Serialization/JsonSerializer.cs

Jellyfin.Server/Jellyfin.Server.csproj

Jellyfin.Server/SocketSharp/WebSocketSharpRequest.cs

JustAMan

@Bond-009 you got me convinced.
LGTM!

EraYaN

LGTM

EraYaN · 2019-02-18T20:34:33Z

Emby.Server.Implementations/Data/SqliteItemRepository.cs


 #if DEBUG
-            slowThreshold = 250;
+            slowThreshold = 10;


These values should probably be documented and maybe even put in a central file.

Before we document them, we should agree on the values

The values make sense to me. It will produce a lot of warnings in DEBUG, but seeing as we want to optimize it anyway, it's fine with me.

EraYaN · 2019-02-18T20:35:50Z

Emby.Server.Implementations/Data/SqliteItemRepository.cs

@@ -2957,6 +2949,7 @@ private string GetOrderByText(InternalItemsQuery query)
            {
                var columnMap = MapOrderByField(i.Item1, query);
                var columnAscending = i.Item2 == SortOrder.Ascending;
+                const bool enableOrderInversion = false;


If this is false, the if statement below this can be removed.

enableOrderInversion was already always false, it can be removed in another PR

Lynxy · 2019-03-02T00:34:26Z

I've been having trouble with Kodi syncing TV Shows and narrowed the issue down to this commit as it works fine prior to this.

Trying to sync TV results in a dialog box saying "Something went wrong during the sync. You'll be able to restore progress when restarting Kodi. If the problem persists, please report on the Jellyfin for Kodi forums, with your Kodi log."

The message indicates I should restart Kodi but that does not help at all. Tested on both Kodi 18.1 and 17.6.

Here is the log from kodi.log when this occurs:

NOTICE: JELLYFIN.helper.wrapper -> Processing TV Shows: 7457ffe4f17493ef374c0545ad496a4e
NOTICE: JELLYFIN.objects.tvshows -> UPDATE tvshow [2/1] Media Name Redacted: 5b39e58f5df97113c8e9216ab3ec56e1
NOTICE: JELLYFIN.database -> ERROR:: type: <type 'exceptions.KeyError'> value: 'SeriesId'
NOTICE: JELLYFIN.database -> [jellyfin] 1 rows updated.
NOTICE: JELLYFIN.database -> ERROR:: type: <type 'exceptions.KeyError'> value: 'SeriesId'
NOTICE: JELLYFIN.database -> [video] 32 rows updated.

Lynxy · 2019-03-02T01:23:11Z

Here is some debug data from the Jellyfin for Kodi addon. I know this is not the repository for that, however since this commit broke it please bear with me.

This exception occurs in the loop in plugin.video.jellyfin/jellyfin/objects/tvshows.py:139.

Here is the value of season prior to this commit:

{'SpecialFeatureCount': 0, 'Genres': [], 'CanDelete': False, 'LocationType': 'FileSystem', 'LocalTrailerCount': 0, 'CanDownload': False, 'ExternalUrls': [], 'Etag': '16ce0ea676474934e49c58fb2ff47719', 'ParentLogoImageTag': '4aa4a1be8326fc683c86b59873aef391', 'ScreenshotImageTags': [], 'RemoteTrailers': [], 'ParentBackdropImageTags': ['92d764ee0b343a86a4b5aecf8da217f4'], 'ServerId': '4d0ac52d083f2915952051a645b9930b', 'Type': 'Season', 'SortName': '0001', 'SeriesId': 'ccb9e58252f971d588e92c2cb3ec7311', 'PlayAccess': 'Full', 'Tags': [], 'IsFolder': True, 'LockedFields': [], 'EnableMediaSourceDisplay': True, 'ProviderIds': {}, 'PremiereDate': '1990-01-01T05:00:00.0000000Z', 'ProductionYear': 1990, 'DisplayPreferencesId': 'dfd065f5787fb957a750d76dcf835aad', 'Path': '/media/path/redacted/Season 1', 'BackdropImageTags': [], 'Name': 'Season 1', 'SeriesStudio': 'redacted', 'LockData': False, 'SeriesName': 'Media Name Redacted', 'Taglines': [], 'ParentId': 'ccb9e58252f971d588e92c2cb3ec7311', 'PrimaryImageAspectRatio': 0.666666666666667, 'ParentLogoItemId': 'ccb9e58252f971d588e92c2cb3ec7311', 'ChildCount': 5, 'UserData': {'IsFavorite': False, 'Played': False, 'PlayedPercentage': 0, 'UnplayedItemCount': 5, 'PlaybackPositionTicks': 0, 'Key': '78886001', 'PlayCount': 0}, 'Studios': [], 'IndexNumber': 1, 'People': [], 'SeriesPrimaryImageTag': '62f0b841bfddcbeabe8a15caf7bed7fb', 'DateCreated': '2017-09-09T01:42:11.0000000Z', 'ParentBackdropItemId': 'ccb9e58252f971d588e92c2cb3ec7311', 'ImageTags': {'Primary': 'a7bd2d4acd11d6e60b789f0a793b66b5'}, 'GenreItems': [], 'RecursiveItemCount': 5, 'Id': '2c2636bf0666962ce563705eda7de9f4'}

And here is the value of season with this commit:

{'SpecialFeatureCount': 0, 'Genres': [], 'CanDelete': False, 'LocationType': 'FileSystem', 'LocalTrailerCount': 0, 'CanDownload': False, 'ExternalUrls': [], 'Etag': '16ce0ea676474934e49c58fb2ff47719', 'ParentLogoImageTag': '4aa4a1be8326fc683c86b59873aef391', 'ScreenshotImageTags': [], 'RemoteTrailers': [], 'ParentBackdropImageTags': ['92d764ee0b343a86a4b5aecf8da217f4'], 'ServerId': '4d0ac52d083f2915952051a645b9930b', 'Type': 'Season', 'SortName': '0001', 'PlayAccess': 'Full', 'Tags': [], 'IsFolder': True, 'LockedFields': [], 'EnableMediaSourceDisplay': True, 'ProviderIds': {}, 'PremiereDate': '1990-01-01T05:00:00.0000000Z', 'ProductionYear': 1990, 'DisplayPreferencesId': 'dfd065f5787fb957a750d76dcf835aad', 'Path': '/media/path/redacted/Season 1', 'BackdropImageTags': [], 'Name': 'Season 1', 'SeriesStudio': 'redacted', 'LockData': False, 'SeriesName': 'Media Name Redacted', 'Taglines': [], 'PrimaryImageAspectRatio': 0.666666666666667, 'ParentLogoItemId': 'ccb9e58252f971d588e92c2cb3ec7311', 'ChildCount': 5, 'UserData': {'IsFavorite': False, 'Played': False, 'PlayedPercentage': 0, 'UnplayedItemCount': 5, 'PlaybackPositionTicks': 0, 'Key': '78886001', 'PlayCount': 0}, 'Studios': [], 'IndexNumber': 1, 'People': [], 'SeriesPrimaryImageTag': '62f0b841bfddcbeabe8a15caf7bed7fb', 'DateCreated': '2017-09-09T01:42:11.0000000Z', 'ParentBackdropItemId': 'ccb9e58252f971d588e92c2cb3ec7311', 'ImageTags': {'Primary': 'a7bd2d4acd11d6e60b789f0a793b66b5'}, 'GenreItems': [], 'RecursiveItemCount': 5, 'Id': '2c2636bf0666962ce563705eda7de9f4'}

The keys SeriesId and ParentId are missing.

Lynxy · 2019-03-02T02:25:22Z

I have further narrowed this down to commit 64d5ec1. I can be on HEAD and revert that commit and it works just fine like it use to.

I am not familiar with what this PR is trying to accomplish and am having difficulty understanding the changes. Perhaps someone more familiar with it could chime in with why this is happening?

Lynxy · 2019-03-02T03:20:20Z

I have found the issue.

In file Emby.Server.Implementations/Data/SqliteItemRepository.cs:2280, the line that sets HashSet<string> _seriesTypes, it looks like the values were duplicated from _artistsTypes above and never properly changed to the correct values for a series. I will follow up with a new PR correcting this.

JustAMan · 2019-03-06T09:35:44Z

@Lynxy thanks for the debugging and finding the root cause!

Bond-009 force-pushed the perf branch from d61bf93 to e3771e3 Compare February 9, 2019 15:07

JustAMan suggested changes Feb 11, 2019

View reviewed changes

Bond-009 added 10 commits February 12, 2019 21:06

Use HashSets for increased perf

64d5ec1

Tuple -> ValueTuple

41fb1e5

Don't clone lists

3e6819c

Some Lists -> IEnumerable

7722cb3

Warn faster for slow requests

9dba930

Useless copy

da9418c

Less string allocations

8d98885

Last bits

585b520

Revert back to netcoreapp2.1

ea446fd

Address comments

3f13851

Bond-009 force-pushed the perf branch from c868b88 to 3f13851 Compare February 12, 2019 20:06

Remove comment

5835c4b

JustAMan approved these changes Feb 13, 2019

View reviewed changes

Merge branch 'master' into perf

4811e76

EraYaN approved these changes Feb 18, 2019

View reviewed changes

cvium approved these changes Feb 18, 2019

View reviewed changes

joshuaboniface approved these changes Feb 20, 2019

View reviewed changes

joshuaboniface merged commit 89d4ce3 into jellyfin:master Feb 20, 2019

Lynxy mentioned this pull request Mar 2, 2019

Correct the list of series types #1032

Merged

Bond-009 deleted the perf branch July 24, 2019 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor changes to reduce allocations #848

Minor changes to reduce allocations #848

Minor changes to reduce allocations #848

Minor changes to reduce allocations #848

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment