Skip to content

Commit

Permalink
Relying on CustomStringConvertible for description of FlagSyncResult
Browse files Browse the repository at this point in the history 
Currently the SDK relies on performing flag sync up post-processing on the Main Thread, this in turn triggers LDClient.onFlagSyncComplete(result:) which has some os_log calls where the result is transformed into a String using String.init(describing:), this in turn uses Swift default implementation on stringifying objects which for large collections can be a big bottle-neck, we could easily identify instances where the Main Thread was running this piece of code for 200ms (we have around 280 flags in our project), causing hitches/hangs in our project.

To address the issue I've conformed FlagSyncResult to CustomStringConvertible and provided a custom description, it mostly behaves the same with the exception for .flagCollection case in which we use JSONEncoder to stringify the list of flags.

We've also transformed StoredItems typealias into a struct wrapping the dictionary in order to be able to provide a similar CustomStringConvertible conformance.

With this change we could notice the execution time of LDClient.onFlagSyncComplete(result:) has dropped to levels between 30-40% of the original execution time.
  • Loading branch information
@rkreutz
rkreutz committed Jan 13, 2025
1 parent a378e29 commit 0a6c588
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 29 deletions.
50 changes: 46 additions & 4 deletions LaunchDarkly/LaunchDarkly/ServiceObjects/FlagStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,59 @@ enum StorageItem: Codable {
}
}

typealias StoredItems = [LDFlagKey: StorageItem]
extension StoredItems {
struct StoredItems {
var items: [LDFlagKey: StorageItem]

var featureFlags: [LDFlagKey: FeatureFlag] {
self.compactMapValues {
items.compactMapValues {
guard case .item(let flag) = $0 else { return nil }
return flag
}
}

var isEmpty: Bool {
items.isEmpty
}

subscript (key: LDFlagKey) -> StorageItem? {
get { items[key] }
set { items[key] = newValue }
}

mutating func updateValue(_ newValue: StorageItem, forKey key: LDFlagKey) {
items.updateValue(newValue, forKey: key)
}
}

extension StoredItems {
init(items: [LDFlagKey: FeatureFlag]) {
self = items.mapValues { .item($0) }
self.items = items.mapValues { .item($0) }
}
}

extension StoredItems: ExpressibleByDictionaryLiteral {
init(dictionaryLiteral elements: (LDFlagKey, StorageItem)...) {
self.items = Dictionary(uniqueKeysWithValues: elements)
}
}

extension StoredItems: Decodable {
init(from decoder: any Decoder) throws {
let container = try decoder.singleValueContainer()
self.items = try container.decode([LDFlagKey: StorageItem].self)
}
}

extension StoredItems: Encodable {
func encode(to encoder: any Encoder) throws {
var container = encoder.singleValueContainer()
try container.encode(items)
}
}

extension StoredItems: CustomStringConvertible {
var description: String {
String(data: (try? JSONEncoder().encode(items)) ?? Data(), encoding: .utf8) ?? ""
}
}

Expand Down
17 changes: 17 additions & 0 deletions LaunchDarkly/LaunchDarkly/ServiceObjects/FlagSynchronizer.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,23 @@ enum FlagSyncResult {
case error(SynchronizingError)
}

extension FlagSyncResult: CustomStringConvertible {
var description: String {
switch self {
case .flagCollection(let flags, let lastUpdated):
return "flagCollection(\(String(data: (try? JSONEncoder().encode(flags.flags)) ?? Data(), encoding: .utf8) ?? ""), \(lastUpdated.flatMap(\.description) ?? ""))"
case .patch(let flag):
return "patch(\(flag))"
case .delete(let deleteResponse):
return "delete(\(deleteResponse))"
case .upToDate:
return "upToDate"
case .error(let error):
return "error(\(error))"
}
}
}

struct DeleteResponse: Decodable {
let key: String
let version: Int?
Expand Down
16 changes: 8 additions & 8 deletions LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,14 @@ final class LDClientSpec: QuickSpec {
}
}
it("when called with cached flags for the context and environment") {
let cachedFlags = ["test-flag": StorageItem.item(FeatureFlag(flagKey: "test-flag"))]
let cachedFlags: StoredItems = ["test-flag": StorageItem.item(FeatureFlag(flagKey: "test-flag"))]
let testContext = TestContext().withCached(flags: cachedFlags.featureFlags)
withTimeout ? testContext.start(timeOut: 10.0) : testContext.start()

expect(testContext.featureFlagCachingMock.getCachedDataCallCount) == 1
expect(testContext.featureFlagCachingMock.getCachedDataReceivedArguments?.cacheKey) == testContext.context.fullyQualifiedHashedKey()

expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags) == cachedFlags
expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags?.items) == cachedFlags.items

expect(testContext.serviceFactoryMock.makeCacheConverterReturnValue.convertCacheDataCallCount) == 1
expect(testContext.serviceFactoryMock.makeCacheConverterReturnValue.convertCacheDataReceivedArguments?.maxCachedContexts) == testContext.config.maxCachedContexts
Expand Down Expand Up @@ -491,7 +491,7 @@ final class LDClientSpec: QuickSpec {

expect(testContext.subject.context) == newContext
expect(testContext.flagStoreMock.replaceStoreCallCount) == 1
expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags) == stubFlags
expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags?.items) == stubFlags.items
}

