Back out "Improve asset management"

Summary:
Original commit changeset: 46d2f763787b

Original Phabricator Diff: D80715432

Since the introduction of D80715432, WhatsApp is seeing an increase in model load failures with error code 0x23 (P1948468840).  From logs, the underlying issue is something like "Model init failed “model.mlmodelc” couldn’t be moved to “models” because an item with the same name already exists."

FB app has also seen an increase in error code 0x23 since 9/5.
We are unable to reproduce this error locally, but out of caution we want to revert D80715432 and see if the errors resolve.

Since D80715432 landed, we also landed D81256207 to add more logging.  To prevent conflicts, we backout in 3 steps:
* In diff 1/3, we backout D81256207, which added more logging
* In diff 2/3, we backout D80715432, which is the problematic diff we're trying to revert
* In diff 3/3, we introduce some of the logging we backed out in diff 1/3

Differential Revision: D82581443

Author: metascroy

backends/apple/coreml/runtime/delegate/ETCoreMLAssetManager.h

@@ -99,17 +99,6 @@ NS_ASSUME_NONNULL_BEGIN
 - (NSUInteger)compact:(NSUInteger)sizeInBytes error:(NSError* __autoreleasing*)error;
 
 
-/// Executes a block with a unique temporary directory.
-///
-/// A new temporary subdirectory URL is created inside the receiver’s designated
-/// base directory. The directory is passed to the block, which can use it to
-/// perform temporary file operations. After the block finishes executing,
-/// the directory and its contents are removed.
-///
-/// @param block A block to execute. The block receives a unique URL.
-- (void)withTemporaryDirectory:(void (^)(NSURL* directoryURL))block;
-
-
 /// Purges the assets storage. The assets are moved to the trash directory and are asynchronously
 /// deleted.
 ///
@@ -128,12 +117,6 @@ NS_ASSUME_NONNULL_BEGIN
 /// contents are deleted asynchronously.
 @property (copy, readonly, nonatomic) NSURL* trashDirectoryURL;
 
-
-/// The staging directory URL, used to hold assets that are being prepared or processed
-/// before they are moved into their final location. The contents of this directory
-/// are temporary and may be cleared when no longer needed.
-@property (copy, readonly, nonatomic) NSURL* stagingDirectoryURL;
-
 /// The file manager.
 @property (strong, readonly, nonatomic) NSFileManager* fileManager;
 

backends/apple/coreml/runtime/delegate/ETCoreMLAssetManager.mm

@@ -254,29 +254,6 @@ BOOL is_asset_alive(NSMapTable<NSString *, ETCoreMLAsset *> *assets_in_use_map,
 
     return assets;
 }
