diff --git a/Bugsnag/BugsnagInternals.h b/Bugsnag/BugsnagInternals.h index 445481b74..bc7f48d2e 100644 --- a/Bugsnag/BugsnagInternals.h +++ b/Bugsnag/BugsnagInternals.h @@ -22,7 +22,8 @@ #import "BugsnagHandledState.h" #import "BugsnagNotifier.h" -typedef NSMutableArray BSGFeatureFlagStore; +@interface BSGFeatureFlagStore : NSObject +@end NS_ASSUME_NONNULL_BEGIN diff --git a/Bugsnag/Client/BugsnagClient.m b/Bugsnag/Client/BugsnagClient.m index bca1c4ce3..13b8e0a57 100644 --- a/Bugsnag/Client/BugsnagClient.m +++ b/Bugsnag/Client/BugsnagClient.m @@ -195,7 +195,7 @@ - (instancetype)initWithConfiguration:(BugsnagConfiguration *)configuration { [_configuration setUser:[BSG_KSSystemInfo deviceAndAppHash] withEmail:_configuration.user.email andName:_configuration.user.name]; } - _featureFlagStore = [configuration.featureFlagStore mutableCopy]; + _featureFlagStore = [configuration.featureFlagStore copy]; _state = [[BugsnagMetadata alloc] initWithDictionary:@{ BSGKeyClient: @{ @@ -762,7 +762,7 @@ - (void)notifyInternal:(BugsnagEvent *_Nonnull)event // App hang events will already contain feature flags if (!event.featureFlagStore.count) { @synchronized (self.featureFlagStore) { - event.featureFlagStore = [self.featureFlagStore mutableCopy]; + event.featureFlagStore = [self.featureFlagStore copy]; } } @@ -976,7 +976,7 @@ - (void)setObserver:(BSGClientObserver)observer { }; @synchronized (self.featureFlagStore) { - for (BugsnagFeatureFlag *flag in self.featureFlagStore) { + for (BugsnagFeatureFlag *flag in self.featureFlagStore.allFlags) { observer(BSGClientObserverAddFeatureFlag, flag); } } @@ -1040,7 +1040,7 @@ - (void)appHangDetectedAtDate:(NSDate *)date withThreads:(NSArray * BSGFeatureFlagStoreToJSON(BSGFeatureFlagStore *store); BSGFeatureFlagStore * BSGFeatureFlagStoreFromJSON(id _Nullable json); + +BSG_OBJC_DIRECT_MEMBERS +@interface BSGFeatureFlagStore () + +@property(nonatomic,nonnull,readonly) NSArray * allFlags; + ++ (nonnull BSGFeatureFlagStore *) fromJSON:(nonnull id)json; + +- (NSUInteger) count; + +- (void) addFeatureFlag:(nonnull NSString *)name withVariant:(nullable NSString *)variant; + +- (void) addFeatureFlags:(nonnull NSArray *)featureFlags; + +- (void) clear:(nullable NSString *)name; + +- (nonnull NSArray *) toJSON; + +@end + NS_ASSUME_NONNULL_END diff --git a/Bugsnag/Helpers/BSGFeatureFlagStore.m b/Bugsnag/Helpers/BSGFeatureFlagStore.m index 176211241..fd2ecaa77 100644 --- a/Bugsnag/Helpers/BSGFeatureFlagStore.m +++ b/Bugsnag/Helpers/BSGFeatureFlagStore.m @@ -11,43 +11,50 @@ #import "BSGKeys.h" #import "BugsnagFeatureFlag.h" -static void internalAddFeatureFlag(BSGFeatureFlagStore *store, BugsnagFeatureFlag *flag) { - [store removeObject:flag]; - [store addObject:flag]; -} - void BSGFeatureFlagStoreAddFeatureFlag(BSGFeatureFlagStore *store, NSString *name, NSString *_Nullable variant) { - internalAddFeatureFlag(store, [BugsnagFeatureFlag flagWithName:name variant:variant]); + [store addFeatureFlag:name withVariant:variant]; } void BSGFeatureFlagStoreAddFeatureFlags(BSGFeatureFlagStore *store, NSArray *featureFlags) { - for (BugsnagFeatureFlag *featureFlag in featureFlags) { - internalAddFeatureFlag(store, featureFlag); - } + [store addFeatureFlags:featureFlags]; } void BSGFeatureFlagStoreClear(BSGFeatureFlagStore *store, NSString *_Nullable name) { - if (name) { - [store removeObject:[BugsnagFeatureFlag flagWithName:(NSString * _Nonnull)name]]; - } else { - [store removeAllObjects]; - } + [store clear:name]; } NSArray * BSGFeatureFlagStoreToJSON(BSGFeatureFlagStore *store) { - NSMutableArray *result = [NSMutableArray array]; - for (BugsnagFeatureFlag *flag in store) { - if (flag.variant) { - [result addObject:@{BSGKeyFeatureFlag: flag.name, BSGKeyVariant: (NSString * _Nonnull)flag.variant}]; - } else { - [result addObject:@{BSGKeyFeatureFlag: flag.name}]; - } - } - return result; + return [store toJSON]; } BSGFeatureFlagStore * BSGFeatureFlagStoreFromJSON(id json) { - BSGFeatureFlagStore *store = [NSMutableArray array]; + return [BSGFeatureFlagStore fromJSON:json]; +} + + +/** + * Stores feature flags as a dictionary containing the flag name as a key, with the + * value being the index into an array containing the complete feature flag. + * + * Removals leave holes in the array, which gets rebuilt on clear once there are too many holes. + * + * This gives the access speed of a dictionary while keeping ordering intact. + */ +BSG_OBJC_DIRECT_MEMBERS +@interface BSGFeatureFlagStore () + +@property(nonatomic, readwrite) NSMutableArray *flags; +@property(nonatomic, readwrite) NSMutableDictionary *indices; + +@end + +static const int REBUILD_AT_HOLE_COUNT = 1000; + +BSG_OBJC_DIRECT_MEMBERS +@implementation BSGFeatureFlagStore + ++ (nonnull BSGFeatureFlagStore *) fromJSON:(nonnull id)json { + BSGFeatureFlagStore *store = [BSGFeatureFlagStore new]; if ([json isKindOfClass:[NSArray class]]) { for (id item in json) { if ([item isKindOfClass:[NSDictionary class]]) { @@ -57,10 +64,119 @@ void BSGFeatureFlagStoreClear(BSGFeatureFlagStore *store, NSString *_Nullable na if (![variant isKindOfClass:[NSString class]]) { variant = nil; } - [store addObject:[BugsnagFeatureFlag flagWithName:featureFlag variant:variant]]; + [store addFeatureFlag:featureFlag withVariant:variant]; } } } } return store; } + +- (nonnull instancetype) init { + if ((self = [super init]) != nil) { + _flags = [NSMutableArray new]; + _indices = [NSMutableDictionary new]; + } + return self; +} + +static inline int getIndexFromDict(NSDictionary *dict, NSString *name) { + NSNumber *boxedIndex = dict[name]; + if (boxedIndex == nil) { + return -1; + } + return boxedIndex.intValue; +} + +- (NSUInteger) count { + return self.indices.count; +} + +- (nonnull NSArray *) allFlags { + NSMutableArray *flags = [NSMutableArray arrayWithCapacity:self.indices.count]; + for (BugsnagFeatureFlag *flag in self.flags) { + if ([flag isKindOfClass:[BugsnagFeatureFlag class]]) { + [flags addObject:flag]; + } + } + return flags; +} + +- (void)rebuildIfTooManyHoles { + int holeCount = (int)self.flags.count - (int)self.indices.count; + if (holeCount < REBUILD_AT_HOLE_COUNT) { + return; + } + + NSMutableArray *newFlags = [NSMutableArray arrayWithCapacity:self.indices.count]; + NSMutableDictionary *newIndices = [NSMutableDictionary new]; + for (BugsnagFeatureFlag *flag in self.flags) { + if ([flag isKindOfClass:[BugsnagFeatureFlag class]]) { + [newFlags addObject:flag]; + } + } + + for (NSUInteger i = 0; i < newFlags.count; i++) { + BugsnagFeatureFlag *flag = newFlags[i]; + newIndices[flag.name] = @(i); + } + self.flags = newFlags; + self.indices = newIndices; +} + +- (void) addFeatureFlag:(nonnull NSString *)name withVariant:(nullable NSString *)variant { + BugsnagFeatureFlag *flag = [BugsnagFeatureFlag flagWithName:name variant:variant]; + + int index = getIndexFromDict(self.indices, name); + if (index >= 0) { + self.flags[(unsigned)index] = flag; + } else { + index = (int)self.flags.count; + [self.flags addObject:flag]; + self.indices[name] = @(index); + } +} + +- (void) addFeatureFlags:(nonnull NSArray *)featureFlags { + for (BugsnagFeatureFlag *flag in featureFlags) { + [self addFeatureFlag:flag.name withVariant:flag.variant]; + } +} + +- (void) clear:(nullable NSString *)name { + if (name != nil) { + int index = getIndexFromDict(self.indices, name); + if (index >= 0) { + self.flags[(unsigned)index] = [NSNull null]; + [self.indices removeObjectForKey:(id)name]; + [self rebuildIfTooManyHoles]; + } + } else { + [self.indices removeAllObjects]; + [self.flags removeAllObjects]; + } +} + +- (nonnull NSArray *) toJSON { + NSMutableArray *result = [NSMutableArray array]; + + for (BugsnagFeatureFlag *flag in self.flags) { + if ([flag isKindOfClass:[BugsnagFeatureFlag class]]) { + if (flag.variant) { + [result addObject:@{BSGKeyFeatureFlag:flag.name, BSGKeyVariant:(NSString *_Nonnull)flag.variant}]; + } else { + [result addObject:@{BSGKeyFeatureFlag:flag.name}]; + } + } + } + return result; +} + +- (id)copyWithZone:(NSZone *)zone { + BSGFeatureFlagStore *store = [[BSGFeatureFlagStore allocWithZone:zone] init]; + store.flags = [self.flags mutableCopy]; + store.indices = [self.indices mutableCopy]; + return store; +} + +@end diff --git a/Bugsnag/Payload/BugsnagEvent.m b/Bugsnag/Payload/BugsnagEvent.m index 60969a36c..6ac4b92c1 100644 --- a/Bugsnag/Payload/BugsnagEvent.m +++ b/Bugsnag/Payload/BugsnagEvent.m @@ -760,7 +760,7 @@ - (void)setUnhandled:(BOOL)unhandled { // MARK: - - (NSArray *)featureFlags { - return [self.featureFlagStore copy]; + return self.featureFlagStore.allFlags; } - (void)addFeatureFlagWithName:(NSString *)name variant:(nullable NSString *)variant { diff --git a/Tests/BugsnagTests/BSGFeatureFlagStoreTests.m b/Tests/BugsnagTests/BSGFeatureFlagStoreTests.m index 1c91646f0..8144687d6 100644 --- a/Tests/BugsnagTests/BSGFeatureFlagStoreTests.m +++ b/Tests/BugsnagTests/BSGFeatureFlagStoreTests.m @@ -44,12 +44,12 @@ - (void)test { XCTAssertEqualObjects(BSGFeatureFlagStoreToJSON(store), (@[ @{@"featureFlag": @"featureC", @"variant": @"checked"}, + @{@"featureFlag": @"featureA"}, @{@"featureFlag": @"featureB"}, - @{@"featureFlag": @"featureA"} ])); - XCTAssertEqualObjects(BSGFeatureFlagStoreFromJSON(BSGFeatureFlagStoreToJSON(store)), - store); + XCTAssertEqualObjects(BSGFeatureFlagStoreToJSON(BSGFeatureFlagStoreFromJSON(BSGFeatureFlagStoreToJSON(store))), + BSGFeatureFlagStoreToJSON(store)); BSGFeatureFlagStoreClear(store, @"featureB"); XCTAssertEqualObjects(BSGFeatureFlagStoreToJSON(store), @@ -62,4 +62,68 @@ - (void)test { XCTAssertEqualObjects(BSGFeatureFlagStoreToJSON(store), @[]); } +- (void)testAddRemoveMany { + // Tests that rebuildIfTooManyHoles works as expected + + BSGFeatureFlagStore *store = [[BSGFeatureFlagStore alloc] init]; + + BSGFeatureFlagStoreAddFeatureFlag(store, @"blah", @"testing"); + for (int j = 0; j < 10; j++) { + for (int i = 0; i < 1000; i++) { + NSString *name = [NSString stringWithFormat:@"%d-%d", j, i]; + BSGFeatureFlagStoreAddFeatureFlag(store, name, nil); + if (i < 999) { + BSGFeatureFlagStoreClear(store, name); + } + } + } + + XCTAssertEqualObjects(BSGFeatureFlagStoreToJSON(store), + (@[ + @{@"featureFlag": @"blah", @"variant": @"testing"}, + @{@"featureFlag": @"0-999"}, + @{@"featureFlag": @"1-999"}, + @{@"featureFlag": @"2-999"}, + @{@"featureFlag": @"3-999"}, + @{@"featureFlag": @"4-999"}, + @{@"featureFlag": @"5-999"}, + @{@"featureFlag": @"6-999"}, + @{@"featureFlag": @"7-999"}, + @{@"featureFlag": @"8-999"}, + @{@"featureFlag": @"9-999"}, + ])); +} + +- (void)testAddFeatureFlagPerformance { + BSGFeatureFlagStore *store = [[BSGFeatureFlagStore alloc] init]; + + __auto_type block = ^{ + for (int i = 0; i < 1000; i++) { + NSString *name = [NSString stringWithFormat:@"%d", i]; + BSGFeatureFlagStoreAddFeatureFlag(store, name, nil); + } + }; + + block(); + + [self measureBlock:block]; +} + +- (void)testDictionaryPerformance { + // For comparision to show the best performance possible + + NSMutableDictionary *dictionary = [NSMutableDictionary dictionary]; + + __auto_type block = ^{ + for (int i = 0; i < 1000; i++) { + NSString *name = [NSString stringWithFormat:@"%d", i]; + [dictionary setObject:[NSNull null] forKey:name]; + } + }; + + block(); + + [self measureBlock:block]; +} + @end