it("when we have opted into auto environment attributes") {
Expand Down Expand Up @@ -573,7 +573,7 @@ final class LDClientSpec: QuickSpec {

expect(testContext.subject.context) == newContext
expect(testContext.flagStoreMock.replaceStoreCallCount) == 1
expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags) == stubFlags
expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags?.items) == stubFlags.items
}
}
}
Expand Down Expand Up @@ -896,14 +896,14 @@ final class LDClientSpec: QuickSpec {
waitUntil { done in
testContext.changeNotifierMock.notifyObserversCallback = done
updateDate = Date()
testContext.onSyncComplete?(.flagCollection((FeatureFlagCollection(newStoredItems.featureFlags), nil)))
testContext.onSyncComplete?(.flagCollection((FeatureFlagCollection(StoredItems(items: newStoredItems).featureFlags), nil)))
}

expect(testContext.flagStoreMock.replaceStoreCallCount) == 1
expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags) == newStoredItems
expect(testContext.flagStoreMock.replaceStoreReceivedNewFlags?.items) == newStoredItems

expect(testContext.featureFlagCachingMock.saveCachedDataCallCount) == 1
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.storedItems) == newStoredItems
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.storedItems.items) == newStoredItems
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.cacheKey) == testContext.context.fullyQualifiedHashedKey()
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.lastUpdated).to(beCloseTo(updateDate, within: Constants.updateThreshold))

Expand All @@ -930,7 +930,7 @@ final class LDClientSpec: QuickSpec {
expect(testContext.flagStoreMock.updateStoreReceivedUpdatedFlag) == updateFlag

expect(testContext.featureFlagCachingMock.saveCachedDataCallCount) == 1
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.storedItems) == testContext.flagStoreMock.storedItems
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.storedItems.items) == testContext.flagStoreMock.storedItems.items
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.cacheKey) == testContext.context.fullyQualifiedHashedKey()
expect(testContext.featureFlagCachingMock.saveCachedDataReceivedArguments?.lastUpdated).to(beCloseTo(updateDate, within: Constants.updateThreshold))

