From 9fc7c99022a91e0ad9d92127c8a0510e67ed0beb Mon Sep 17 00:00:00 2001 From: Christopher Snowhill Date: Thu, 27 Feb 2025 19:02:33 -0800 Subject: [PATCH] Optimization: Perform container checks in queue Perform the file container checks in an operation queue, since those are a major bottleneck at this point, too. Signed-off-by: Christopher Snowhill --- Playlist/PlaylistLoader.h | 1 + Playlist/PlaylistLoader.m | 182 +++++++++++++++++++++++++------------- 2 files changed, 120 insertions(+), 63 deletions(-) diff --git a/Playlist/PlaylistLoader.h b/Playlist/PlaylistLoader.h index 8b59e8b29..8dea8d2c8 100644 --- a/Playlist/PlaylistLoader.h +++ b/Playlist/PlaylistLoader.h @@ -27,6 +27,7 @@ typedef enum { IBOutlet NSScrollView *playlistView; IBOutlet PlaybackController *playbackController; + NSOperationQueue *containerQueue; NSOperationQueue *queue; BOOL metadataLoadInProgress; diff --git a/Playlist/PlaylistLoader.m b/Playlist/PlaylistLoader.m index 87b115925..a992666b6 100644 --- a/Playlist/PlaylistLoader.m +++ b/Playlist/PlaylistLoader.m @@ -52,6 +52,9 @@ extern NSMutableDictionary *kArtworkDictionary; if(self) { [self initDefaults]; + containerQueue = [[NSOperationQueue alloc] init]; + [containerQueue setMaxConcurrentOperationCount:8]; + queue = [[NSOperationQueue alloc] init]; [queue setMaxConcurrentOperationCount:8]; @@ -335,15 +338,15 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc } - (NSArray *)insertURLs:(NSArray *)urls atIndex:(NSInteger)index sort:(BOOL)sort { - NSMutableSet *uniqueURLs = [NSMutableSet set]; + __block NSMutableSet *uniqueURLs = [NSMutableSet set]; - NSMutableArray *expandedURLs = [NSMutableArray array]; - NSMutableArray *containedURLs = [NSMutableArray array]; - NSMutableArray *fileURLs = [NSMutableArray array]; - NSMutableArray *validURLs = [NSMutableArray array]; - NSMutableArray *folderURLs = [NSMutableArray array]; - NSMutableArray *dependencyURLs = [NSMutableArray array]; - NSDictionary *xmlData = nil; + __block NSMutableArray *expandedURLs = [[NSMutableArray alloc] init]; + __block NSMutableArray *containedURLs = [[NSMutableArray alloc] init]; + __block NSMutableArray *fileURLs = [[NSMutableArray alloc] init]; + NSMutableArray *validURLs = [[NSMutableArray alloc] init]; + NSMutableArray *folderURLs = [[NSMutableArray alloc] init]; + NSMutableArray *dependencyURLs = [[NSMutableArray alloc] init]; + __block NSDictionary *xmlData = nil; BOOL addOtherFilesInFolder = [[NSUserDefaults standardUserDefaults] boolForKey:@"addOtherFilesInFolders"]; @@ -422,73 +425,126 @@ static inline void dispatch_sync_reentrant(dispatch_queue_t queue, dispatch_bloc progressstep = [expandedURLs count] ? 100.0 / (double)([expandedURLs count]) : 0; - id containerTask = [mainTask startChildWithOperation:@"Process paths for containers"]; + if([expandedURLs count]) { + __block id containerTask = [mainTask startChildWithOperation:@"Process paths for containers"]; + + __block NSLock *lock = [[NSLock alloc] init]; + + __block NSArray *acceptableContainerTypes = [self acceptableContainerTypes]; + + __block double weakProgress = progress; + __block double weakProgressstep = progressstep; - for(url in expandedURLs) { // Container vs non-container url - id pathTask = nil; - @try { - pathTask = [containerTask startChildWithOperation:@"Process path as container" description:[NSString stringWithFormat:@"Checking if file is container: %@", url]]; - if([[self acceptableContainerTypes] containsObject:[[url pathExtension] lowercaseString]]) { - id innerTask = [pathTask startChildWithOperation:@"Container, processing"]; - - NSArray *urls = [AudioContainer urlsForContainerURL:url]; - - if(urls != nil && [urls count] != 0) { - [containedURLs addObjectsFromArray:urls]; - - // Make sure the container isn't added twice. - [uniqueURLs addObject:url]; - - // Find the dependencies - NSArray *depURLs = [AudioContainer dependencyUrlsForContainerURL:url]; - - BOOL localFound = NO; - for(NSURL *u in urls) { - if([u isFileURL]) { - localFound = YES; - break; - } + for(size_t i = 0, j = [expandedURLs count]; i < j; ++i) { + NSBlockOperation *op = [[NSBlockOperation alloc] init]; + + [op addExecutionBlock:^{ + id pathTask = nil; + id innerTask = nil; + @try { + if(containerTask) { + pathTask = [containerTask startChildWithOperation:@"Process path as container" description:[NSString stringWithFormat:@"Checking if file is container: %@", url]]; } - if(depURLs) { - [dependencyURLs addObjectsFromArray:depURLs]; - - for(NSURL *u in depURLs) { - if([u isFileURL]) { - localFound = YES; - break; + + [lock lock]; + NSURL *url = [expandedURLs objectAtIndex:0]; + [expandedURLs removeObjectAtIndex:0]; + [lock unlock]; + + if([acceptableContainerTypes containsObject:[[url pathExtension] lowercaseString]]) { + if(pathTask) { + innerTask = [pathTask startChildWithOperation:@"Container, processing"]; + } + + NSArray *urls = [AudioContainer urlsForContainerURL:url]; + + if(urls != nil && [urls count] != 0) { + [lock lock]; + [containedURLs addObjectsFromArray:urls]; + [lock unlock]; + + // Make sure the container isn't added twice. + [lock lock]; + [uniqueURLs addObject:url]; + [lock unlock]; + + // Find the dependencies + NSArray *depURLs = [AudioContainer dependencyUrlsForContainerURL:url]; + + BOOL localFound = NO; + for(NSURL *u in urls) { + if([u isFileURL]) { + localFound = YES; + break; + } } + if(depURLs) { + [lock lock]; + [dependencyURLs addObjectsFromArray:depURLs]; + [lock unlock]; + + for(NSURL *u in depURLs) { + if([u isFileURL]) { + localFound = YES; + break; + } + } + } + if(localFound) { + [[SandboxBroker sharedSandboxBroker] requestFolderForFile:url]; + } + } else { + /* Fall back on adding the raw file if all container parsers have failed. */ + [lock lock]; + [fileURLs addObject:url]; + [lock unlock]; } + if(innerTask) { + [innerTask finish]; + innerTask = nil; + } + } else if([[[url pathExtension] lowercaseString] isEqualToString:@"xml"]) { + [lock lock]; + xmlData = [XmlContainer entriesForContainerURL:url]; + [lock unlock]; + } else { + [lock lock]; + [fileURLs addObject:url]; + [lock unlock]; } - if(localFound) { - [[SandboxBroker sharedSandboxBroker] requestFolderForFile:url]; + if(pathTask) { + [pathTask finish]; + pathTask = nil; } - } else { - /* Fall back on adding the raw file if all container parsers have failed. */ - [fileURLs addObject:url]; } - [innerTask finish]; - } else if([[[url pathExtension] lowercaseString] isEqualToString:@"xml"]) { - xmlData = [XmlContainer entriesForContainerURL:url]; - } else { - [fileURLs addObject:url]; - } - [pathTask finish]; - } - @catch(id anException) { - DLog(@"Exception caught while processing for containers: %@", anException); - [SentrySDK captureException:anException]; - if(pathTask) { - [pathTask finishWithStatus:kSentrySpanStatusInternalError]; - } + @catch(id anException) { + DLog(@"Exception caught while processing for containers: %@", anException); + [SentrySDK captureException:anException]; + if(innerTask) { + [innerTask finishWithStatus:kSentrySpanStatusInternalError]; + } + if(pathTask) { + [pathTask finishWithStatus:kSentrySpanStatusInternalError]; + } + } + + [lock lock]; + weakProgress += weakProgressstep; + [self setProgressJobStatus:weakProgress]; + [lock unlock]; + }]; + + [containerQueue addOperation:op]; } - progress += progressstep; - [self setProgressJobStatus:progress]; + [containerQueue waitUntilAllOperationsAreFinished]; + + progress = weakProgress; + + [containerTask finish]; } - [containerTask finish]; - progress = 0.0; [self completeProgressJob];