Skip to content

Commit 8ad1594

Browse files
cymbalrushcymbalrush
committed
Improve error logging
Signed-off-by: Gyanendra Sinha <[email protected]> Address comments Address comments
1 parent bcf4b46 commit 8ad1594

30 files changed

+291
-258
lines changed

‎backends/apple/coreml/runtime/delegate/ETCoreMLAsset.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
#import <Foundation/Foundation.h>
99

10-
#import <asset.h>
10+
#import "asset.h"
1111

1212
NS_ASSUME_NONNULL_BEGIN
1313

‎backends/apple/coreml/runtime/delegate/ETCoreMLAsset.mm‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@
55
//
66
// Please refer to the license found in the LICENSE file in the root directory of the source tree.
77

8-
#import <ETCoreMLAsset.h>
8+
#import "ETCoreMLAsset.h"
9+
10+
#import "ETCoreMLLogging.h"
11+
#import "objc_safe_cast.h"
912

1013
#import <fcntl.h>
1114
#import <os/lock.h>
1215
#import <stdio.h>
1316
#import <system_error>
14-
15-
#import <objc_safe_cast.h>
16-
1717
namespace {
1818
using namespace executorchcoreml;
1919

@@ -85,6 +85,10 @@ - (void)dealloc {
8585

8686
- (BOOL)_keepAliveAndReturnError:(NSError * __autoreleasing *)error {
8787
if (!_isValid) {
88+
ETCoreMLLogErrorAndSetNSError(error,
89+
ETCoreMLErrorCorruptedModel,
90+
"The asset with identifier = %@ is invalid. Some required asset files appear to be missing.",
91+
_identifier);
8892
return NO;
8993
}
9094

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
#import <Foundation/Foundation.h>
99

10-
#import <database.hpp>
10+
#import "database.hpp"
1111

1212
@class ETCoreMLAsset;
1313

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

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@
66
// Please refer to the license found in the LICENSE file in the root directory of the source tree.
77

88
#import "ETCoreMLAssetManager.h"
9-
#import <ETCoreMLAsset.h>
10-
#import <ETCoreMLLogging.h>
11-
#import <database.hpp>
9+
10+
#import "ETCoreMLAsset.h"
11+
#import "ETCoreMLLogging.h"
12+
#import "database.hpp"
13+
#import "json_key_value_store.hpp"
14+
#import "serde_json.h"
15+
1216
#import <iostream>
13-
#import <json_key_value_store.hpp>
14-
#import <serde_json.h>
1517
#import <sstream>
1618

1719
namespace {
@@ -365,8 +367,7 @@ - (void)cleanupAssetIfNeeded:(ETCoreMLAsset *)asset {
365367
NSError *cleanupError = nil;
366368
if (![self _removeAssetWithIdentifier:asset.identifier error:&cleanupError]) {
367369
ETCoreMLLogError(cleanupError,
368-
"%@: Failed to remove asset with identifier = %@",
369-
NSStringFromClass(ETCoreMLAssetManager.class),
370+
"Failed to remove asset with identifier = %@",
370371
identifier);
371372
}
372373
});
@@ -440,9 +441,7 @@ - (void)triggerCompaction {
440441
dispatch_async(self.syncQueue, ^{
441442
NSError *localError = nil;
442443
if (![weakSelf _compact:self.maxAssetsSizeInBytes error:&localError]) {
443-
ETCoreMLLogError(localError,
444-
"%@: Failed to compact asset store.",
445-
NSStringFromClass(ETCoreMLAssetManager.class));
444+
ETCoreMLLogError(localError, "Failed to compact asset store.");
446445
}
447446
});
448447
}
@@ -486,11 +485,11 @@ - (nullable ETCoreMLAsset *)assetWithIdentifier:(NSString *)identifier
486485

487486
if ([result keepAliveAndReturnError:error]) {
488487
[self.assetsInUseMap setObject:result forKey:identifier];
489-
} else {
490-
[self cleanupAssetIfNeeded:result];
491-
}
488+
return result;
489+
}
492490

493-
return result;
491+
[self cleanupAssetIfNeeded:result];
492+
return nil;
494493
}
495494

496495
- (BOOL)_containsAssetWithIdentifier:(NSString *)identifier
@@ -587,8 +586,7 @@ - (BOOL)removeAssetWithIdentifier:(NSString *)identifier
587586
[assets addObject:asset];
588587
} else if (localError) {
589588
ETCoreMLLogError(localError,
590-
"%@: Failed to retrieve asset with identifier = %@",
591-
NSStringFromClass(ETCoreMLAssetManager.class),
589+
"Failed to retrieve asset with identifier = %@.",
592590
identifier);
593591
}
594592

