New feature flags store implementation that preserves insertion order.

Bugsnag/BugsnagInternals.h

@@ -22,7 +22,25 @@
 #import "BugsnagHandledState.h"
 #import "BugsnagNotifier.h"
 
-typedef NSMutableArray<BugsnagFeatureFlag *> BSGFeatureFlagStore;
+@interface BSGFeatureFlagStore : NSObject
+
+@property(nonatomic,nonnull,readonly) NSArray<BugsnagFeatureFlag *> * allFlags;
+
++ (BSGFeatureFlagStore *_Nonnull) fromJSON:(id _Nonnull )json;
+
+- (_Nonnull instancetype) init;
+
+- (NSUInteger) count;
+
+- (void) addFeatureFlag:(NSString *_Nonnull)name withVariant:(NSString *_Nullable)variant;
+
+- (void) addFeatureFlags:(NSArray<BugsnagFeatureFlag *> *_Nonnull)featureFlags;
+
+- (void) clear:(NSString *_Nullable)name;
+
+- (NSArray<NSDictionary *> *_Nonnull) toJSON;
+
+@end
 
 NS_ASSUME_NONNULL_BEGIN
 

Bugsnag/Client/BugsnagClient.m

@@ -185,7 +185,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: @{
@@ -752,7 +752,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];
         }
     }
 
@@ -966,7 +966,7 @@ - (void)setObserver:(BSGClientObserver)observer {
         };
 
         @synchronized (self.featureFlagStore) {
-            for (BugsnagFeatureFlag *flag in self.featureFlagStore) {
+            for (BugsnagFeatureFlag *flag in self.featureFlagStore.allFlags) {
                 observer(BSGClientObserverAddFeatureFlag, flag);
             }
         }
@@ -1030,7 +1030,7 @@ - (void)appHangDetectedAtDate:(NSDate *)date withThreads:(NSArray<BugsnagThread
     self.appHangEvent.context = self.context;
 
     @synchronized (self.featureFlagStore) {
-        self.appHangEvent.featureFlagStore = [self.featureFlagStore mutableCopy];
+        self.appHangEvent.featureFlagStore = [self.featureFlagStore copy];
     }
 
     [self.appHangEvent symbolicateIfNeeded];

Bugsnag/Helpers/BSGFeatureFlagStore.m

@@ -11,43 +11,48 @@
 #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<BugsnagFeatureFlag *> *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<NSDictionary *> * BSGFeatureFlagStoreToJSON(BSGFeatureFlagStore *store) {
-    NSMutableArray<NSDictionary *> *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.
+ */
+@interface BSGFeatureFlagStore ()
+
+@property(nonatomic, readwrite) NSMutableArray *flags;
+@property(nonatomic, readwrite) NSMutableDictionary *indices;
+
+@end
+
+static const int REBUILD_AT_HOLE_COUNT = 1000;
+
+@implementation BSGFeatureFlagStore
+
++ (BSGFeatureFlagStore *_Nonnull) fromJSON:(id _Nonnull )json {
+    BSGFeatureFlagStore *store = [BSGFeatureFlagStore new];
     if ([json isKindOfClass:[NSArray class]]) {
         for (id item in json) {
             if ([item isKindOfClass:[NSDictionary class]]) {
@@ -57,10 +62,150 @@ 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;
+}
+
+- (BOOL)isEqual:(nullable id)object {
+    if (object == nil) {
+        return NO;
+    }
+
+    if (self == object) {
+        return YES;
+    }
+
+    if (![object isKindOfClass:[BSGFeatureFlagStore class]]) {
+        return NO;
+    }
+
+    BSGFeatureFlagStore *other = (BSGFeatureFlagStore *)object;
+
+    if (self.indices.count != other.indices.count) {
+        return NO;
+    }
+
+    for (NSString *name in self.indices.allKeys) {
+        NSNumber *selfIndex = self.indices[name];
+        NSNumber *otherIndex = other.indices[name];
+        if (otherIndex == nil) {
+            return NO;
+        }
+        if (![self.flags[selfIndex.unsignedIntValue]isEqual:other.flags[otherIndex.unsignedIntValue]]) {
+            return NO;
+        }
+    }
+    return YES;
+}
+
+- (NSUInteger) count {
+    return self.indices.count;
+}
+
+- (NSArray<BugsnagFeatureFlag *> *_Nonnull) allFlags {
+    NSMutableArray<BugsnagFeatureFlag *> *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] = [NSNumber numberWithInt:(int)i];
+    }
+    self.flags = newFlags;
+    self.indices = newIndices;
+}
+
+- (void) addFeatureFlag:(NSString *_Nonnull)name withVariant:(NSString *_Nullable)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] = [NSNumber numberWithInt:index];
+    }
+}
+
+- (void) addFeatureFlags:(NSArray<BugsnagFeatureFlag *> *_Nonnull)featureFlags {
+    for (BugsnagFeatureFlag *flag in featureFlags) {
+        [self addFeatureFlag:flag.name withVariant:flag.variant];
+    }
+}
+
+- (void) clear:(NSString *_Nullable)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];
+    }
+}
+
+- (NSArray<NSDictionary *> *_Nonnull) toJSON {
+    NSMutableArray<NSDictionary *> *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;
+}
+
+- (instancetype) copy {
+    BSGFeatureFlagStore *store = [BSGFeatureFlagStore new];
+    store.flags = [self.flags mutableCopy];
+    store.indices = [self.indices mutableCopy];
+    return store;
+}
+@end

Bugsnag/Payload/BugsnagEvent.m

@@ -759,7 +759,7 @@ - (void)setUnhandled:(BOOL)unhandled {
 // MARK: - <BugsnagFeatureFlagStore>
 
 - (NSArray<BugsnagFeatureFlag *> *)featureFlags {
-    return [self.featureFlagStore copy];
+    return self.featureFlagStore.allFlags;
 }
 
 - (void)addFeatureFlagWithName:(NSString *)name variant:(nullable NSString *)variant {

Tests/BugsnagTests/BSGFeatureFlagStoreTests.m

@@ -44,8 +44,8 @@ - (void)test {
     XCTAssertEqualObjects(BSGFeatureFlagStoreToJSON(store),
                           (@[
                             @{@"featureFlag": @"featureC", @"variant": @"checked"},
+                            @{@"featureFlag": @"featureA"},
                             @{@"featureFlag": @"featureB"},
-                            @{@"featureFlag": @"featureA"}
                           ]));
 
     XCTAssertEqualObjects(BSGFeatureFlagStoreFromJSON(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