-
-NSURL * _Nullable move_to_directory(NSURL *url,
-                                    NSURL *directoryURL,
-                                    NSFileManager *fileManager,
-                                    NSError * __autoreleasing *error) {
-    if (!url) {
-        ETCoreMLLogErrorAndSetNSError(error, ETCoreMLErrorInternalError, "Move operation failed: source URL is nil.");
-        return nil;
-    }
-
-    if (!directoryURL) {
-        ETCoreMLLogErrorAndSetNSError(error, ETCoreMLErrorInternalError, "Move operation failed: destination URL is nil.");
-        return nil;
-    }
-
-    NSURL *dstURL = [directoryURL URLByAppendingPathComponent:[NSUUID UUID].UUIDString];
-    if (![fileManager moveItemAtURL:url toURL:dstURL error:error]) {
-        return nil;
-    }
-
-    return dstURL;
-}
-
 } //namespace
 
 @interface ETCoreMLAssetManager () <NSFileManagerDelegate> {
@@ -322,17 +299,12 @@ - (nullable instancetype)initWithDatabase:(const std::shared_ptr<Database>&)data
     if (!managedAssetsDirectoryURL) {
         return nil;
     }
-
+    
     NSURL *managedTrashDirectoryURL = ::create_directory_if_needed(trashDirectoryURL, @"models", fileManager, error);
     if (!managedTrashDirectoryURL) {
         return nil;
     }
-
-    NSURL *managedStagingDirectoryURL = ::create_directory_if_needed(assetsDirectoryURL, @"staging", fileManager, error);
-    if (!managedStagingDirectoryURL) {
-        return nil;
-    }
-
+    
     // If directory is empty then purge the stores
     if (::is_directory_empty(managedAssetsDirectoryURL, fileManager, nil)) {
         assetsMetaStore.impl()->purge(ec);
@@ -343,7 +315,6 @@ - (nullable instancetype)initWithDatabase:(const std::shared_ptr<Database>&)data
         _assetsStore = std::move(assetsStore);
         _assetsMetaStore = std::move(assetsMetaStore);
         _assetsDirectoryURL = managedAssetsDirectoryURL;
-        _stagingDirectoryURL = managedStagingDirectoryURL;
         _trashDirectoryURL = managedTrashDirectoryURL;
         _estimatedSizeInBytes = sizeInBytes.value();
         _maxAssetsSizeInBytes = maxAssetsSizeInBytes;
@@ -375,15 +346,15 @@ - (nullable instancetype)initWithDatabaseURL:(NSURL *)databaseURL
                             error:error];
 }
 