@@ -647,8 +645,7 @@ - (NSUInteger)_compact:(NSUInteger)sizeInBytes error:(NSError * __autoreleasing
647645
NSString *identifier = @(asset.identifier.c_str());
648646
if (![self _removeAssetWithIdentifier:identifier error:&cleanupError] && cleanupError) {
649647
ETCoreMLLogError(cleanupError,
650-
"%@: Failed to remove asset with identifier = %@",
651-
NSStringFromClass(ETCoreMLAssetManager.class),
648+
"Failed to remove asset with identifier = %@.",
652649
identifier);
653650
}
654651
}
@@ -689,8 +686,7 @@ - (void)removeFilesInTrashDirectory {
689686
for (NSURL *itemURL in enumerator) {
690687
if (![fileManager removeItemAtURL:itemURL error:&localError]) {
691688
ETCoreMLLogError(localError,
692-
"%@: Failed to remove item in trash directory with name = %@",
693-
NSStringFromClass(ETCoreMLAssetManager.class),
689+
"Failed to remove item in trash directory with name = %@",
694690
itemURL.lastPathComponent);
695691
}
696692
}
@@ -720,9 +716,7 @@ - (BOOL)_purge:(NSError * __autoreleasing *)error {
720716
NSError *localError = nil;
721717
// Create the assets directory, if we fail here it's okay.
722718
if (![self.fileManager createDirectoryAtURL:self.assetsDirectoryURL withIntermediateDirectories:NO attributes:@{} error:&localError]) {
723-
ETCoreMLLogError(localError,
724-
"%@: Failed to create assets directory",
725-
NSStringFromClass(ETCoreMLAssetManager.class));
719+
ETCoreMLLogError(localError, "Failed to create assets directory.");
726720
}
727721

728722
return true;

‎backends/apple/coreml/runtime/delegate/ETCoreMLDefaultModelExecutor.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
//
2-
// ETCoreMLDefaultModelExecutor.h
3-
// executorchcoreml_tests
2+
// ETCoreMLDefaultModelExecutor.h
43
//
5-
// Created by Gyan Sinha on 2/25/24.
4+
// Copyright © 2024 Apple Inc. All rights reserved.
65
//
6+
// Please refer to the license found in the LICENSE file in the root directory of the source tree.
77

88
#import <CoreML/CoreML.h>
99

10-
#import <ETCoreMLModelExecutor.h>
10+
#import "ETCoreMLModelExecutor.h"
1111

1212
@class ETCoreMLModel;
1313

‎backends/apple/coreml/runtime/delegate/ETCoreMLDefaultModelExecutor.mm‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
//
2-
// ETCoreMLDefaultModelExecutor.m
3-
// executorchcoreml_tests
2+
// ETCoreMLDefaultModelExecutor.mm
43
//
5-
// Created by Gyan Sinha on 2/25/24.
4+
// Copyright © 2024 Apple Inc. All rights reserved.
65
//
6+
// Please refer to the license found in the LICENSE file in the root directory of the source tree.
77

8-
#import <ETCoreMLAsset.h>
9-
#import <ETCoreMLDefaultModelExecutor.h>
10-
#import <ETCoreMLLogging.h>
11-
#import <ETCoreMLModel.h>
8+
#import "ETCoreMLAsset.h"
9+
#import "ETCoreMLDefaultModelExecutor.h"
10+
#import "ETCoreMLLogging.h"
11+
#import "ETCoreMLModel.h"
1212

1313
@implementation ETCoreMLDefaultModelExecutor
1414

@@ -27,7 +27,9 @@ - (instancetype)initWithModel:(ETCoreMLModel *)model {
2727
eventLogger:(const executorchcoreml::ModelEventLogger* _Nullable __unused)eventLogger
2828
error:(NSError * __autoreleasing *)error {
2929
if (self.ignoreOutputBackings) {
30-
predictionOptions.outputBackings = @{};
30+
if (@available(macOS 11.0, iOS 16.0, tvOS 16.0, watchOS 9.0, *)) {
31+
predictionOptions.outputBackings = @{};
32+
}
3133
}
3234

3335
id<MLFeatureProvider> outputs = [self.model predictionFromFeatures:inputs
@@ -44,8 +46,7 @@ - (instancetype)initWithModel:(ETCoreMLModel *)model {
4446
if (!featureValue.multiArrayValue) {
4547
ETCoreMLLogErrorAndSetNSError(error,
4648
ETCoreMLErrorBrokenModel,
47-
"%@: Model is broken, expected multiarray for output=%@.",
48-
NSStringFromClass(self.class),
49+
"Model is broken, expected multiarray for output=%@.",
4950
outputName);
5051
return nil;
5152
}

‎backends/apple/coreml/runtime/delegate/ETCoreMLLogging.h‎

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
// Please refer to the license found in the LICENSE file in the root directory of the source tree.
77

88
#import <Foundation/Foundation.h>
9+
#import <os/log.h>
910

1011
#import <executorch/runtime/platform/log.h>
11-
#import <os/log.h>
1212

1313
NS_ASSUME_NONNULL_BEGIN
1414

@@ -18,15 +18,15 @@ extern NSErrorDomain const ETCoreMLErrorDomain;
1818
/// The error codes that are exposed publicly.
1919
typedef NS_ERROR_ENUM(ETCoreMLErrorDomain, ETCoreMLError) {
2020
ETCoreMLErrorCorruptedData = 1, // AOT blob can't be parsed.
21-
ETCoreMLErrorCorruptedMetadata, // AOT blob has incorrect or missing metadata.
22-
ETCoreMLErrorCorruptedModel, // AOT blob has incorrect or missing CoreML model.
23-
ETCoreMLErrorBrokenModel, // CoreML model doesn't match the input and output specification.
24-
ETCoreMLErrorCompilationFailed, // CoreML model failed to compile.
25-
ETCoreMLErrorModelCompilationNotSupported, // CoreML model compilation is not supported by the target.
26-
ETCoreMLErrorModelProfilingNotSupported, // Model profiling is not supported by the target.
27-
ETCoreMLErrorModelSaveFailed, // Failed to save CoreML model to disk.
28-
ETCoreMLErrorModelCacheCreationFailed, // Failed to create model cache.
29-
ETCoreMLErrorInternalError, // Internal error.
21+
ETCoreMLErrorCorruptedMetadata = 2, // AOT blob has incorrect or missing metadata.
22+
ETCoreMLErrorCorruptedModel = 3, // AOT blob has incorrect or missing CoreML model.
23+
ETCoreMLErrorBrokenModel = 4, // CoreML model doesn't match the input and output specification.
24+
ETCoreMLErrorCompilationFailed = 5, // CoreML model failed to compile.
25+
ETCoreMLErrorModelCompilationNotSupported = 6, // CoreML model compilation is not supported by the target.
26+
ETCoreMLErrorModelProfilingNotSupported = 7, // Model profiling is not supported by the target.
27+
ETCoreMLErrorModelSaveFailed = 8, // Failed to save CoreML model to disk.
28+
ETCoreMLErrorModelCacheCreationFailed = 9, // Failed to create model cache.
29+
ETCoreMLErrorInternalError = 10, // Internal error.
3030
};
3131

3232
@interface ETCoreMLErrorUtils : NSObject
@@ -47,47 +47,47 @@ typedef NS_ERROR_ENUM(ETCoreMLErrorDomain, ETCoreMLError) {
4747
#pragma clang diagnostic push
4848
#pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments"
4949

50+
#if ET_LOG_ENABLED
51+
#define ETCoreMLLogError(error, formatString, ...) \
52+
do { \
53+
NSString* message = error.localizedDescription; \
54+
message = [NSString stringWithFormat:@"[Core ML] " formatString " %@", ##__VA_ARGS__, message]; \
55+
ET_LOG(Error, "%s", message.UTF8String); \
56+
} while (0)
57+
#else
58+
#define ETCoreMLLogError(error, formatString, ...) \
59+
os_log_error(ETCoreMLErrorUtils.loggingChannel, " %@", ##__VA_ARGS__, error.localizedDescription)
60+
#endif
61+
62+
#if ET_LOG_ENABLED
63+
#define ETCoreMLLogInfo(formatString, ...) \
64+
ET_LOG(Info, "%s", [NSString stringWithFormat:formatString, ##__VA_ARGS__].UTF8String)
65+
#else
66+
#define ETCoreMLLogInfo(formatString, ...) os_log_info(ETCoreMLErrorUtils.loggingChannel, formatString, ##__VA_ARGS__)
67+
#endif
68+
5069
/// Record the error with `os_log_error` and fills `*errorOut` with `NSError`.
51-
#define ETCoreMLLogErrorAndSetNSError(errorOut, errorCode, formatString, ...) \
52-
if (ET_LOG_ENABLED) { \
53-
ET_LOG(Error, "%s", [NSString stringWithFormat:@formatString, ##__VA_ARGS__].UTF8String); \
54-
} else { \
55-
os_log_error(ETCoreMLErrorUtils.loggingChannel, formatString, ##__VA_ARGS__); \
56-
} \
57-
if (errorOut) { \
58-
*errorOut = \
59-
[NSError errorWithDomain:ETCoreMLErrorDomain \
60-
code:errorCode \
61-
userInfo:@{ \
62-
NSLocalizedDescriptionKey : [NSString stringWithFormat:@formatString, ##__VA_ARGS__] \
63-
}]; \
64-
}
70+
#define ETCoreMLLogErrorAndSetNSError(errorOut, errorCode, formatString, ...) \
71+
do { \
72+
NSDictionary* userInfo = \
73+
@{ NSLocalizedDescriptionKey : [NSString stringWithFormat:@formatString, ##__VA_ARGS__] }; \
74+
NSError* localError = [NSError errorWithDomain:ETCoreMLErrorDomain code:errorCode userInfo:userInfo]; \
75+
ETCoreMLLogError(localError, "%@", @"error"); \
76+
if (errorOut) { \
77+
*errorOut = localError; \
78+
} \
79+
} while (0)
6580

6681
/// Record the error and its underlying error with `os_log_error` and fills `*errorOut` with `NSError`.
6782
#define ETCoreMLLogUnderlyingErrorAndSetNSError(errorOut, errorCode, underlyingNSError, formatString, ...) \
68-
if (ET_LOG_ENABLED) { \
69-
ET_LOG(Error, "%s", [NSString stringWithFormat:@formatString, ##__VA_ARGS__].UTF8String); \
70-
} else { \
71-
os_log_error(ETCoreMLErrorUtils.loggingChannel, \
72-
formatString ", with underlying error= %@.", \
73-
##__VA_ARGS__, \
74-
(underlyingNSError).localizedDescription); \
75-
} \
76-
if (errorOut) { \
77-
*errorOut = [ETCoreMLErrorUtils errorWithCode:errorCode \
78-
underlyingError:underlyingNSError \
79-
format:@formatString, ##__VA_ARGS__]; \
80-
}
81-
82-
#define ETCoreMLLogError(error, formatString, ...) \
83-
if (ET_LOG_ENABLED) { \
84-
ET_LOG(Error, "%s", [NSString stringWithFormat:@formatString, ##__VA_ARGS__].UTF8String); \
85-
} else { \
86-
os_log_error(ETCoreMLErrorUtils.loggingChannel, \
87-
formatString ", with error= %@.", \
88-
##__VA_ARGS__, \
89-
(error).localizedDescription); \
90-
}
83+
do { \
84+
ETCoreMLLogError(underlyingNSError, formatString, ##__VA_ARGS__); \
85+
if (errorOut) { \
86+
*errorOut = [ETCoreMLErrorUtils errorWithCode:errorCode \
87+
underlyingError:underlyingNSError \
88+
format:@formatString, ##__VA_ARGS__]; \
89+
} \
90+
} while (0)
9191

9292

9393
#pragma clang diagnostic pop

‎backends/apple/coreml/runtime/delegate/ETCoreMLLogging.mm‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
//
66
// Please refer to the license found in the LICENSE file in the root directory of the source tree.
77

8-
#import <ETCoreMLLogging.h>
8+
#import "ETCoreMLLogging.h"
99

10-
#import <ETCoreMLStrings.h>
10+
#import "ETCoreMLStrings.h"
1111

1212
const NSErrorDomain ETCoreMLErrorDomain = @"com.apple.executorchcoreml";
1313

‎backends/apple/coreml/runtime/delegate/ETCoreMLModel.mm‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
//
66
// Please refer to the license found in the LICENSE file in the root directory of the source tree.
77

8-
#import <ETCoreMLModel.h>
8+
#import "ETCoreMLModel.h"
99

1010
#import "ETCoreMLAsset.h"
1111
#import "ETCoreMLLogging.h"
@@ -318,8 +318,7 @@ - (BOOL)prewarmAndReturnError:(NSError* __autoreleasing*)error {
318318
BOOL result = [self.mlModel prewarmUsingState:self.state error:error];
319319
if (!result) {
320320
ETCoreMLLogError(localError,
321-
"%@: Failed to prewarm model with identifier = %@",
322-
NSStringFromClass(self.class),
321+
"Failed to prewarm model with identifier = %@",
323322
self.identifier);
324323
}
325324

0 commit comments

Comments
 (0)