Expand Down
2 changes: 1 addition & 1 deletion LaunchDarkly/LaunchDarklyTests/Models/FeatureFlag/FeatureFlagSpec.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ final class FeatureFlagSpec: XCTestCase {
]
let encoded = try JSONEncoder().encode(testData)
let storedItemCollection = try JSONDecoder().decode(StoredItemCollection.self, from: encoded)
XCTAssertEqual(storedItemCollection.flags.count, 2)
XCTAssertEqual(storedItemCollection.flags.items.count, 2)
XCTAssertEqual(storedItemCollection.flags.featureFlags["key1"]?.flagKey, "key1")
XCTAssertNil(storedItemCollection.flags.featureFlags["key2"])
}
Expand Down
10 changes: 5 additions & 5 deletions LaunchDarkly/LaunchDarklyTests/ServiceObjects/Cache/FeatureFlagCacheSpec.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ final class FeatureFlagCacheSpec: XCTestCase {
func testRetrieveEmptyData() throws {
mockValueCache.dataReturnValue = try JSONEncoder().encode(StoredItemCollection([:]))
let flagCache = FeatureFlagCache(serviceFactory: serviceFactory, mobileKey: "abc", maxCachedContexts: 2)
XCTAssertEqual(flagCache.getCachedData(cacheKey: "context1", contextHash: "context").items?.count, 0)
XCTAssertEqual(flagCache.getCachedData(cacheKey: "context1", contextHash: "context").items?.items.count, 0)
}

func testRetrieveValidData() throws {
mockValueCache.dataReturnValue = try JSONEncoder().encode(testFlagCollection)
let flagCache = FeatureFlagCache(serviceFactory: serviceFactory, mobileKey: "abc", maxCachedContexts: 1)
let retrieved = flagCache.getCachedData(cacheKey: "context1", contextHash: "contextHash")
XCTAssertEqual(retrieved.items, testFlagCollection.flags)
XCTAssertEqual(retrieved.items?.items, testFlagCollection.flags.items)
XCTAssertEqual(mockValueCache.dataCallCount, 2)
XCTAssertEqual(mockValueCache.dataReceivedForKey, "fingerprint-context1")
}
Expand All @@ -75,7 +75,7 @@ final class FeatureFlagCacheSpec: XCTestCase {
flagCache.saveCachedData(testFlagCollection.flags, cacheKey: "key", contextHash: "hash", lastUpdated: now, etag: "example-etag")

let results = flagCache.getCachedData(cacheKey: "key", contextHash: "hash")
XCTAssertEqual(results.items, testFlagCollection.flags)
XCTAssertEqual(results.items?.items, testFlagCollection.flags.items)
XCTAssertEqual(results.etag, "example-etag")
XCTAssertEqual(results.lastUpdated!.millisSince1970, now.millisSince1970, accuracy: 1_000)
}
Expand All @@ -86,7 +86,7 @@ final class FeatureFlagCacheSpec: XCTestCase {
flagCache.saveCachedData(testFlagCollection.flags, cacheKey: "key", contextHash: "hash", lastUpdated: now, etag: "example-etag")

let results = flagCache.getCachedData(cacheKey: "key", contextHash: "changed-hash")
XCTAssertEqual(results.items, testFlagCollection.flags)
XCTAssertEqual(results.items?.items, testFlagCollection.flags.items)
XCTAssertEqual(results.etag, nil)
XCTAssertEqual(results.lastUpdated, nil)
}
Expand All @@ -97,7 +97,7 @@ final class FeatureFlagCacheSpec: XCTestCase {
flagCache.saveCachedData(testFlagCollection.flags, cacheKey: "key", contextHash: "hash", lastUpdated: now, etag: "example-etag")

let results = flagCache.getCachedData(cacheKey: "changed-key", contextHash: "hash")
XCTAssertEqual(results.items, nil)
XCTAssertEqual(results.items?.items, nil)
XCTAssertEqual(results.etag, nil)
XCTAssertEqual(results.lastUpdated, nil)
}
Expand Down
22 changes: 11 additions & 11 deletions LaunchDarkly/LaunchDarklyTests/ServiceObjects/FlagStoreSpec.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,38 @@ final class FlagStoreSpec: XCTestCase {
let stubFlags = DarklyServiceMock.Constants.stubFeatureFlags()

func testInit() {
XCTAssertEqual(FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests")).storedItems, [:])
XCTAssertEqual(FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests")).storedItems.items, [:])
XCTAssertEqual(FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: self.stubFlags)).storedItems.featureFlags, self.stubFlags)
}

func testReplaceStore() {
let featureFlags = StoredItems(items: DarklyServiceMock.Constants.stubFeatureFlags())
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"))
flagStore.replaceStore(newStoredItems: featureFlags)
XCTAssertEqual(flagStore.storedItems, featureFlags)
XCTAssertEqual(flagStore.storedItems.items, featureFlags.items)
}

func testUpdateStoreNewFlag() {
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: stubFlags))
let flagUpdate = FeatureFlag(flagKey: "new-int-flag", value: "abc", version: 0)
flagStore.updateStore(updatedFlag: flagUpdate)
XCTAssertEqual(flagStore.storedItems.count, stubFlags.count + 1)
XCTAssertEqual(flagStore.storedItems.items.count, stubFlags.count + 1)
XCTAssertEqual(flagStore.storedItems.featureFlags["new-int-flag"], flagUpdate)
}