-- (void)withTemporaryDirectory:(void (^)(NSURL *directoryURL))block {
-    NSURL *dstURL = [self.stagingDirectoryURL URLByAppendingPathComponent:[NSUUID UUID].UUIDString];
-    block(dstURL);
-    if (![self.fileManager fileExistsAtPath:dstURL.path]) {
-        return;
+- (nullable NSURL *)moveURL:(NSURL *)url
+     toUniqueURLInDirectory:(NSURL *)directoryURL
+                      error:(NSError * __autoreleasing *)error {
+    NSURL *dstURL = [directoryURL URLByAppendingPathComponent:[NSUUID UUID].UUIDString];
+    if (![self.fileManager moveItemAtURL:url toURL:dstURL error:error]) {
+        return nil;
     }
-
-    move_to_directory(dstURL, self.trashDirectoryURL, self.fileManager, nil);
-    [self cleanupTrashDirectory];
+    
+    return dstURL;
 }
 
 - (void)cleanupAssetIfNeeded:(ETCoreMLAsset *)asset {
@@ -436,8 +407,9 @@ - (nullable ETCoreMLAsset *)_storeAssetAtURL:(NSURL *)srcURL
             return false;
         }
 
-        // If a file already exists at `dstURL`, move it to the trash for removal.
-        move_to_directory(dstURL, self.trashDirectoryURL, self.fileManager, nil);
+        // If an asset exists move it
+        [self moveURL:dstURL toUniqueURLInDirectory:self.trashDirectoryURL error:nil];
+        
         // Move the asset to assets directory.
         if (![self.fileManager moveItemAtURL:srcURL toURL:dstURL error:error]) {
             return false;
@@ -461,25 +433,16 @@ - (nullable ETCoreMLAsset *)_storeAssetAtURL:(NSURL *)srcURL
 }
 
 - (void)triggerCompaction {
-    if (self.estimatedSizeInBytes >= self.maxAssetsSizeInBytes) {
-        __weak __typeof(self) weakSelf = self;
-        dispatch_async(self.syncQueue, ^{
-            NSError *localError = nil;
-            if (![weakSelf _compact:self.maxAssetsSizeInBytes error:&localError]) {
-                ETCoreMLLogError(localError, "Failed to compact asset store.");
-            }
-        });
+    if (self.estimatedSizeInBytes < self.maxAssetsSizeInBytes) {
+        return;
     }
-
-    // Always clean the trash directory to ensure a minimal footprint.
-    // The `trashQueue` is serialized, so only one cleanup will run at a time.
-    [self cleanupTrashDirectory];
-}
-
-- (void)cleanupTrashDirectory {
+    
     __weak __typeof(self) weakSelf = self;
-    dispatch_async(self.trashQueue, ^{
-        [weakSelf removeFilesInTrashDirectory];
+    dispatch_async(self.syncQueue, ^{
+        NSError *localError = nil;
+        if (![weakSelf _compact:self.maxAssetsSizeInBytes error:&localError]) {
+            ETCoreMLLogError(localError, "Failed to compact asset store.");
+        }
     });
 }
 
@@ -585,7 +548,7 @@ - (BOOL)_removeAssetWithIdentifier:(NSString *)identifier
 
         NSURL *assetURL = ::get_asset_url(assetValue);
         if ([self.fileManager fileExistsAtPath:assetURL.path] &&
-            !move_to_directory(assetURL, self.trashDirectoryURL, self.fileManager, error)) {
+            ![self moveURL:assetURL toUniqueURLInDirectory:self.trashDirectoryURL error:error]) {
             return false;
         }
 
@@ -686,7 +649,13 @@ - (NSUInteger)_compact:(NSUInteger)sizeInBytes error:(NSError * __autoreleasing
                              identifier);
         }
     }
-
+    
+    // Trigger cleanup.
+    __weak __typeof(self) weakSelf = self;
+    dispatch_async(self.trashQueue, ^{
+        [weakSelf removeFilesInTrashDirectory];
+    });
+    
     return _estimatedSizeInBytes;
 }
 
@@ -695,10 +664,7 @@ - (NSUInteger)compact:(NSUInteger)sizeInBytes error:(NSError * __autoreleasing *
     dispatch_sync(self.syncQueue, ^{
         result = [self _compact:sizeInBytes error:error];
     });
-
-    // Always clean the trash directory to ensure a minimal footprint.
-    // The `trashQueue` is serialized, so only one cleanup will run at a time.
-    [self cleanupTrashDirectory];
+    
     return result;
 }
 
@@ -742,7 +708,7 @@ - (BOOL)_purge:(NSError * __autoreleasing *)error {
         }
 
         // Move the the whole assets directory to the temp directory.
-        if (!move_to_directory(self.assetsDirectoryURL, self.trashDirectoryURL, self.fileManager, error)) {
+        if (![self moveURL:self.assetsDirectoryURL toUniqueURLInDirectory:self.trashDirectoryURL error:error]) {
             return false;
         }
 
@@ -758,7 +724,13 @@ - (BOOL)_purge:(NSError * __autoreleasing *)error {
 
     ::set_error_from_error_code(ec, error);
     // Trigger cleanup
-    [self cleanupTrashDirectory];
+    if (status) {
+        __weak __typeof(self) weakSelf = self;
+        dispatch_async(self.trashQueue, ^{
+            [weakSelf removeFilesInTrashDirectory];
+        });
+    }
+    
     return static_cast<BOOL>(status);
 }
 

backends/apple/coreml/runtime/delegate/ETCoreMLModelLoader.mm

@@ -62,12 +62,21 @@ + (nullable ETCoreMLModel *)loadModelWithContentsOfURL:(NSURL *)compiledModelURL
     if (model) {
         return model;
     }
-
-    if (error) {
-        *error = localError;
+    
+    if (localError) {
+        ETCoreMLLogError(localError,
+                         "Failed to load model from compiled asset with identifier = %@",
+                         identifier);
     }
-
-    return nil;
+    
+    // If store failed then we will load the model from compiledURL.
+    auto backingAsset = Asset::make(compiledModelURL, identifier, assetManager.fileManager, error);
+    if (!backingAsset) {
+        return nil;
+    }
+    
+    asset = [[ETCoreMLAsset alloc] initWithBackingAsset:backingAsset.value()];
+    return ::get_model_from_asset(asset, configuration, metadata, error);
 }
 
 @end