func testUpdateStoreNewerVersion() {
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: stubFlags))
let flagUpdate = DarklyServiceMock.Constants.stubFeatureFlag(for: DarklyServiceMock.FlagKeys.int, useAlternateVersion: true)
flagStore.updateStore(updatedFlag: flagUpdate)
XCTAssertEqual(flagStore.storedItems.count, stubFlags.count)
XCTAssertEqual(flagStore.storedItems.items.count, stubFlags.count)
XCTAssertEqual(flagStore.storedItems.featureFlags[DarklyServiceMock.FlagKeys.int], flagUpdate)
}

func testUpdateStoreNoVersion() {
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: stubFlags))
let flagUpdate = FeatureFlag(flagKey: DarklyServiceMock.FlagKeys.int, value: "abc", version: nil)
flagStore.updateStore(updatedFlag: flagUpdate)
XCTAssertEqual(flagStore.storedItems.count, stubFlags.count)
XCTAssertEqual(flagStore.storedItems.items.count, stubFlags.count)
XCTAssertEqual(flagStore.storedItems.featureFlags[DarklyServiceMock.FlagKeys.int], flagUpdate)
}

Expand All @@ -57,15 +57,15 @@ final class FlagStoreSpec: XCTestCase {
func testDeleteFlagNewerVersion() {
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: stubFlags))
flagStore.deleteFlag(deleteResponse: DeleteResponse(key: DarklyServiceMock.FlagKeys.int, version: DarklyServiceMock.Constants.version + 1))
XCTAssertEqual(flagStore.storedItems.count, self.stubFlags.count)
XCTAssertEqual(flagStore.storedItems.items.count, self.stubFlags.count)
XCTAssertEqual(flagStore.storedItems.featureFlags.count, self.stubFlags.count - 1)
XCTAssertEqual(StorageItem.tombstone(5), flagStore.storedItems[DarklyServiceMock.FlagKeys.int])
}

func testDeleteFlagMissingVersion() {
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: stubFlags))
flagStore.deleteFlag(deleteResponse: DeleteResponse(key: DarklyServiceMock.FlagKeys.int, version: nil))
XCTAssertEqual(flagStore.storedItems.count, self.stubFlags.count)
XCTAssertEqual(flagStore.storedItems.items.count, self.stubFlags.count)
XCTAssertEqual(flagStore.storedItems.featureFlags.count, self.stubFlags.count - 1)
XCTAssertEqual(StorageItem.tombstone(0), flagStore.storedItems[DarklyServiceMock.FlagKeys.int])
}
Expand All @@ -82,21 +82,21 @@ final class FlagStoreSpec: XCTestCase {
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: stubFlags))
let flagUpdate = FeatureFlag(flagKey: "new-int-flag", value: "abc", version: 0)
flagStore.updateStore(updatedFlag: flagUpdate)
XCTAssertEqual(stubFlags.count + 1, flagStore.storedItems.count)
XCTAssertEqual(stubFlags.count + 1, flagStore.storedItems.items.count)
XCTAssertEqual(stubFlags.count + 1, flagStore.storedItems.featureFlags.count)

flagStore.deleteFlag(deleteResponse: DeleteResponse(key: "new-int-flag", version: 1))
XCTAssertEqual(stubFlags.count + 1, flagStore.storedItems.count)
XCTAssertEqual(stubFlags.count + 1, flagStore.storedItems.items.count)
XCTAssertEqual(stubFlags.count, flagStore.storedItems.featureFlags.count)

flagStore.updateStore(updatedFlag: flagUpdate)
XCTAssertEqual(stubFlags.count + 1, flagStore.storedItems.count)
XCTAssertEqual(stubFlags.count + 1, flagStore.storedItems.items.count)
XCTAssertEqual(stubFlags.count, flagStore.storedItems.featureFlags.count)
}

func testFeatureFlag() {
let flagStore = FlagStore(logger: OSLog(subsystem: "com.launchdarkly", category: "tests"), storedItems: StoredItems(items: stubFlags))
flagStore.storedItems.forEach { flagKey, featureFlag in
flagStore.storedItems.items.forEach { flagKey, featureFlag in
guard case .item(let flag) = featureFlag
else {
XCTAssertNil(flagStore.featureFlag(for: flagKey))
Expand Down

0 comments on commit 0a6c588

Please sign in to comment.